Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Enable TRY and FLY Ruff rules #2264

Merged
merged 4 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,8 @@ select = [
"PLE", # pylint (error)
"PLR", # pylint (refactor)
"PLW", # pylint (warning)
"TRY", # tryceratops
"FLY", # flynt
"PERF", # perflint
"RUF", # ruff
]
Expand Down
4 changes: 4 additions & 0 deletions singer_sdk/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,10 @@ class MissingKeyPropertiesError(Exception):
"""Raised when a recieved (and/or transformed) record is missing key properties."""


class InvalidInputLine(Exception):
"""Raised when an input line is not a valid Singer message."""


class InvalidJSONSchema(Exception):
"""Raised when a JSON schema is invalid."""

Expand Down
6 changes: 4 additions & 2 deletions singer_sdk/helpers/_catalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@
f"'{type(schema_at_breadcrumb).__name__}' '{schema_at_breadcrumb}' for "
f"'{stream_name}' bookmark '{breadcrumb!s}' in '{schema}'"
)
raise ValueError(msg)
# TODO: this should be a ValueError, but it's a breaking change.
raise ValueError(msg) # noqa: TRY004

Check warning on line 52 in singer_sdk/helpers/_catalog.py

View check run for this annotation

Codecov / codecov/patch

singer_sdk/helpers/_catalog.py#L52

Added line #L52 was not covered by tests

if "properties" not in schema_at_breadcrumb:
return
Expand Down Expand Up @@ -126,7 +127,8 @@
f"Expected tuple value for breadcrumb '{breadcrumb}'. Got "
f"{type(breadcrumb).__name__}"
)
raise ValueError(msg)
# TODO: this should be a ValueError, but it's a breaking change.
raise ValueError(msg) # noqa: TRY004

Check warning on line 131 in singer_sdk/helpers/_catalog.py

View check run for this annotation

Codecov / codecov/patch

singer_sdk/helpers/_catalog.py#L131

Added line #L131 was not covered by tests

catalog_entry = catalog.get_stream(stream_name)
if not catalog_entry:
Expand Down
7 changes: 4 additions & 3 deletions singer_sdk/io_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from singer_sdk._singerlib.messages import Message, SingerMessageType
from singer_sdk._singerlib.messages import format_message as singer_format_message
from singer_sdk._singerlib.messages import write_message as singer_write_message
from singer_sdk.exceptions import InvalidInputLine

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -44,12 +45,12 @@ def _assert_line_requires(line_dict: dict, requires: set[str]) -> None:
requires: TODO

Raises:
Exception: TODO
InvalidInputLine: raised if any required keys are missing
"""
if not requires.issubset(line_dict):
missing = requires - set(line_dict)
msg = f"Line is missing required {', '.join(missing)} key(s): {line_dict}"
raise Exception(msg) # TODO: Raise a more specific exception
raise InvalidInputLine(msg)

def deserialize_json(self, line: str) -> dict: # noqa: PLR6301
"""Deserialize a line of json.
Expand All @@ -69,7 +70,7 @@ def deserialize_json(self, line: str) -> dict: # noqa: PLR6301
parse_float=decimal.Decimal,
)
except json.decoder.JSONDecodeError as exc:
logger.error("Unable to parse:\n%s", line, exc_info=exc)
logger.exception("Unable to parse:\n%s", line, exc_info=exc)
raise

def _process_lines(self, file_input: t.IO[str]) -> t.Counter[str]:
Expand Down
2 changes: 1 addition & 1 deletion singer_sdk/plugin_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def invoke(self, ctx: click.Context) -> t.Any: # noqa: ANN401
return super().invoke(ctx)
except ConfigValidationError as exc:
for error in exc.errors:
self.logger.error("Config validation error: %s", error)
self.logger.exception("Config validation error: %s", error)
edgarrmondragon marked this conversation as resolved.
Show resolved Hide resolved
sys.exit(1)


Expand Down
4 changes: 2 additions & 2 deletions singer_sdk/sinks/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -494,10 +494,10 @@ def _validate_and_parse(self, record: dict) -> dict:
# on every record, so it's probably bad and should be moved up the stack.
try:
self._validator.validate(record)
except InvalidRecord as e:
except InvalidRecord:
if self.fail_on_record_validation_exception:
raise
self.logger.exception("Record validation failed %s", e)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was flagged by Ruff's TRY401 as being redundant

self.logger.exception("Record validation failed")

