Skip to content

Commit

Permalink
chore: Apply repo-review to this project (#2645)
Browse files Browse the repository at this point in the history
Applied `pipx run 'sp-repo-review[cli]' .` to this proejct.
  • Loading branch information
edgarrmondragon authored Sep 6, 2024
1 parent 640e00b commit 63cc0a3
Show file tree
Hide file tree
Showing 15 changed files with 45 additions and 37 deletions.
17 changes: 15 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,15 @@ types-PyYAML = ">=6.0.12"
pytest-codspeed = ">=2.2.0"

[tool.pytest.ini_options]
addopts = '--durations=10 --ignore=singer_sdk/helpers/_simpleeval.py -m "not external"'
addopts = [
"--durations=10",
"--ignore=singer_sdk/helpers/_simpleeval.py",
"-m",
"not external",
"-ra",
"--strict-config",
"--strict-markers",
]
filterwarnings = [
"error",
"ignore:Could not configure external gitlab tests:UserWarning",
Expand All @@ -167,13 +175,16 @@ filterwarnings = [
# https://github.com/joblib/joblib/pull/1518
"ignore:Attribute n is deprecated:DeprecationWarning:joblib._utils",
]
log_cli_level = "INFO"
markers = [
"external: Tests relying on external resources",
"windows: Tests that only run on Windows",
"snapshot: Tests that use pytest-snapshot",
]
minversion = "7"
testpaths = ["tests"]
norecursedirs = "cookiecutter"
xfail_strict = false

[tool.commitizen]
name = "cz_version_bump"
Expand Down Expand Up @@ -257,11 +268,14 @@ DEP002 = [
]

[tool.mypy]
enable_error_code = ["ignore-without-code", "redundant-expr", "truthy-bool"]
exclude = "tests"
files = "singer_sdk"
local_partial_types = true
strict = false
warn_redundant_casts = true
warn_return_any = true
warn_unreachable = true
warn_unused_configs = true
warn_unused_ignores = true

Expand Down Expand Up @@ -295,7 +309,6 @@ extend-exclude = [
"*simpleeval*",
]
line-length = 88
src = ["samples", "singer_sdk", "tests"]
target-version = "py38"

[tool.ruff.format]
Expand Down
4 changes: 2 additions & 2 deletions singer_sdk/_singerlib/encoding/_simple.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,9 @@ def __post_init__(self) -> None:
self.type = SingerMessageType.SCHEMA

if isinstance(self.bookmark_properties, (str, bytes)):
self.bookmark_properties = [self.bookmark_properties]
self.bookmark_properties = [self.bookmark_properties] # type: ignore[unreachable]
if self.bookmark_properties and not isinstance(self.bookmark_properties, list):
msg = "bookmark_properties must be a string or list of strings"
msg = "bookmark_properties must be a string or list of strings" # type: ignore[unreachable]
raise ValueError(msg)


Expand Down
12 changes: 6 additions & 6 deletions singer_sdk/authenticators.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def __call__(cls, *args: t.Any, **kwargs: t.Any) -> t.Any: # noqa: ANN401
A singleton instance of the derived class.
"""
if cls.__single_instance:
return cls.__single_instance
return cls.__single_instance # type: ignore[unreachable]
single_obj = cls.__new__(cls, None) # type: ignore[call-overload]
single_obj.__init__(*args, **kwargs)
cls.__single_instance = single_obj
Expand Down Expand Up @@ -165,7 +165,7 @@ def __init__(
"""
super().__init__(stream=stream)
if self.auth_headers is None:
self.auth_headers = {}
self.auth_headers = {} # type: ignore[unreachable]
if auth_headers:
self.auth_headers.update(auth_headers)

Expand Down Expand Up @@ -206,11 +206,11 @@ def __init__(

if location == "header":
if self.auth_headers is None:
self.auth_headers = {}
self.auth_headers = {} # type: ignore[unreachable]
self.auth_headers.update(auth_credentials)
elif location == "params":
if self.auth_params is None:
self.auth_params = {}
self.auth_params = {} # type: ignore[unreachable]
self.auth_params.update(auth_credentials)

@classmethod
Expand Down Expand Up @@ -255,7 +255,7 @@ def __init__(self, stream: RESTStream, token: str) -> None:
auth_credentials = {"Authorization": f"Bearer {token}"}

if self.auth_headers is None:
self.auth_headers = {}
self.auth_headers = {} # type: ignore[unreachable]
self.auth_headers.update(auth_credentials)

@classmethod
Expand Down Expand Up @@ -314,7 +314,7 @@ def __init__(
auth_credentials = {"Authorization": f"Basic {auth_token}"}

if self.auth_headers is None:
self.auth_headers = {}
self.auth_headers = {} # type: ignore[unreachable]
self.auth_headers.update(auth_credentials)

@classmethod
Expand Down
4 changes: 2 additions & 2 deletions singer_sdk/connectors/sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ def discover_catalog_entry(
# Detect key properties
possible_primary_keys: list[list[str]] = []
pk_def = inspected.get_pk_constraint(table_name, schema=schema_name)
if pk_def and "constrained_columns" in pk_def:
if pk_def and "constrained_columns" in pk_def: # type: ignore[redundant-expr]
possible_primary_keys.append(pk_def["constrained_columns"])

# An element of the columns list is ``None`` if it's an expression and is
Expand Down Expand Up @@ -953,7 +953,7 @@ def merge_sql_types(
# Get the generic type class
for opt in sql_types:
# Get the length
opt_len: int = getattr(opt, "length", 0)
opt_len: int | None = getattr(opt, "length", 0)
generic_type = type(opt.as_generic())

if isinstance(generic_type, type):
Expand Down
2 changes: 1 addition & 1 deletion singer_sdk/helpers/_batch.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ def __post_init__(self) -> None:
self.storage = StorageTarget.from_dict(self.storage)

if self.batch_size is None:
self.batch_size = DEFAULT_BATCH_SIZE
self.batch_size = DEFAULT_BATCH_SIZE # type: ignore[unreachable]

def asdict(self) -> dict[str, t.Any]:
"""Return a dictionary representation of the message.
Expand Down
4 changes: 2 additions & 2 deletions singer_sdk/helpers/_catalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def _pop_deselected_schema(
schema_at_breadcrumb = schema_at_breadcrumb.get(crumb, {})

if not isinstance(schema_at_breadcrumb, dict): # pragma: no cover
msg = (
msg = ( # type: ignore[unreachable]
"Expected dictionary type instead of "
f"'{type(schema_at_breadcrumb).__name__}' '{schema_at_breadcrumb}' for "
f"'{stream_name}' bookmark '{breadcrumb!s}' in '{schema}'"
Expand Down Expand Up @@ -123,7 +123,7 @@ def set_catalog_stream_selected(
"""
breadcrumb = breadcrumb or ()
if not isinstance(breadcrumb, tuple): # pragma: no cover
msg = (
msg = ( # type: ignore[unreachable]
f"Expected tuple value for breadcrumb '{breadcrumb}'. Got "
f"{type(breadcrumb).__name__}"
)
Expand Down
2 changes: 1 addition & 1 deletion singer_sdk/helpers/_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def get_writeable_state_dict(
ValueError: Raise an error if duplicate entries are found.
"""
if tap_state is None:
msg = "Cannot write state to missing state dictionary."
msg = "Cannot write state to missing state dictionary." # type: ignore[unreachable]
raise ValueError(msg)

if "bookmarks" not in tap_state:
Expand Down
10 changes: 4 additions & 6 deletions singer_sdk/mapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ def _eval_type(
ValueError: If the expression is ``None``.
"""
if expr is None:
msg = "Expression should be str, not None"
msg = "Expression should be str, not None" # type: ignore[unreachable]
raise ValueError(msg)

default = default or th.StringType()
Expand Down Expand Up @@ -564,7 +564,7 @@ def always_true(record: dict) -> bool:
elif filter_rule is None:
filter_fn = always_true
else:
msg = (
msg = ( # type: ignore[unreachable]
f"Unexpected filter rule type '{type(filter_rule).__name__}' in "
f"expression {filter_rule!s}. Expected 'str' or 'None'."
)
Expand Down Expand Up @@ -783,9 +783,7 @@ def register_raw_stream_schema( # noqa: PLR0912, C901
key_properties=key_properties,
flattening_options=self.flattening_options,
)
elif stream_def is None or (
isinstance(stream_def, str) and stream_def == NULL_STRING
):
elif stream_def is None or (stream_def == NULL_STRING):
mapper = RemoveRecordTransform(
stream_alias=stream_alias,
raw_schema=schema,
Expand All @@ -800,7 +798,7 @@ def register_raw_stream_schema( # noqa: PLR0912, C901
raise StreamMapConfigError(msg)

else:
msg = (
msg = ( # type: ignore[unreachable]
f"Unexpected stream definition type. Expected str, dict, or None. "
f"Got '{type(stream_def).__name__}'."
)
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 @@ -153,7 +153,7 @@ def __init__(
elif isinstance(config, dict):
config_dict = config
else:
msg = f"Error parsing config of type '{type(config).__name__}'."
msg = f"Error parsing config of type '{type(config).__name__}'." # type: ignore[unreachable]
raise ValueError(msg)
if parse_env_config:
self.logger.info("Parsing env var for settings config...")
Expand Down
4 changes: 2 additions & 2 deletions singer_sdk/streams/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ def __init__(
elif isinstance(schema, singer.Schema):
self._schema = schema.to_dict()
else:
msg = f"Unexpected type {type(schema).__name__} for arg 'schema'."
msg = f"Unexpected type {type(schema).__name__} for arg 'schema'." # type: ignore[unreachable]
raise ValueError(msg)

if self.schema_filepath:
Expand All @@ -187,7 +187,7 @@ def stream_maps(self) -> list[StreamMap]:
if self._stream_maps:
return self._stream_maps

if self._tap.mapper:
if self._tap.mapper: # type: ignore[truthy-bool]
self._stream_maps = self._tap.mapper.stream_maps[self.name]
self.logger.info(
"Tap has custom mapper. Using %d provided map(s).",
Expand Down
2 changes: 1 addition & 1 deletion singer_sdk/streams/graphql.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def prepare_request_payload(
query = self.query

if query is None:
msg = "Graphql `query` property not set."
msg = "Graphql `query` property not set." # type: ignore[unreachable]
raise ValueError(msg)

if not query.lstrip().startswith("query"):
Expand Down
4 changes: 2 additions & 2 deletions singer_sdk/tap_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ def state(self) -> dict:
RuntimeError: If state has not been initialized.
"""
if self._state is None:
msg = "Could not read from uninitialized state."
msg = "Could not read from uninitialized state." # type: ignore[unreachable]
raise RuntimeError(msg)
return self._state

Expand Down Expand Up @@ -398,7 +398,7 @@ def load_state(self, state: dict[str, t.Any]) -> None:
initialized.
"""
if self.state is None:
msg = "Cannot write to uninitialized state dictionary."
msg = "Cannot write to uninitialized state dictionary." # type: ignore[unreachable]
raise ValueError(msg)

for stream_name, stream_state in state.get("bookmarks", {}).items():
Expand Down
2 changes: 1 addition & 1 deletion singer_sdk/testing/legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def _test_discovery() -> None:
catalog1 = _get_tap_catalog(tap_class, config or {})
# Reset and re-initialize with an input catalog
tap2: Tap = tap_class(config=config, parse_env_config=True, catalog=catalog1)
assert tap2
assert tap2 # type: ignore[truthy-bool]

def _test_stream_connections() -> None:
# Initialize with basic config
Expand Down
4 changes: 2 additions & 2 deletions singer_sdk/testing/tap_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def test(self) -> None:
catalog=catalog,
**kwargs,
)
assert tap2
assert tap2 # type: ignore[truthy-bool]


class TapStreamConnectionTest(TapTestTemplate):
Expand Down Expand Up @@ -218,7 +218,7 @@ def test(self) -> None:
try:
for v in self.non_null_attribute_values:
error_message = f"Unable to parse value ('{v}') with datetime parser."
assert datetime_fromisoformat(v), error_message
assert datetime_fromisoformat(v), error_message # type: ignore[truthy-bool]
except ValueError as e:
raise AssertionError(error_message) from e

Expand Down
9 changes: 3 additions & 6 deletions singer_sdk/typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ def __get__(self, instance: P, owner: type[P]) -> t.Any: # noqa: ANN401
The property value.
"""
if instance is None:
instance = owner()
instance = owner() # type: ignore[unreachable]
return self.fget(instance)


Expand Down Expand Up @@ -1123,13 +1123,10 @@ def to_jsonschema_type(
type_name = from_type
elif isinstance(from_type, sa.types.TypeEngine):
type_name = type(from_type).__name__
elif isinstance(from_type, type) and issubclass(
from_type,
sa.types.TypeEngine,
):
elif issubclass(from_type, sa.types.TypeEngine):
type_name = from_type.__name__
else: # pragma: no cover
msg = "Expected `str` or a SQLAlchemy `TypeEngine` object or type."
msg = "Expected `str` or a SQLAlchemy `TypeEngine` object or type." # type: ignore[unreachable]
# TODO: this should be a TypeError, but it's a breaking change.
raise ValueError(msg) # noqa: TRY004

Expand Down

0 comments on commit 63cc0a3

Please sign in to comment.