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

Force to use Airflow Deprecation warnings categories on @deprecated decorator #39205

Merged
merged 2 commits into from
Apr 25, 2024

Conversation

Taragolis
Copy link
Contributor

By default deprecations decorators use DeprecationWarning which might be not what we want to use in core and providers.
This one is a bit more extended version of:

  • check-no-airflow-deprecation-in-providers
  • check-core-deprecation-classes

However it work only with decorators, from Deprecation or PEP-702


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@Taragolis Taragolis added the full tests needed We need to run full set of tests for this PR to merge label Apr 23, 2024
@boring-cyborg boring-cyborg bot added area:API Airflow's REST/HTTP API area:dev-tools area:webserver Webserver related Issues labels Apr 23, 2024
@Taragolis Taragolis added the type:misc/internal Changelog: Misc changes that should appear in change log label Apr 23, 2024
@Taragolis Taragolis added this to the Airflow 2.9.1 milestone Apr 23, 2024
@Taragolis Taragolis changed the title Force to use Airflow Deprecation warnings categories on @deprecated decorator Force to use Airflow Deprecation warnings categories on @deprecated decorator Apr 23, 2024
@potiuk
Copy link
Member

potiuk commented Apr 23, 2024

Pityy we do not have a way to specify deprecated_since - that was the reason why the warnings were originally introduced. But it's a good thing to keep the way we raise warnings consistent at least for decorators.

@Taragolis
Copy link
Contributor Author

Personally I more for the @deprecated from the warning/typing_extension rather than Deprecation module even if it not supports deprecated_since/version by design but it should supported more widely by third-parties tools/IDEs, and also it become in some (most) cases runtime checkable.

In general warnings.warn supports provide Warning object instead of message + category:

import warnings
from airflow.exceptions import AirflowProviderDeprecationWarning

wrn = AirflowProviderDeprecationWarning("FooBar")
wrn.deprecated_provider_since = "42.0.1"
warnings.warn(wrn)

But this one not supported by the decorators because it is expected to have message a string literal and category as a class, and also there is no point to set deprecated_provider_since because it doesn't have any affect.

@Taragolis Taragolis merged commit c2ef1da into apache:main Apr 25, 2024
70 checks passed
@Taragolis Taragolis deleted the deprecation-control branch April 25, 2024 13:44
jedcunningham pushed a commit that referenced this pull request Apr 26, 2024
… decorator (#39205)

* Force to use Airflow Deprecation warnings categories on @deprecated decorator

* Catch warning in Experimental API test

(cherry picked from commit c2ef1da)
RodrigoGanancia pushed a commit to RodrigoGanancia/airflow that referenced this pull request May 10, 2024
… decorator (apache#39205)

* Force to use Airflow Deprecation warnings categories on @deprecated decorator

* Catch warning in Experimental API test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:API Airflow's REST/HTTP API area:dev-tools area:webserver Webserver related Issues full tests needed We need to run full set of tests for this PR to merge type:misc/internal Changelog: Misc changes that should appear in change log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants