Standardize deprecations in providers [part1]

[kacpermuda]

TLDR;

I propose standardizing our approach to deprecating elements in the Airflow codebase, aiming for improved readability and easier automated parsing. 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. This PR serves as an introduction and first step. 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. 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. Move all deprecation related Warnings and decorators to a separate module and extend their functionality by providing some additional arguments and formatting the Deprecation message.
3. Create a documentation site listing all deprecations, which simple static analysis should adequately maintain.
4. Develop additional decorators, such as arg, arg_values etc., to replace current warnings within function bodies and include these in the documentation.
5. 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.

---

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

[o-nikolas]

I don't have time to look through this large PR in detail at the moment. But I love the goals of documentation and standardization! +1

[potiuk]

Nice! @dstandish - I guess we could connect it with #31830

[kacpermuda]

@potiuk I already prepared a POC for the custom decorator itself in #36952 . @dstandish check it out, maybe it will help detecting the things that should be removed in specific versions of the providers

[eladkal]

@potiuk I already prepared a POC for the custom decorator itself in #36952 . @dstandish check it out, maybe it will help detecting the things that should be removed in specific versions of the providers

Its important to detect but we must make sure the mechanisem doesnt force our hands.
We have cases where we delebertly keep deprecation for long time even through several major versions.

[kacpermuda]

@potiuk I already prepared a POC for the custom decorator itself in #36952 . @dstandish check it out, maybe it will help detecting the things that should be removed in specific versions of the providers

Its important to detect but we must make sure the mechanisem doesnt force our hands. We have cases where we delebertly keep deprecation for long time even through several major versions.

Sure, we can still not provide that information on purpose and leave it blank and then work that in the detection mechanism.

[kacpermuda]

Hey, is there anything else that i can do to make this PR merged? Pinging those who at least saw it 😄 @eladkal @potiuk @o-nikolas

[potiuk]

I am good for it - @dstandish ? What do you think?

[dstandish]

I am good for it - @dstandish ? What do you think?

Seems we just replace warnings.warn with deprecated decorator, seems fine, to me