-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Refactor Taskflow decorator for extensibility #14709
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
Conversation
121e1f9 to
fcc2510
Compare
|
The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*. |
|
The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*. |
ashb
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.
This feels a little bit like a move for the sake of a move.
Can you explain your thinking a little bit more please?
|
@ashb the next step when this is merged will be to create a @docker_task decorator. The way that that the @task decorator is written right now, it's HEAVILY tied into only running a python function on the current virtual environment. Extending it is basically impossible and a lot of boilerplate would need to be rewritten to write new ones. Ultimately I want to create three extensions: 1. create a virtual environment for this task, 2. run this python function in the specified docker container, 3. launch this task in a kubernetes pod using the KPO. |
|
Ah I missed the addition of |
|
The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*. |
|
The Workflow run is cancelling this PR. Building image for the PR has been cancelled |
|
The Workflow run is cancelling this PR. Building images for the PR has failed. Follow the workflow link to check the reason. |
The Taskflow API is a very clean way to turn python functions into Airflow tasks, but it is currently limited to only running in the virtualenv of the parent worker. We eventually want to offer the ability to use decorators that tie to docker images, kubernetes pods, etc. This commit seperates out the python specific code and creates a decorators directory, which will allow us to both build in more "core" decorators as well as decorators in providers.
Co-authored-by: Ash Berlin-Taylor <[email protected]>
Co-authored-by: Ash Berlin-Taylor <[email protected]>
80f6a07 to
9779090
Compare
|
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease. |
Co-authored-by: Kaxil Naik <[email protected]>
|
The Workflow run is cancelling this PR. Building images for the PR has failed. Follow the workflow link to check the reason. |
| return wrapper(python_callable) | ||
| elif python_callable is not None: | ||
| raise AirflowException('No args allowed while using @task, use kwargs instead') | ||
| return wrapper |
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.
How is it possible to get to this final return wrapper? What's the scenario where python_callable is valid but not a callable?
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.
I think this is the difference between @task (python_callable will be passed already) and @task() -- python_callable won't yet be passed.
The Taskflow API is a very clean way to turn python functions into
Airflow tasks, but it is currently limited to only running in the
virtualenv of the parent worker. We eventually want to offer the ability
to use decorators that tie to docker images, kubernetes pods, etc.
This commit seperates out the python specific code and creates a
decorators directory, which will allow us to both build in more "core"
decorators as well as decorators in providers.
^ 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.