Fix dag.clear() to set multiple dags to running when necessary #15382
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yuqian90
requested review from
ashb,
ephraimbuddy,
kaxil,
mik-laj,
turbaszek and
XD-DENG
as code owners
boring-cyborg
bot
added
area:API
yuqian90
commented
Apr 15, 2021
yuqian90
force-pushed
the
clear_activate_dagrun
branch
from
5d093f4 to
18bb10c
Compare
yuqian90
commented
Apr 16, 2021
uranusjr
reviewed
Apr 20, 2021
yuqian90
force-pushed
the
clear_activate_dagrun
branch
from
967c96e to
64b0b8d
Compare
yuqian90
force-pushed
the
clear_activate_dagrun
branch
from
64b0b8d to
1afaf51
Compare
yuqian90
force-pushed
the
clear_activate_dagrun
branch
3 times, most recently
from
968f19b to
89f6ed6
Compare
|
Closing/reopening to trigger build again. |
yuqian90
force-pushed
the
clear_activate_dagrun
branch
from
89f6ed6 to
937c170
Compare
|
What's the status of this? @yuqian90 are you just waiting for a final review? |
Yes. The PR has been tested and works well. Just waiting for reviews. |
ashb
reviewed
May 27, 2021
This fixes test_clear_dag in test_dag.py
yuqian90
force-pushed
the
clear_activate_dagrun
branch
from
937c170 to
b251a1d
Compare
Thanks. I put it back. |
potiuk
approved these changes
May 27, 2021
|
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease. |
github-actions
bot
added
the
full tests needed
ashb
reviewed
May 27, 2021
ashb
approved these changes
May 27, 2021
|
Two random failures on MSSQL2019 only (where the DB didn't create the user -- /cc @potiuk @aneesh-joseph ) Merging this PR anyway |
|
Yeah. Some teething problems with MSSQL and @aneesh-joseph created the #16134 that might help! |
yuqian90
added a commit
that referenced
this pull request
May 29, 2021
closes: #14260 related: #9824 When clearing task across dags using ExternalTaskMarker the dag state of the external DagRun is not set to active. So cleared tasks in the external dag will not automatically start if the DagRun is a Failed or Succeeded state. Two changes are made to fix the issue: Make clear_task_instances set DagRuns' state to dag_run_state for all the affected DagRuns. The filter for DagRun in clear_task_instances is fixed too. Previously, it made an assumption that execution_dates for all the dag_ids are the same, which is not always correct. test_external_task_marker_clear_activate is added to make sure the fix does the right thing. (cherry picked from commit 2bca8a5)
jhtimmins
pushed a commit
to astronomer/airflow
that referenced
this pull request
Jun 3, 2021
…e#15382) closes: apache#14260 related: apache#9824 When clearing task across dags using ExternalTaskMarker the dag state of the external DagRun is not set to active. So cleared tasks in the external dag will not automatically start if the DagRun is a Failed or Succeeded state. apache#9824 tried to fix a similar issue for subdag. But it did not fix ExternalTaskMarker. This PR fixes both. Two changes are made to fix the issue: Make clear_task_instances set DagRuns' state to dag_run_state for all the affected DagRuns. The filter for DagRun in clear_task_instances is fixed too. Previously, it made an assumption that execution_dates for all the dag_ids are the same, which is not always correct. test_external_task_marker_clear_activate is added to make sure the fix does the right thing. (cherry picked from commit 2bca8a5)
ashb
pushed a commit
that referenced
this pull request
Jun 22, 2021
closes: #14260 related: #9824 When clearing task across dags using ExternalTaskMarker the dag state of the external DagRun is not set to active. So cleared tasks in the external dag will not automatically start if the DagRun is a Failed or Succeeded state. Two changes are made to fix the issue: Make clear_task_instances set DagRuns' state to dag_run_state for all the affected DagRuns. The filter for DagRun in clear_task_instances is fixed too. Previously, it made an assumption that execution_dates for all the dag_ids are the same, which is not always correct. test_external_task_marker_clear_activate is added to make sure the fix does the right thing. (cherry picked from commit 2bca8a5)
|
Do we have an idea of when this fix will be released and made available via PyPI? |
It's cherry-picked to 2.1 branch and we are just about to release 2.1.1 with it (we are in feature-freeze/final test mode for it). Expect final release in ~ 1 week if everything goes well. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes: #14260
related: #9824
When clearing task across dags using
ExternalTaskMarkerthe dag state of the external DagRun is not set to active. So cleared tasks in the external dag will not automatically start if theDagRunis a Failed or Succeeded state.#9824 tried to fix a similar issue for subdag. But it did not fix
ExternalTaskMarker. This PR fixes both.Two changes are made to fix the issue:
clear_task_instancesset DagRuns' state todag_run_statefor all the affected DagRuns.DagRuninclear_task_instancesis fixed too. Previously, it made an assumption that execution_dates for all the dag_ids are the same, which is not always correct.test_external_task_marker_clear_activateis added to make sure the fix does the right thing.