-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Unify Pod Startup Tracking: KubernetesPodTriggerer and KubernetesPodOperator Now Share Common Startup Logic #56875
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
2df75db to
18c1f6a
Compare
jscheffl
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.
I see where this is heading to - and thanks for the consolidation. Seems to be a bit of effort. Hope you are not scared by the amount of comments but a bit of rework is needed.
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/hooks/kubernetes.py
Outdated
Show resolved
Hide resolved
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/hooks/kubernetes.py
Outdated
Show resolved
Hide resolved
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/triggers/pod.py
Outdated
Show resolved
Hide resolved
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/triggers/pod.py
Outdated
Show resolved
Hide resolved
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/utils/pod_manager.py
Outdated
Show resolved
Hide resolved
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/utils/pod_manager.py
Outdated
Show resolved
Hide resolved
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/utils/pod_manager.py
Outdated
Show resolved
Hide resolved
providers/cncf/kubernetes/tests/unit/cncf/kubernetes/triggers/test_pod.py
Outdated
Show resolved
Hide resolved
providers/cncf/kubernetes/tests/unit/cncf/kubernetes/utils/test_pod_manager.py
Outdated
Show resolved
Hide resolved
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/utils/pod_manager.py
Outdated
Show resolved
Hide resolved
18c1f6a to
18faa47
Compare
providers/cncf/kubernetes/tests/unit/cncf/kubernetes/triggers/test_pod.py
Show resolved
Hide resolved
jscheffl
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.
Thanks for the cleanup and improvement. Looks good now. I'll leave the PR open for some other eyes to review, otherwise will merge the next day or so.
…perator Now Share Common Startup Logic (apache#56875) * Move container-related functions from PodManager to a separate file * Moved unit tests * Sync and async workflow use the same code to track Pod startup * Reworked unit tests and pod startup logic * Add api permission error detection for triggerer * Fix pytest fixture * Removed not requried code --------- Co-authored-by: AutomationDev85 <AutomationDev85>
Overview
This PR aligns the startup behavior of KubernetesPodTriggerer and KubernetesPodOperator, ensuring both the synchronous and asynchronous workflows use the same code to track pod startup.
It incorporates changes from the preparation PR #56700, making this PR easier to review once merged. Additionally, PR #56872 is a prerequisite, as it grants the triggerer the necessary permissions to access pod events.
As this is a significant update, we welcome feedback from the community!
Details of change: