kacpermuda opened a new pull request, #36876:
URL: https://github.com/apache/airflow/pull/36876
<!--
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.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
## TLDR;
I propose standardizing our approach to deprecating elements in the Airflow
codebase, aiming for improved readability and easier automated parsing. This PR
serves as a forum to discuss this, supported by some POC code. The whole
process will probably take couple of PR's.
I encourage everyone to share their thoughts and feedback on this proposal.
## Genesis
The genesis of this proposal stems from my attempt to compile a
comprehensive documentation of all deprecations in Airflow. The lack of a
standardized process for deprecation made it challenging to track and document
these changes effectively.
## Problem
Currently, deprecation in Airflow relies mainly on the content of warning
messages, making it less accessible for automated discovery. The typical
process includes:
- Modules: Raising a DeprecationWarning at the top, suggesting full module
deprecation as indicated by the message.
- Classes: Issuing a DeprecationWarning within `__init__`, detailing the
deprecation. The @deprecated decorator is rarely applied to the class.
- Functions/Methods: Emitting a DeprecationWarning within the
function/method, with a description of the deprecation. The @deprecated
decorator is more common here than in classes, but still rare.
- Specific Arguments: Announcing deprecation of an argument with a
DeprecationWarning and a descriptive message within the function/method.
- Argument Values: For changes or deprecations in argument values (e.g.,
changing `schedule` from "@once" to "@single"), a DeprecationWarning is raised
with a detailed message in the function/method.
The current practice of raising a DeprecationWarning in the `__init__` or a
function in Airflow makes it unclear what exactly is being deprecated, whether
it's the class, function, a specific argument, or a particular argument value.
## Proposed solution
I propose adopting a structured approach, potentially enforceable through
the CI process, for handling deprecations in Airflow. This would involve
primarily using decorators for the deprecation process. We could develop new
decorators or utilize existing ones, similar to those in the TensorFlow
[codebase](https://github.com/tensorflow/tensorflow/blob/dec8e0b11f4f87693b67e125e67dfbc68d26c205/tensorflow/python/util/deprecation.py#L274).
These could be housed in a dedicated module, such as `airflow/deprecation.py`.
By implementing our own decorators, we can standardize the information
provided during deprecation, allowing us to specify details using separate
keyword arguments instead of embedding everything within a message.
## Demo
- For modules, we would probably use the current solution.
- For classes, instead of raising a Warning in init, we would decorate a
class (as in change files to this PR).
- For functions and methods, instead of raising a Warning in the body, we
would decorate the class or method. Now, depending on the deprecated thing, we
would either use @deprecated, @deprecated_args, @deprecated_arg_values etc.
Instead of this:
```
def __init__(self, *args, **kwargs) -> None:
super().__init__(*args, **kwargs)
if kwargs.get("sleep_time") is not None:
warnings.warn(
"The `sleep_time` parameter is deprecated, use sleep instead.",
AirflowProviderDeprecationWarning,
stacklevel=2,
)
if kwargs.get("run_type") == "defferable":
warnings.warn(
"Providing 'defferable' as `run_type` is deprecated, use
`defferable=True`.",
AirflowProviderDeprecationWarning,
stacklevel=2,
)
```
We could have:
```
@deprecated_args_value(old_name="run_type", old_value="defferable",
new_name="defferable", new_value=True)
@deprecated_args("sleep_time", new_value_name="sleep")
def __init__(self, *args, **kwargs) -> None:
super().__init__(*args, **kwargs)
```
## Challenges
I'm uncertain if we can entirely remove explicit calls to `warnings.warn`.
For instance, in cases where complex logic determines if a provided argument
value is deprecated, especially with complex objects or nested structures,
using `warnings.warn` directly might still be the most effective method of
notification. It remains to be seen if a decorator can neatly handle such
scenarios.
Please share any potential drawbacks of this approach in the comments.
## Future possibilities / work plan
We adopt the following steps for deprecating components in Airflow:
1. Utilize decorators for deprecating entire classes, functions, and
methods, as improved through discussions in this PR.
2. Create a documentation site listing all deprecations, which simple static
analysis should adequately maintain.
3. Develop additional decorators, such as `arg`, `arg_values` etc., to
replace current warnings within function bodies and include these in the
documentation.
4. Implement a CI check to ensure decorators are consistently used for
deprecation.
Further considerations for future development:
- Utilizing decorators allows for automatic modification of docstrings for
each entity (class, function, etc.), enabling the addition of comprehensive
deprecation information (including details about deprecated arguments and
argument values).
- If we have specific removal timelines (i.e., versions by which components
should be removed from the provider or core Airflow), we can verify their
removal in the pre-release CI process.
Feel free to suggest additional ideas or improvements.
Mentioning @potiuk , as we talked about it on the Slack. Feel free to tag
others who might be interested in contributing their thoughts.
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request
Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)**
for more information.
In case of fundamental code changes, an Airflow Improvement Proposal
([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals))
is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party
License Policy](https://www.apache.org/legal/resolved.html#category-x).
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](https://github.com/apache/airflow/tree/main/newsfragments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
