Skip to content

Conversation

@dstandish
Copy link
Contributor

When not configured to delete pods, at end of task execution the current behavior is to patch the pod as "already checked", but only if pod not successful. We should also patch when successful so it isn't "reattached" to after a task clear.

@boring-cyborg boring-cyborg bot added provider:cncf-kubernetes Kubernetes (k8s) provider related issues area:providers labels Apr 4, 2022
@dstandish dstandish force-pushed the kpo-patch-always branch 2 times, most recently from 85a9e5d to d8cbde2 Compare April 14, 2022 17:09
@dstandish dstandish marked this pull request as ready for review April 14, 2022 21:48
@jedcunningham
Copy link
Member

Looks good, but no unit test changes? Hmm.

@dstandish
Copy link
Contributor Author

Looks good, but no unit test changes? Hmm.

Ah you mean as opposed to integration tests? Sure I can add a unit test for this.

@dstandish
Copy link
Contributor Author

updated unit tests to test for this directly. removed one test that was a duplicate.

When not configured to delete pods, at end of task execution the current behavior is to patch the pod as "already checked", but only if pod not successful.  We should also patch when successful so it isn't "reattached" to after a task clear.
@github-actions github-actions bot added the okay to merge It's ok to merge this PR as it does not require more tests label Apr 16, 2022
@github-actions
Copy link

The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.

@dstandish dstandish merged commit c3d883a into apache:main Apr 19, 2022
@dstandish dstandish deleted the kpo-patch-always branch April 19, 2022 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers okay to merge It's ok to merge this PR as it does not require more tests provider:cncf-kubernetes Kubernetes (k8s) provider related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants