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

Make decorators pluggable #17270

Closed
wants to merge 13 commits into from

Conversation

dimberman
Copy link
Contributor

This PR will allow users to add custom "@task.____" decorators by adding
to their setup.cfg files. This will make decorators seem native to
airflow while living in provider packages.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 UPDATING.md.

@dimberman dimberman requested review from jhtimmins, potiuk and ashb and removed request for jhtimmins and potiuk July 27, 2021 18:27
Copy link
Member

@ashb ashb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General comments: I rather than airflow.decorator_implementations.python etc I would just leave them as airflow.decorators.python -- I don't think we really need to separate the folders

Have you played with creating a type-stub file for the attrs on the @task decorator? Does it work for IDE completion?

airflow/decorators/__init__.py Outdated Show resolved Hide resolved
airflow/decorators/__init__.py Show resolved Hide resolved
@dimberman
Copy link
Contributor Author

dimberman commented Jul 28, 2021

@ashb unfortunately the problem with leaving them where they are is that because we are loading this stuff in the __init__ file, it leads to an infinite loop.

@ashb
Copy link
Member

ashb commented Jul 28, 2021

@ashb unfortunately the problem with leaving them where they are is that because we are loading this stuff in the __init__ file, it leads to an infinite loop.

Given python operator is built in, for the sake of efficiencey of parsing it might be worth not having an entrypoint for task.python etc. anyway.

@dimberman dimberman marked this pull request as ready for review July 28, 2021 17:47
@dimberman dimberman requested a review from kaxil as a code owner July 28, 2021 17:47
@ashb
Copy link
Member

ashb commented Jul 28, 2021

potiuk
potiuk previously requested changes Jul 28, 2021
airflow/decorators/__init__.py Outdated Show resolved Hide resolved
@dimberman dimberman force-pushed the make-decorators-pluggable branch from 30a6c95 to d0fdb93 Compare August 9, 2021 15:48
This PR will allow users to add custom "@task.____" decorators by adding
to their setup.cfg files. This will make decorators seem native to
airflow while living in provider packages.
@dimberman dimberman force-pushed the make-decorators-pluggable branch from d0fdb93 to 3121eef Compare August 9, 2021 15:48
@dimberman
Copy link
Contributor Author

@potiuk PTAL I think this addresses what you wanted?

@dimberman dimberman requested a review from potiuk August 9, 2021 16:59
@dimberman dimberman dismissed potiuk’s stale review August 9, 2021 17:19

Changes addressed

@dimberman
Copy link
Contributor Author

@potiuk just realized I need to fix the documentation too. Doing that now.

@dimberman
Copy link
Contributor Author

@potiuk Ok now it's updated PTAL thank you :)

@dimberman dimberman closed this Sep 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants