Skip to content

Commit

Permalink
refactor: Deprecate row parameter of Stream.post_process in favor…
Browse files Browse the repository at this point in the history
… or `record`

Take 2 for #966
  • Loading branch information
edgarrmondragon committed Jul 18, 2024
1 parent fe5a01b commit c8f38fc
Show file tree
Hide file tree
Showing 6 changed files with 112 additions and 5 deletions.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ repos:
- id: check-readthedocs

- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.5.2
rev: v0.5.3
hooks:
- id: ruff
args: [--fix, --exit-non-zero-on-fix, --show-fixes]
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,7 @@ unfixable = [
"SLF001",
"PLC2701", # Allow usage of private members in tests
"PLR6301", # Don't suggest making test methods static, etc.
"INP001", # flake8-no-pep420: implicit-namespace-package
]
# Disabled some checks in samples code
"samples/*" = ["ANN", "D"]
Expand Down
21 changes: 21 additions & 0 deletions singer_sdk/helpers/_deprecation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
from __future__ import annotations

import functools
import typing as t
import warnings


def deprecate_row_param(func: t.Callable) -> t.Callable:
@functools.wraps(func)
def wrapper(*args: t.Any, **kwargs: t.Any) -> t.Any: # noqa: ANN401
if "row" in kwargs:
warnings.warn(
f"The 'row' parameter for '{func.__qualname__}' is deprecated. "
"Use 'record' instead.",
category=DeprecationWarning,
stacklevel=2,
)
kwargs["record"] = kwargs.pop("row")
return func(*args, **kwargs)

return wrapper
21 changes: 18 additions & 3 deletions singer_sdk/streams/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import datetime
import json
import typing as t
from contextlib import contextmanager
from os import PathLike
from pathlib import Path
from types import MappingProxyType
Expand All @@ -28,6 +29,7 @@
)
from singer_sdk.helpers._catalog import pop_deselected_record_properties
from singer_sdk.helpers._compat import datetime_fromisoformat
from singer_sdk.helpers._deprecation import deprecate_row_param
from singer_sdk.helpers._flattening import get_flattening_options
from singer_sdk.helpers._state import (
finalize_state_progress_markers,
Expand Down Expand Up @@ -1155,6 +1157,15 @@ def _sync_batches(
self._write_batch_message(encoding=encoding, manifest=manifest)
self._write_state_message()

@contextmanager
def _with_context(
self,
context: types.Context | None,
) -> t.Generator[None, None, None]:
self.context = MappingProxyType(context) if context else None
yield
self.context = None

# Public methods ("final", not recommended to be overridden)

@t.final
Expand Down Expand Up @@ -1387,9 +1398,10 @@ def get_batches(
for manifest in batcher.get_batches(records=records):
yield batch_config.encoding, manifest

@deprecate_row_param
def post_process( # noqa: PLR6301
self,
row: types.Record,
record: types.Record,
context: types.Context | None = None, # noqa: ARG002
) -> dict | None:
"""As needed, append or transform raw data to match expected structure.
Expand All @@ -1403,10 +1415,13 @@ def post_process( # noqa: PLR6301
invalid or not-applicable records from the stream.
Args:
row: Individual record in the stream.
record: Individual record in the stream.
context: Stream partition or context dictionary.
Returns:
The resulting record dict, or `None` if the record should be excluded.
.. versionchanged:: 0.39.0
The ``row`` parameter was renamed to ``record`` with a deprecation warning.
"""
return row
return record
2 changes: 1 addition & 1 deletion singer_sdk/typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,7 @@ class Property(JSONTypeHelper[T], t.Generic[T]):
"""Generic Property. Should be nested within a `PropertiesList`."""

# TODO: Make some of these arguments keyword-only. This is a breaking change.
def __init__( # noqa: PLR0913
def __init__(
self,
name: str,
wrapped: JSONTypeHelper[T] | type[JSONTypeHelper[T]],
Expand Down
70 changes: 70 additions & 0 deletions tests/core/helpers/test_deprecation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
from __future__ import annotations

import warnings

from singer_sdk.helpers._deprecation import deprecate_row_param


class Deprecated:
@deprecate_row_param
def check_row(self, record):
pass


class StaleSubclass(Deprecated):
def check_row(self, row):
return super().check_row(row=row)


class OkSubclass(Deprecated):
def check_row(self, row):
return super().check_row(row)


class NewSubclass(Deprecated):
def check_row(self, record):
return super().check_row(record)


def test_deprecated_row_parameter():
d = Deprecated()

# No warning should be raised
with warnings.catch_warnings():
warnings.simplefilter("error")
d.check_row("test")

# No warning should be raised
with warnings.catch_warnings():
warnings.simplefilter("error")
d.check_row(record="test")

# Warning should be raised
_assert_deprecation_warning(d)

s = StaleSubclass()
_assert_deprecation_warning(s)

o = OkSubclass()
# No warning should be raised
with warnings.catch_warnings():
warnings.simplefilter("error")
o.check_row(row="test")

n = NewSubclass()
# No warning should be raised
with warnings.catch_warnings():
warnings.simplefilter("error")
n.check_row(record="test")


def _assert_deprecation_warning(instance: Deprecated):
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter("always")
instance.check_row(row="test")
assert len(w) == 1
assert issubclass(w[-1].category, DeprecationWarning)
assert str(w[-1].message) == (
"The 'row' parameter for 'Deprecated.check_row' is deprecated. "
"Use 'record' instead."
)

0 comments on commit c8f38fc

Please sign in to comment.