Skip to content

Conversation

insomnes
Copy link
Contributor

@insomnes insomnes commented Feb 19, 2025

Adding a new task flow decorator to run generated cmds in KubernetesPodOperator.

This decorator is based on the same idea as @task.bash: enrich Kubernetes pod command generation ability with simplified Python usage on the executor side to generate the command.
With the args_only option, the decorator would set the return value as arguments of the image entrypoint instead of rewriting the cmds.

This can significantly simplify usage for pure orchestrator cases and dynamic task mapping or generating the command from another task flow task result.

Changes:

  • Add @task.kubernetes_cmd and register it in provider.yaml for cncf.kubernetes;
  • Refactor @task.kubernetes unit tests to use the same base and have common tests with @task.kubernetes_cmd;
  • Add @overload for @task.kubernetes description in decorators pyi file to enable proper doc-string and signature checking in IDEs
  • System tests and docs example;

closes: #46414


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

@boring-cyborg boring-cyborg bot added the provider:cncf-kubernetes Kubernetes (k8s) provider related issues label Feb 19, 2025
@insomnes insomnes force-pushed the task-kubernetes-cmd-decorator branch 4 times, most recently from 4afbe38 to f4567b7 Compare March 2, 2025 18:48
@insomnes insomnes marked this pull request as ready for review March 2, 2025 19:45
@insomnes insomnes changed the title WIP: Add @task.kuberenetes_cmd Add @task.kuberenetes_cmd Mar 2, 2025
@insomnes
Copy link
Contributor Author

insomnes commented Mar 5, 2025

Should I separate the test refactoring from this PR?

@insomnes insomnes force-pushed the task-kubernetes-cmd-decorator branch from 6836426 to da0a9c2 Compare March 5, 2025 20:53
@insomnes
Copy link
Contributor Author

insomnes commented Mar 5, 2025

The taskflow tutorial docs change docker -> cncf.kubernetes already applied in
#47387

@insomnes insomnes force-pushed the task-kubernetes-cmd-decorator branch 3 times, most recently from d0741ce to 64269e3 Compare March 10, 2025 08:44
@insomnes insomnes marked this pull request as draft March 10, 2025 09:38
@insomnes
Copy link
Contributor Author

I think that this should be adjusted to use TaskSDK and not depend on direct DB access, so made it a draft for now, before I would understand the proper way of doing it.

@insomnes insomnes force-pushed the task-kubernetes-cmd-decorator branch 2 times, most recently from 6ca0c03 to 99943c9 Compare March 25, 2025 22:19
@insomnes insomnes marked this pull request as ready for review March 25, 2025 22:21
@insomnes
Copy link
Contributor Author

Updated based on the recent example of @task.bash decorator. Do not use the database session directly now.

I am still unsure: how to trigger system tests for example DAG in CI.

@insomnes insomnes force-pushed the task-kubernetes-cmd-decorator branch 3 times, most recently from b305d6b to 683961a Compare March 29, 2025 07:55
@potiuk potiuk force-pushed the task-kubernetes-cmd-decorator branch from 683961a to a766622 Compare April 1, 2025 16:17
@potiuk
Copy link
Member

potiuk commented Apr 1, 2025

Rebased it again - just to check - but I think this one will not get too much attention at least before RC1 - but please keep on rebasing it. Being on top of the main and fresh in the inbox helps.

@insomnes
Copy link
Contributor Author

insomnes commented Apr 1, 2025

Rebased it again - just to check - but I think this one will not get too much attention at least before RC1 - but please keep on rebasing it. Being on top of the main and fresh in the inbox helps.

Yeah, I understand, there are a lot of things to do for the maintainers and committers now, and this PR is vast.
I am working on lowering the surface for it and also trying to rebase it from time to time.

Thank you!

Copy link
Contributor

@eladkal eladkal left a comment

Choose a reason for hiding this comment

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

Temporary block till we sort out the decorators in providers

@insomnes insomnes force-pushed the task-kubernetes-cmd-decorator branch from a766622 to 07ae5c1 Compare April 8, 2025 07:46
@insomnes insomnes force-pushed the task-kubernetes-cmd-decorator branch from f4000cf to 0bae401 Compare April 28, 2025 06:26
@kaxil kaxil merged commit 75140f6 into apache:main Apr 28, 2025
97 checks passed
kaxil pushed a commit that referenced this pull request Apr 28, 2025
closes: #46414
(cherry picked from commit 75140f6)
mvfc pushed a commit to mvfc/airflow that referenced this pull request Apr 29, 2025
mvfc pushed a commit to mvfc/airflow that referenced this pull request Apr 29, 2025
jroachgolf84 pushed a commit to jroachgolf84/airflow that referenced this pull request Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
full tests needed We need to run full set of tests for this PR to merge provider:cncf-kubernetes Kubernetes (k8s) provider related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add @task.kubernetes_cmd decorator to run returned command
5 participants