Skip to content

Commit

Permalink
[internal] Avoid duplicated deprecation warnings by memoization. (Che…
Browse files Browse the repository at this point in the history
…rry-pick of #14511) (#14553)

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
Eric-Arellano authored Feb 22, 2022
1 parent 699ef2a commit 3a69f2f
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 34 deletions.
3 changes: 2 additions & 1 deletion src/python/pants/base/deprecated.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

from packaging.version import InvalidVersion, Version

from pants.util.memo import memoized_method
from pants.util.memo import memoized, memoized_method
from pants.version import PANTS_SEMVER

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -103,6 +103,7 @@ def validate_deprecation_semver(version_string: str, version_description: str) -
return v


@memoized
def warn_or_error(
removal_version: str,
entity: str,
Expand Down
20 changes: 20 additions & 0 deletions src/python/pants/base/deprecated_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,26 @@ def test_deprecation_start_period(caplog) -> None:
assert "DEPRECATED: demo will be removed in version 999.999.999.dev999." in caplog.text


@unittest.mock.patch("pants.base.deprecated.PANTS_SEMVER", Version(_FAKE_CUR_VERSION))
def test_deprecation_memoization(caplog) -> None:
caplog.clear()
for i in range(3):
warn_or_error(
removal_version="999.999.999.dev999",
entity="memo",
hint=None,
start_version=_FAKE_CUR_VERSION,
)
assert len(caplog.records) == 1
warn_or_error(
removal_version="999.999.999.dev999",
entity="another",
hint=None,
start_version=_FAKE_CUR_VERSION,
)
assert len(caplog.records) == 2


def test_resolve_conflicting_options() -> None:
old_val = "ancient"
new_val = "modern"
Expand Down
36 changes: 4 additions & 32 deletions src/python/pants/engine/internals/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,20 +116,7 @@ def target_types_to_generate_targets_requests(
)


# We use a rule for this warning so that it gets memoized, i.e. doesn't get repeated for every
# offending target.
class _WarnDeprecatedTarget:
pass


@dataclass(frozen=True)
class _WarnDeprecatedTargetRequest:
tgt_type: type[Target]


@rule
def warn_deprecated_target_type(request: _WarnDeprecatedTargetRequest) -> _WarnDeprecatedTarget:
tgt_type = request.tgt_type
def warn_deprecated_target_type(tgt_type: type[Target]) -> None:
assert tgt_type.deprecated_alias_removal_version is not None
warn_or_error(
removal_version=tgt_type.deprecated_alias_removal_version,
Expand All @@ -139,23 +126,9 @@ def warn_deprecated_target_type(request: _WarnDeprecatedTargetRequest) -> _WarnD
"update-build-files` to automatically fix your BUILD files."
),
)
return _WarnDeprecatedTarget()


# We use a rule for this warning so that it gets memoized, i.e. doesn't get repeated for every
# offending field.
class _WarnDeprecatedField:
pass


@dataclass(frozen=True)
class _WarnDeprecatedFieldRequest:
field_type: type[Field]


@rule
def warn_deprecated_field_type(request: _WarnDeprecatedFieldRequest) -> _WarnDeprecatedField:
field_type = request.field_type
def warn_deprecated_field_type(field_type: type[Field]) -> None:
assert field_type.deprecated_alias_removal_version is not None
warn_or_error(
removal_version=field_type.deprecated_alias_removal_version,
Expand All @@ -165,7 +138,6 @@ def warn_deprecated_field_type(request: _WarnDeprecatedFieldRequest) -> _WarnDep
"update-build-files` to automatically fix your BUILD files."
),
)
return _WarnDeprecatedField()


@rule
Expand All @@ -187,14 +159,14 @@ async def resolve_target(
and target_type.deprecated_alias == target_adaptor.type_alias
and not address.is_generated_target
):
await Get(_WarnDeprecatedTarget, _WarnDeprecatedTargetRequest(target_type))
warn_deprecated_target_type(target_type)
target = target_type(target_adaptor.kwargs, address, union_membership)
for field_type in target.field_types:
if (
field_type.deprecated_alias is not None
and field_type.deprecated_alias in target_adaptor.kwargs
):
await Get(_WarnDeprecatedField, _WarnDeprecatedFieldRequest(field_type))
warn_deprecated_field_type(field_type)
return WrappedTarget(target)

wrapped_generator_tgt = await Get(
Expand Down
4 changes: 3 additions & 1 deletion src/python/pants/option/options_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import yaml
from packaging.version import Version

from pants.base.deprecated import CodeRemovedError
from pants.base.deprecated import CodeRemovedError, warn_or_error
from pants.base.hash_utils import CoercingEncoder
from pants.engine.fs import FileContent
from pants.option.config import Config
Expand Down Expand Up @@ -346,6 +346,7 @@ def assert_deprecated(
config: dict[str, dict[str, str]] | None = None,
) -> None:
caplog.clear()
warn_or_error.clear() # type: ignore[attr-defined]
opts = create_options([GLOBAL_SCOPE, "scope"], register, args, env=env, config=config)
assert opts.for_scope(scope)[opt] == expected
assert len(caplog.records) == 1
Expand All @@ -370,6 +371,7 @@ def assert_deprecated(

# Make sure the warnings don't come out for regular options.
caplog.clear()
warn_or_error.clear() # type: ignore[attr-defined]
assert (
create_options([GLOBAL_SCOPE, "scope"], register, ["--scope-valid=x"])
.for_scope("scope")
Expand Down

0 comments on commit 3a69f2f

Please sign in to comment.