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 all commits
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
10 changes: 6 additions & 4 deletions singer_sdk/helpers/_catalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,14 @@ def _pop_deselected_schema(
for crumb in breadcrumb:
schema_at_breadcrumb = schema_at_breadcrumb.get(crumb, {})

if not isinstance(schema_at_breadcrumb, dict):
if not isinstance(schema_at_breadcrumb, dict): # pragma: no cover
msg = (
"Expected dictionary type instead of "
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

if "properties" not in schema_at_breadcrumb:
return
Expand Down Expand Up @@ -121,12 +122,13 @@ def set_catalog_stream_selected(
breadcrumb is the path to a property within the stream.
"""
breadcrumb = breadcrumb or ()
if not isinstance(breadcrumb, tuple):
if not isinstance(breadcrumb, tuple): # pragma: no cover
msg = (
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

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.error("Config validation error: %s", error) # noqa: TRY400
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
28 changes: 10 additions & 18 deletions singer_sdk/streams/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -905,25 +905,17 @@ def _check_max_record_limit(self, current_record_index: int) -> None:

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 @@ -1096,7 +1088,7 @@ def _sync_records( # noqa: C901
child_context=child_context,
partition_context=state_partition_context,
)
except InvalidStreamSortException as ex:
except InvalidStreamSortException as ex: # pragma: no cover
log_sort_error(
log_fn=self.logger.error,
ex=ex,
Expand All @@ -1106,7 +1098,7 @@ def _sync_records( # noqa: C901
state_partition_context=state_partition_context,
stream_name=self.name,
)
raise ex
raise

if selected:
if write_messages:
Expand Down Expand Up @@ -1189,12 +1181,12 @@ def sync(self, context: dict | None = None) -> None:
# 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.error("Config validation error: %s", error) # noqa: TRY400
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.error("Error initializing '%s' target sink", self.name) # noqa: TRY400
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
8 changes: 5 additions & 3 deletions singer_sdk/typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,8 @@ def type_dict(self) -> dict: # type: ignore[override]
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 @@ -1068,9 +1069,10 @@ def to_jsonschema_type(
sa.types.TypeEngine,
):
type_name = from_type.__name__
else:
else: # pragma: no cover
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

return next(
(
Expand Down