From 83eea40b5db61c96203a88ba6a59cc12d124e6ed Mon Sep 17 00:00:00 2001 From: Deepyaman Datta Date: Mon, 31 Aug 2026 22:25:55 -0600 Subject: [PATCH] [FLINK-40529][python] Warn on use of deprecated APIs, not at import time Deprecated emitted its DeprecationWarning from __call__, which the decorator syntax invokes in order to apply the decorator. The warning therefore fired at decoration time -- that is, at import -- for every deprecated API a module defines, whether or not the user touches it, while actually calling one emitted nothing, and stacklevel=2 pointed at the decoration site inside PyFlink's own source rather than at user code. Functions now get a functools.wraps wrapper that warns when called. Classes are returned unchanged, with __init__ wrapped in place so that isinstance checks and subclassing keep working; as in PEP 702, only instantiating the deprecated class itself warns, which also avoids warning twice when a deprecated class inherits the __init__ of a deprecated base class. staticmethod and classmethod objects are unwrapped and re-packaged, and properties, ABCs and Enum subclasses degrade to the docstring directive rather than raising -- decorating any of these used to fail. A missing detail argument no longer raises either. The message format, the DeprecationWarning category, the docstring directives and the __stability_decorators attribute read by PythonAPICompletenessTestCase are unchanged, and Experimental, Internal, Public and PublicEvolving are unaffected. pyflink/util was not in the list of modules that dev/integration_test.sh runs, so it is added there for the new tests to run in CI. Generated-by: Claude Code 2.1.252 (Claude Opus 5) --- flink-python/dev/integration_test.sh | 3 + .../pyflink/util/api_stability_decorators.py | 107 ++++- flink-python/pyflink/util/tests/__init__.py | 17 + .../tests/test_api_stability_decorators.py | 410 ++++++++++++++++++ 4 files changed, 526 insertions(+), 11 deletions(-) create mode 100644 flink-python/pyflink/util/tests/__init__.py create mode 100644 flink-python/pyflink/util/tests/test_api_stability_decorators.py diff --git a/flink-python/dev/integration_test.sh b/flink-python/dev/integration_test.sh index ba7ceb47aea0b4..7bd9f34340cfa8 100755 --- a/flink-python/dev/integration_test.sh +++ b/flink-python/dev/integration_test.sh @@ -42,6 +42,9 @@ function test_all_modules() { # test table module test_module "table" + + # test util module + test_module "util" } # CURRENT_DIR is "flink/flink-python/dev/" diff --git a/flink-python/pyflink/util/api_stability_decorators.py b/flink-python/pyflink/util/api_stability_decorators.py index abfa508cd4a9d9..5fc25f3a098ebf 100644 --- a/flink-python/pyflink/util/api_stability_decorators.py +++ b/flink-python/pyflink/util/api_stability_decorators.py @@ -16,8 +16,9 @@ # limitations under the License. ################################################################################ +import functools from inspect import getmembers, isfunction, isclass -from typing import TypeVar, Callable, Any, Union, Type, Optional +from typing import TypeVar, Callable, Any, Union, Type, Optional, cast from abc import ABCMeta, abstractmethod import warnings from typing_extensions import override @@ -76,7 +77,10 @@ def __call__(self, func_or_cls: T) -> T: # Avoid duplicating directives if already present in the docstring. if directive not in docstring: - func_or_cls.__doc__ = f"{docstring}\n{directive}" + try: + func_or_cls.__doc__ = f"{docstring}\n{directive}" + except (AttributeError, TypeError): + pass # Add the decorator to an internal __stability_decorators set on the class/function # being decorated, for later introspection. @@ -84,7 +88,13 @@ def __call__(self, func_or_cls: T) -> T: stability_decorators = getattr(func_or_cls, '__stability_decorators') stability_decorators.add(self.__class__) else: - setattr(func_or_cls, '__stability_decorators', {self.__class__}) + # Not every decorated object accepts attribute assignment (a property, for + # example). Those simply cannot be introspected; that is not a reason to fail + # at import time. + try: + setattr(func_or_cls, '__stability_decorators', {self.__class__}) + except (AttributeError, TypeError): + pass if isclass(func_or_cls): for name, method in getmembers( @@ -126,20 +136,95 @@ def __init__(self, since: str, detail: Optional[str] = None): self.detail = detail def get_directive(self, func_or_cls: T) -> str: - return f".. deprecated:: {self.since}\n{indent(dedent(self.detail), ' ')}" + directive = f".. deprecated:: {self.since}" + if self.detail is not None: + directive = f"{directive}\n{indent(dedent(self.detail), ' ')}" + return directive - @override - def __call__(self, func_or_cls: T) -> T: + def _get_message(self, func_or_cls: T) -> str: """ - Emit a warning on the deprecation of the given function/class. Then call the base class - for docstring modification. + Returns the warning message emitted when the deprecated API element is used. """ - msg = f"{func_or_cls.__qualname__} has been deprecated since version {self.since}." + name = getattr(func_or_cls, "__qualname__", None) or getattr( + func_or_cls, "__name__", "This API" + ) + msg = f"{name} has been deprecated since version {self.since}." if self.detail is not None: msg = f"{msg} {self.detail}" + return msg + + @override + def __call__(self, func_or_cls: T) -> T: + """ + Arranges for a :class:`DeprecationWarning` to be emitted when the decorated API element + is *used*, and calls the base class for docstring modification. + + The warning must not be emitted here: this method runs while the module defining the + API is being imported, so warning here would warn every user that imports PyFlink, + whether or not they use the deprecated API, and would never warn the ones who do. + """ + # staticmethod/classmethod objects are not functions and do not proxy __qualname__ on + # all supported Python versions. Decorate the function they wrap instead, and + # re-package it so that the descriptor still behaves as one. + if isinstance(func_or_cls, (staticmethod, classmethod)): + return cast(T, type(func_or_cls)(self(func_or_cls.__func__))) + + func_or_cls = super().__call__(func_or_cls) - warnings.warn(msg, category=DeprecationWarning, stacklevel=2) - return super().__call__(func_or_cls) + if isclass(func_or_cls): + self._deprecate_class(func_or_cls) + elif isfunction(func_or_cls): + return cast(T, self._deprecate_function(func_or_cls)) + # Anything else (a property, for instance) cannot be wrapped without changing what the + # decorated name refers to, so the docstring directive is all we apply. + return func_or_cls + + def _deprecate_function(self, func: Callable[..., Any]) -> Callable[..., Any]: + """ + Returns a wrapper around the given function that warns before delegating to it. + """ + msg = self._get_message(func) + + @functools.wraps(func) + def wrapper(*args: Any, **kwargs: Any) -> Any: + # stacklevel=2 attributes the warning to the caller of the deprecated function + # rather than to this wrapper. + warnings.warn(msg, category=DeprecationWarning, stacklevel=2) + return func(*args, **kwargs) + + return wrapper + + def _deprecate_class(self, cls: Type[Any]) -> None: + """ + Wraps the __init__ of the given class so that instantiating it warns. + + The class itself is returned unchanged by :func:`__call__`; replacing it with a wrapper + would break isinstance checks and subclassing. + """ + msg = self._get_message(cls) + original_init = cls.__init__ + + @functools.wraps(original_init) + def __init__(self: Any, *args: Any, **kwargs: Any) -> None: + # As in PEP 702, only instantiating the deprecated class itself warns. A subclass + # is not necessarily deprecated, and this also avoids warning twice when a + # deprecated class inherits the __init__ of a deprecated base class. + if type(self) is cls: + warnings.warn(msg, category=DeprecationWarning, stacklevel=2) + if original_init is object.__init__ and (args or kwargs) \ + and type(self).__new__ is object.__new__: + # object.__new__ rejects excess arguments only for classes that define + # neither __new__ nor __init__; installing this __init__ would otherwise + # silence that error. + raise TypeError(f"{type(self).__name__}() takes no arguments") + original_init(self, *args, **kwargs) + + try: + cls.__init__ = __init__ # type: ignore[misc] + except (AttributeError, TypeError): + # Extension types and the like do not allow their __init__ to be replaced; fall + # back to documenting the deprecation only. + pass class Experimental(BaseAPIStabilityDecorator): diff --git a/flink-python/pyflink/util/tests/__init__.py b/flink-python/pyflink/util/tests/__init__.py new file mode 100644 index 00000000000000..65b48d4d79b4e3 --- /dev/null +++ b/flink-python/pyflink/util/tests/__init__.py @@ -0,0 +1,17 @@ +################################################################################ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +################################################################################ diff --git a/flink-python/pyflink/util/tests/test_api_stability_decorators.py b/flink-python/pyflink/util/tests/test_api_stability_decorators.py new file mode 100644 index 00000000000000..b6b7770982527e --- /dev/null +++ b/flink-python/pyflink/util/tests/test_api_stability_decorators.py @@ -0,0 +1,410 @@ +################################################################################ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +################################################################################ +import abc +import enum +import inspect +import os +import subprocess +import sys +import unittest +import warnings + +from pyflink.util.api_stability_decorators import ( + Deprecated, + Experimental, + Internal, + Public, + PublicEvolving, +) + + +def _catch_warnings(): + """ + Returns a context manager recording every warning raised within it. + """ + context = warnings.catch_warnings(record=True) + warnings.simplefilter("always") + return context + + +class DeprecatedTests(unittest.TestCase): + """ + Tests for the :class:`Deprecated` decorator, which must warn when a deprecated API is + used, and not when it is defined. + """ + + def test_decoration_does_not_warn(self): + with _catch_warnings() as caught: + + @Deprecated(since="1.0.0", detail="Use :func:`new_func` instead.") + def func(): + pass + + @Deprecated(since="1.0.0") + class Cls(object): + def __init__(self): + pass + + self.assertEqual([], [str(warning.message) for warning in caught]) + + def test_importing_pyflink_table_does_not_warn(self): + # A regression test for the decorators warning at import time: every deprecated API of + # pyflink.table used to warn as soon as the package was imported. This needs a fresh + # interpreter, as pyflink.table is already imported in the one running the tests. + script = ( + "import warnings\n" + "with warnings.catch_warnings(record=True) as caught:\n" + " warnings.simplefilter('always')\n" + " import pyflink.table\n" + "print([str(warning.message) for warning in caught\n" + " if 'has been deprecated since version' in str(warning.message)])\n" + ) + result = subprocess.run( + [sys.executable, "-c", script], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + + self.assertEqual(0, result.returncode, result.stderr.decode("utf-8")) + self.assertEqual("[]", result.stdout.decode("utf-8").strip()) + + def test_function_warns_when_called(self): + @Deprecated(since="1.0.0", detail="Use :func:`new_func` instead.") + def func(a, b=2): + return a + b + + with _catch_warnings() as caught: + self.assertEqual(3, func(1)) + + self.assertEqual(1, len(caught)) + self.assertIs(DeprecationWarning, caught[0].category) + self.assertEqual( + f"{func.__qualname__} has been deprecated since version 1.0.0. " + f"Use :func:`new_func` instead.", + str(caught[0].message), + ) + + def test_function_without_detail_warns_when_called(self): + @Deprecated(since="1.0.0") + def func(): + pass + + with _catch_warnings() as caught: + func() + + self.assertEqual(1, len(caught)) + self.assertEqual( + f"{func.__qualname__} has been deprecated since version 1.0.0.", + str(caught[0].message), + ) + + def test_function_wrapper_preserves_metadata(self): + @Deprecated(since="1.0.0") + def func(): + """Some documentation.""" + + self.assertEqual("func", func.__name__) + self.assertEqual( + "DeprecatedTests.test_function_wrapper_preserves_metadata..func", + func.__qualname__, + ) + self.assertIn("Some documentation.", func.__doc__) + + def test_class_warns_when_instantiated(self): + @Deprecated(since="1.0.0", detail="Use :class:`NewClass` instead.") + class Cls(object): + def __init__(self, x): + self.x = x + + with _catch_warnings() as caught: + instance = Cls(1) + + self.assertEqual(1, instance.x) + self.assertEqual(1, len(caught)) + self.assertIs(DeprecationWarning, caught[0].category) + self.assertEqual( + f"{Cls.__qualname__} has been deprecated since version 1.0.0. " + f"Use :class:`NewClass` instead.", + str(caught[0].message), + ) + + def test_class_without_own_init_warns_when_instantiated(self): + @Deprecated(since="1.0.0") + class Cls(object): + pass + + with _catch_warnings() as caught: + Cls() + + self.assertEqual(1, len(caught)) + self.assertEqual( + f"{Cls.__qualname__} has been deprecated since version 1.0.0.", + str(caught[0].message), + ) + + # The wrapper must not turn an unexpected argument into a silent no-op. + with _catch_warnings(): + with self.assertRaises(TypeError): + Cls(1) + + def test_class_is_returned_unchanged(self): + @Deprecated(since="1.0.0") + class Cls(object): + def __init__(self): + self.x = 1 + + class Subclass(Cls): + pass + + with _catch_warnings(): + instance = Subclass() + + self.assertIsInstance(instance, Cls) + self.assertTrue(issubclass(Subclass, Cls)) + self.assertEqual("Cls", Cls.__name__) + + def test_subclass_of_deprecated_class_does_not_warn(self): + # Deprecating a class says nothing about its subclasses, which are the ones users are + # typically pointed at. This mirrors PEP 702. + @Deprecated(since="1.0.0") + class Cls(object): + def __init__(self): + pass + + class Subclass(Cls): + pass + + with _catch_warnings() as caught: + Subclass() + + self.assertEqual([], [str(warning.message) for warning in caught]) + + def test_deprecated_subclass_inheriting_init_warns_once(self): + @Deprecated(since="1.0.0") + class Cls(object): + def __init__(self): + pass + + @Deprecated(since="2.0.0") + class Subclass(Cls): + pass + + with _catch_warnings() as caught: + Subclass() + + self.assertEqual( + [f"{Subclass.__qualname__} has been deprecated since version 2.0.0."], + [str(warning.message) for warning in caught], + ) + + def test_function_warning_points_at_the_caller(self): + @Deprecated(since="1.0.0") + def func(): + pass + + with _catch_warnings() as caught: + lineno = inspect.currentframe().f_lineno + 1 + func() + + self.assertEqual(1, len(caught)) + self.assertEqual(os.path.abspath(__file__), os.path.abspath(caught[0].filename)) + self.assertEqual(lineno, caught[0].lineno) + + def test_class_warning_points_at_the_caller(self): + @Deprecated(since="1.0.0") + class Cls(object): + def __init__(self): + pass + + with _catch_warnings() as caught: + lineno = inspect.currentframe().f_lineno + 1 + Cls() + + self.assertEqual(1, len(caught)) + self.assertEqual(os.path.abspath(__file__), os.path.abspath(caught[0].filename)) + self.assertEqual(lineno, caught[0].lineno) + + def test_docstring_directives_are_still_applied(self): + @Deprecated(since="1.0.0", detail="Use :func:`new_func` instead.") + def func(): + """Function documentation.""" + + @Deprecated(since="1.0.0") + class Cls(object): + """Class documentation.""" + + def method(self): + """Method documentation.""" + + self.assertEqual( + "Function documentation.\n.. deprecated:: 1.0.0\n Use :func:`new_func` instead.", + func.__doc__, + ) + self.assertEqual("Class documentation.\n.. deprecated:: 1.0.0", Cls.__doc__) + self.assertEqual("Method documentation.\n.. deprecated:: 1.0.0", Cls.method.__doc__) + + def test_stability_decorators_attribute_is_still_populated(self): + @Deprecated(since="1.0.0") + def func(): + pass + + @Deprecated(since="1.0.0") + @PublicEvolving() + class Cls(object): + def __init__(self): + pass + + self.assertEqual({Deprecated}, getattr(func, "__stability_decorators")) + self.assertEqual({Deprecated, PublicEvolving}, getattr(Cls, "__stability_decorators")) + + def test_static_and_class_methods(self): + with _catch_warnings() as caught_at_decoration: + + class Cls(object): + @Deprecated(since="1.0.0") + @staticmethod + def static_method(x): + """Static method documentation.""" + return x + + @Deprecated(since="1.0.0") + @classmethod + def class_method(cls, x): + return x + + self.assertEqual([], [str(warning.message) for warning in caught_at_decoration]) + self.assertIsInstance(Cls.__dict__["static_method"], staticmethod) + self.assertIsInstance(Cls.__dict__["class_method"], classmethod) + self.assertIn(".. deprecated:: 1.0.0", Cls.static_method.__doc__) + + with _catch_warnings() as caught: + self.assertEqual(1, Cls.static_method(1)) + self.assertEqual(2, Cls.class_method(2)) + + self.assertEqual( + [ + f"{Cls.__qualname__}.static_method has been deprecated since version 1.0.0.", + f"{Cls.__qualname__}.class_method has been deprecated since version 1.0.0.", + ], + [str(warning.message) for warning in caught], + ) + + def test_property(self): + # A property cannot be wrapped without replacing the descriptor, so it only gets the + # docstring directive. It must not raise. + with _catch_warnings() as caught: + + class Cls(object): + @Deprecated(since="1.0.0") + @property + def value(self): + """Property documentation.""" + return 1 + + self.assertEqual(1, Cls().value) + + self.assertEqual([], [str(warning.message) for warning in caught]) + self.assertIn(".. deprecated:: 1.0.0", Cls.__dict__["value"].__doc__) + + def test_abstract_class(self): + with _catch_warnings() as caught_at_decoration: + + @Deprecated(since="1.0.0") + class Abstract(abc.ABC): + """Abstract class documentation.""" + + @abc.abstractmethod + def method(self): + pass + + self.assertEqual([], [str(warning.message) for warning in caught_at_decoration]) + self.assertIn(".. deprecated:: 1.0.0", Abstract.__doc__) + + with self.assertRaises(TypeError): + Abstract() + + def test_enum_class(self): + with _catch_warnings() as caught: + + @Deprecated(since="1.0.0") + class Colour(enum.Enum): + """Enum documentation.""" + + RED = 1 + + self.assertIs(Colour.RED, Colour(1)) + self.assertEqual(1, Colour.RED.value) + + self.assertEqual([], [str(warning.message) for warning in caught]) + self.assertIn(".. deprecated:: 1.0.0", Colour.__doc__) + + +class OtherStabilityDecoratorTests(unittest.TestCase): + """ + Tests that the decorators other than :class:`Deprecated` document without warning. + """ + + def test_decorators_never_warn(self): + for decorator in (Experimental, Internal, Public, PublicEvolving): + with self.subTest(decorator=decorator.__name__): + with _catch_warnings() as caught: + + @decorator() + def func(): + """Function documentation.""" + + @decorator() + class Cls(object): + """Class documentation.""" + + def method(self): + """Method documentation.""" + + func() + Cls().method() + + self.assertEqual([], [str(warning.message) for warning in caught]) + + def test_decorated_elements_are_returned_unchanged(self): + for decorator in (Experimental, Internal, Public, PublicEvolving): + with self.subTest(decorator=decorator.__name__): + + def func(): + pass + + class Cls(object): + pass + + self.assertIs(func, decorator()(func)) + self.assertIs(Cls, decorator()(Cls)) + + def test_docstring_directives_and_attribute(self): + @Public() + class Cls(object): + """Class documentation.""" + + def method(self): + """Method documentation.""" + + self.assertIn("is marked as **public**", Cls.__doc__) + self.assertIn("is marked as **public**", Cls.method.__doc__) + self.assertEqual({Public}, getattr(Cls, "__stability_decorators")) + + +if __name__ == "__main__": + unittest.main()