Skip to content

Conversation

@dimberman
Copy link
Contributor

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.

@dimberman dimberman requested a review from kaxil March 11, 2021 00:48
@dimberman dimberman requested review from ashb and turbaszek March 11, 2021 00:48
@dimberman dimberman force-pushed the seperate-decorators branch from 121e1f9 to fcc2510 Compare March 11, 2021 00:53
@github-actions
Copy link

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

@github-actions
Copy link

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

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.

This feels a little bit like a move for the sake of a move.

Can you explain your thinking a little bit more please?

@dimberman
Copy link
Contributor Author

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

@ashb
Copy link
Member

ashb commented Mar 11, 2021

Ah I missed the addition of impl_class. Cool.

@github-actions
Copy link

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

@github-actions
Copy link

The Workflow run is cancelling this PR. Building image for the PR has been cancelled

@dimberman dimberman marked this pull request as ready for review March 12, 2021 15:18
@dimberman dimberman requested a review from XD-DENG as a code owner March 12, 2021 15:18
@github-actions
Copy link

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.
@dimberman dimberman force-pushed the seperate-decorators branch from 80f6a07 to 9779090 Compare March 12, 2021 17:28
@github-actions
Copy link

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.

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Mar 12, 2021
@github-actions
Copy link

The Workflow run is cancelling this PR. Building images for the PR has failed. Follow the workflow link to check the reason.

@dimberman dimberman merged commit 4e49adc into apache:master Mar 12, 2021
@dimberman dimberman deleted the seperate-decorators branch March 12, 2021 21:43
return wrapper(python_callable)
elif python_callable is not None:
raise AirflowException('No args allowed while using @task, use kwargs instead')
return wrapper
Copy link
Contributor

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?

Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:dev-tools full tests needed We need to run full set of tests for this PR to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants