-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Migrate standard decorators to standard provider #48683
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
Migrate standard decorators to standard provider #48683
Conversation
kaxil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should move tests too airflow-core/tests/unit/decorators
|
Yeah i am looking into it |
|
Added more tasks in the task to test for shortcircuit and sensor too. DAG: |
564d810 to
42542b5
Compare
|
Just squashed it all together. Hopefully the compat and mypy have been fixed now! 🤞🏽 |
| @pytest.mark.skipif( | ||
| not AIRFLOW_V_3_0_PLUS, | ||
| reason="Decorators were part of core not providers, so this test doesnt make sense for < AF3.", | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just want to highlight this for others.
The reason we did this, is that On Airflow 2.x, task.python and all the other previous "core" operators were hard-coded in https://github.com/apache/airflow/blob/2.10.5/airflow/decorators/__init__.py#L61-L71
Which means that it is impossible for users to use these decorators on Airflow 2, meaning we should skip these tests in the compat tests, as the decorators specifically won't ever be using the one from the standard provider, even if installed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolving this comment, but yes thats right
providers/standard/tests/unit/standard/decorators/test_python.py
Outdated
Show resolved
Hide resolved
be10686 to
be7502e
Compare

related: #45436
Why?
Since the move of operators / hooks to standard provider, we should ideally also be moving the decorators to the standard provider or where they truly belong. This will permit those being out of core and allow for a more flexible release and bug fix cadence.
What have been moved so far
What i am not so sure if it needs moving to standard:
Points to note
python_taskinsideairflow-core/src/airflow/decorators/__init__.pyto be able to continue allowing it to be the said default, do comment if you know of a better way :)Testing:
Created a massive dag with all these decorators in it
Works as expected:

^ 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.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.