Skip to content

Conversation

@amoghrajesh
Copy link
Contributor

Building on top of #31146 to introduce the active_deadline_seconds for airflow KPO launched job pods.


^ 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 area:providers provider:cncf-kubernetes Kubernetes (k8s) provider related issues labels Aug 14, 2023
@amoghrajesh amoghrajesh requested a review from uranusjr August 14, 2023 05:34
@potiuk
Copy link
Member

potiuk commented Aug 14, 2023

Needs conflict resolution and fixes in tests ...

@amoghrajesh
Copy link
Contributor Author

@potiuk I fixed the tests as well as rebased it. Can you take a look when free?

Copy link
Member

@hussein-awala hussein-awala left a comment

Choose a reason for hiding this comment

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

I pushed a commit hoping it might fix the failed test. LGTM

@eladkal eladkal changed the title Inserting active_deadline_seconds in KPO Add active_deadline_seconds parameter to KubernetesPodOperator Aug 14, 2023
@amoghrajesh
Copy link
Contributor Author

@hussein-awala seems like the commit didn't do the trick here. I pushed a new fix now, might just do the trick now

@potiuk
Copy link
Member

potiuk commented Aug 17, 2023

Nope. Seems like it break our K8S tests. FYI. There is a recipe how to setup the environment for K8S tests locally with breeze including step-by-step explanation and example (and Breeze's k8s is really a wizard-like setup that hand-holds you while setting the environment up).

https://github.com/apache/airflow/blob/main/TESTING.rst#kubernetes-tests

@hussein-awala hussein-awala merged commit e991f60 into apache:main Aug 18, 2023
@hussein-awala
Copy link
Member

🎉

@amoghrajesh
Copy link
Contributor Author

Silly mistake by me. Thanks Hussein!

@andyfcx
Copy link

andyfcx commented Aug 22, 2023

Thanks @amoghrajesh

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

Labels

area:providers provider:cncf-kubernetes Kubernetes (k8s) provider related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants