-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
Rescue if a DagRun's DAG was removed from db #17544
Rescue if a DagRun's DAG was removed from db #17544
Conversation
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. |
Should we add a test for this as well? |
Good point, I'll add one (and fix the typo in the commit message 🤦♂️) |
Writing a proper test turns out to be a long journey figuring out a right combination to trigger this bug. So to trigger this bug, a DAG should not be in the DagBag, but in the DagModel table, but not in the SerializedDagModel table. @georborodin Does the DAG you deleted from the web UI have subdags? This is the only situation I’ve managed to set up a proper environment for the test. (Although I wouldn’t be surprised if there’s more I couldn’t locate.) |
cd47635
to
d7a94c5
Compare
For future reference, the complete steps to reproduce this from the web UI is:
|
Fix #17442.
The exception happens when a DAG is removed from the database (via web UI or something else), but there are still unfinished runs associated to it. This catches the scenario and use the existing fallback setting
max_active_runs
to zero.