self._parse_timestamps_in_record(
record=record,
Expand Down
26 changes: 9 additions & 17 deletions singer_sdk/streams/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -905,25 +905,17 @@

Args:
current_record_index: The zero-based index of the current record.

Raises:
AbortedSyncFailedException: Raised if sync could not reach a valid state.
AbortedSyncPausedException: Raised if sync was able to be transitioned into
a valid state without data loss or corruption.
"""
if (
self.ABORT_AT_RECORD_COUNT is not None
and current_record_index > self.ABORT_AT_RECORD_COUNT - 1
):
try:
self._abort_sync(
abort_reason=MaxRecordsLimitException(
"Stream prematurely aborted due to the stream's max dry run "
f"record limit ({self.ABORT_AT_RECORD_COUNT}) being reached.",
),
)
except (AbortedSyncFailedException, AbortedSyncPausedException) as ex:
raise ex
Comment on lines -925 to -926
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was also redundant

self._abort_sync(
abort_reason=MaxRecordsLimitException(
"Stream prematurely aborted due to the stream's max dry run "
f"record limit ({self.ABORT_AT_RECORD_COUNT}) being reached.",
),
)

def _abort_sync(self, abort_reason: Exception) -> None:
"""Handle a sync operation being aborted.
Expand Down Expand Up @@ -1106,7 +1098,7 @@
state_partition_context=state_partition_context,
stream_name=self.name,
)
raise ex
raise

Check warning on line 1101 in singer_sdk/streams/core.py

View check run for this annotation

Codecov / codecov/patch

singer_sdk/streams/core.py#L1101

Added line #L1101 was not covered by tests

if selected:
if write_messages:
Expand Down Expand Up @@ -1189,12 +1181,12 @@
# Sync the records themselves:
for _ in self._sync_records(context=context):
pass
except Exception as ex:
except Exception:
self.logger.exception(
"An unhandled error occurred while syncing '%s'",
self.name,
)
raise ex
raise

def _sync_children(self, child_context: dict | None) -> None:
if child_context is None:
Expand Down
2 changes: 1 addition & 1 deletion singer_sdk/streams/rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ def get_url(self, context: dict | None) -> str:
vals = copy.copy(dict(self.config))
vals.update(context or {})
for k, v in vals.items():
search_text = "".join(["{", k, "}"])
search_text = f"{{{k}}}"
if search_text in url:
url = url.replace(search_text, self._url_encode(v))
return url
Expand Down
2 changes: 1 addition & 1 deletion singer_sdk/tap_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ def cb_discover(
)
except ConfigValidationError as exc: # pragma: no cover
for error in exc.errors:
cls.logger.error("Config validation error: %s", error)
cls.logger.exception("Config validation error: %s", error)
edgarrmondragon marked this conversation as resolved.
Show resolved Hide resolved
ctx.exit(1)
tap.run_discovery()
ctx.exit()
Expand Down
2 changes: 1 addition & 1 deletion singer_sdk/target_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ def add_sink(
try:
sink.setup()
except Exception: # pragma: no cover
self.logger.error("Error initializing '%s' target sink", self.name)
self.logger.exception("Error initializing '%s' target sink", self.name)
edgarrmondragon marked this conversation as resolved.
Show resolved Hide resolved
raise

self._sinks_active[stream_name] = sink
Expand Down
2 changes: 1 addition & 1 deletion singer_sdk/testing/tap_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ def test(self) -> None:
for v in self.non_null_attribute_values:
error_message = f"Unable to cast value ('{v}') to float type."
if not isinstance(v, (float, int)):
raise AssertionError(error_message)
raise AssertionError(error_message) # noqa: TRY004

@classmethod
def evaluate(
Expand Down
6 changes: 4 additions & 2 deletions singer_sdk/typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,8 @@
f"Type dict for {wrapped} is not defined. Try instantiating it with a "
f"nested type such as {wrapped.__name__}(StringType)."
)
raise ValueError(msg)
# TODO: this should be a TypeError, but it's a breaking change.
raise ValueError(msg) # noqa: TRY004

return t.cast(dict, wrapped.type_dict)

Expand Down Expand Up @@ -1070,7 +1071,8 @@
type_name = from_type.__name__
else:
msg = "Expected `str` or a SQLAlchemy `TypeEngine` object or type."
raise ValueError(msg)
# TODO: this should be a TypeError, but it's a breaking change.
raise ValueError(msg) # noqa: TRY004

Check warning on line 1075 in singer_sdk/typing.py

View check run for this annotation

Codecov / codecov/patch

singer_sdk/typing.py#L1075

Added line #L1075 was not covered by tests

return next(
(
Expand Down