Skip to content

Conversation

@johnhofman
Copy link
Contributor

Description

Creating a pod that exceeds a namespace's resource quota throws an ApiException. This change catches the exception and the task is re-queued inside the Executor instead of killing the scheduler.

click 7.0 was recently released but flask-appbuilder 1.11.1 has requirement click==6.7. I have pinned click==6.7 to make the dependencies resolve.

Tests

This adds a single test TestKubernetesExecutor. test_run_next_exception that covers this single scenario. Without the changes this test fails when the ApiException is not caught.

This is the first test case for the KubernetesExecutor, so I needed to add the [kubernetes] section to default_test.cfg so that the KubernetesExecutor can be built without exceptions.

Jira ticket: https://issues.apache.org/jira/browse/AIRFLOW-2966

Creating a pod that exceeds a namespace's resource quota throws an
ApiException. This change catches the exception and the task is
re-queued inside the Executor instead of killing the scheduler.
@johnhofman
Copy link
Contributor Author

@Fokko This is a resubmission of #3960, which was reverted when it broke the CI on master. This was due to the refactoring of the task instance key in PR #3994, which was not included in the PR #3960 tests, but got merged to master first.

@codecov-io
Copy link

codecov-io commented Nov 18, 2018

Codecov Report

Merging #4209 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4209      +/-   ##
==========================================
+ Coverage    77.7%   77.71%   +<.01%     
==========================================
  Files         199      199              
  Lines       16317    16317              
==========================================
+ Hits        12679    12680       +1     
+ Misses       3638     3637       -1
Impacted Files Coverage Δ
airflow/models.py 92.33% <0%> (+0.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94d1970...b4365c4. Read the comment docs.

Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

Thanks @johnhofman, looking good. Thanks for rebasing and fixing the tests.

@Fokko Fokko merged commit e74ec34 into apache:master Nov 20, 2018
tmiller-msft pushed a commit to cse-airflow/incubator-airflow that referenced this pull request Nov 27, 2018
…4209)

Creating a pod that exceeds a namespace's resource quota throws an
ApiException. This change catches the exception and the task is
re-queued inside the Executor instead of killing the scheduler.
elizabethhalper pushed a commit to cse-airflow/incubator-airflow that referenced this pull request Dec 7, 2018
…4209)

Creating a pod that exceeds a namespace's resource quota throws an
ApiException. This change catches the exception and the task is
re-queued inside the Executor instead of killing the scheduler.
aliceabe pushed a commit to aliceabe/incubator-airflow that referenced this pull request Jan 3, 2019
…4209)

Creating a pod that exceeds a namespace's resource quota throws an
ApiException. This change catches the exception and the task is
re-queued inside the Executor instead of killing the scheduler.
ashb pushed a commit that referenced this pull request Mar 21, 2019
Creating a pod that exceeds a namespace's resource quota throws an
ApiException. This change catches the exception and the task is
re-queued inside the Executor instead of killing the scheduler.
ashb pushed a commit that referenced this pull request Mar 22, 2019
Creating a pod that exceeds a namespace's resource quota throws an
ApiException. This change catches the exception and the task is
re-queued inside the Executor instead of killing the scheduler.
wmorris75 pushed a commit to modmed-external/incubator-airflow that referenced this pull request Jul 29, 2019
…4209)

Creating a pod that exceeds a namespace's resource quota throws an
ApiException. This change catches the exception and the task is
re-queued inside the Executor instead of killing the scheduler.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants