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

[Core] Add deprecation module with decorator and custom warnings #36952

Closed

Conversation

kacpermuda
Copy link
Contributor

@kacpermuda kacpermuda commented Jan 22, 2024

Rationale

This is a step number 2 of the process described in #36876, please refer to it for the rationale and in depth description. Possibly, it can also replace part of the mechanism described in #31830.

TLDR

See the tests i wrote, for the demo of how it works (look at the end of the file).

I want to add a decorator (for now only for classes, functions, methods etc.) that we can use for deprecations in the entire codebase (core and providers). By defining a custom function, we can have the most common information (now included in deprecation message) as separate arguments, so it's easier to parse it and use in CI, docs etc.

To do in next PRs

  • Possibly some changes in the documentation will be required, to describe how the developers should deprecate things and what are the options.
  • We can probably remove the warnings from current location (airflow.exceptions.py) and only keep them in airflow.deprecation.py, but this requires some time to migrate etc. so I am not sure how do properly do it.

Questions

This is my first PR that touches the core airflow, so let me know if anything needs to be changed 😄 I also have some questions to the functionality itself:

  1. Are there any more arguments that we can define in the deprecation decorator? Some ideas?
  2. Is it a good place (deprecation module in core) to have helper functions like: get_callable_full_path or validate_release_string. We can just make them private.
  3. Should we define separate provider_deprecated decorator that will have a default category set to AirflowProviderDeprecationWarning ?
  4. Should we allow passing a different Adapter, so that You can alter the deprecation message or we want to enforce standardization in the whole codebase and remove that option for the end user?

^ 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.

@kacpermuda kacpermuda marked this pull request as draft January 22, 2024 16:21
@kacpermuda kacpermuda force-pushed the feat/add-deprecation-module branch 3 times, most recently from 69d6239 to 76295b7 Compare January 25, 2024 08:54
@kacpermuda kacpermuda marked this pull request as ready for review January 25, 2024 08:56
@mobuchowski mobuchowski added kind:documentation type:improvement Changelog: Improvements kind:feature Feature Requests labels Jan 25, 2024
Copy link

github-actions bot commented Apr 1, 2024

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Apr 1, 2024
@kacpermuda
Copy link
Contributor Author

Will reopen when i have time to do this.

@kacpermuda kacpermuda closed this Apr 2, 2024
@Taragolis
Copy link
Contributor

If you have a time also worthwhile to check PEP-702 – Marking deprecations using the type system (and discussion) and maybe it is possible to adopt it to use it, in python before 3.13 it back-ported in typing_extensions

Some upstream PEP-702 related issues/feature requests

@kacpermuda
Copy link
Contributor Author

Adding another example of deprecations from Ray: https://github.com/ray-project/ray/blob/master/python/ray/_private/utils.py#L1090

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:documentation kind:feature Feature Requests stale Stale PRs per the .github/workflows/stale.yml policy file type:improvement Changelog: Improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants