Skip to content

Allow keeping finished task states on Dag run patch#68666

Merged
pierrejeambrun merged 1 commit into
apache:mainfrom
guan404ming:fix-67695-mark-dagrun-success-state
Jul 9, 2026
Merged

Allow keeping finished task states on Dag run patch#68666
pierrejeambrun merged 1 commit into
apache:mainfrom
guan404ming:fix-67695-mark-dagrun-success-state

Conversation

@guan404ming

@guan404ming guan404ming commented Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Related Issue

Related #67695

Why

Re-marking a run to its existing state (success → success) rewrote already-finished task states.

How

  • Allow keeping finished task states on Dag run patch
Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)
    Claude Code with Opus4.8
  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

@boring-cyborg boring-cyborg Bot added the area:API Airflow's REST/HTTP API label Jun 17, 2026
@guan404ming guan404ming marked this pull request as ready for review June 17, 2026 10:56
yuseok89
yuseok89 reviewed Jun 19, 2026
View reviewed changes
Comment thread airflow-core/src/airflow/api_fastapi/core_api/services/public/dag_run.py
pierrejeambrun
pierrejeambrun reviewed Jun 23, 2026
View reviewed changes

@pierrejeambrun pierrejeambrun left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This was deliberately allowed in #66198, both behaviors conflict, we need to decide what we want to do here. An early like this removes a 'feature'.

This was the behavior implemented in 2.x I believe

@guan404ming guan404ming force-pushed the fix-67695-mark-dagrun-success-state branch from 38d81be to a52660b Compare June 26, 2026 12:07
@guan404ming

guan404ming commented Jun 26, 2026

Copy link
Copy Markdown
Member Author

Thanks for the information. In order to resolve the conflict, I made overwriting opt-out instead of removing it: overwrite now defaults to True, keeping the #66198 behavior of re-marking already-finished (success/failed/skipped) tasks. In the UI I added a GitHub-style split button (like the merge dropdown). the main action stays "Confirm with overwrite", and the dropdown switches to "Confirm without overwrite" for overwrite=False.

So both behaviors are supported and the default is unchanged. Does that direction work for you? Please feel free to let me know your opinion and happy to refine it to make it better, thanks!

Screen.Recording.2026-06-26.at.7.51.41.PM.mov

@guan404ming guan404ming changed the title Prevent run state patch from overwriting finished task states Allow keeping finished task states on Dag run patch Jun 26, 2026
@guan404ming guan404ming force-pushed the fix-67695-mark-dagrun-success-state branch 6 times, most recently from a5b3c67 to 723f40d Compare June 29, 2026 07:11
pierrejeambrun
pierrejeambrun reviewed Jul 1, 2026
View reviewed changes

@pierrejeambrun pierrejeambrun left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Intead of doing this, I believe similarly to the set_dag_run_state_to_failed, the set_dag_run_state_to_success should:

  • Operate only on running states TaskInstance.state.in_(running_states),
  • Mark non-finished tasks as SKIPPED
  • Do not skip teardown tasks

That would solve it. Basically having a set_dag_run_state_to_success closer to set_dag_run_state_to_failed.

(Which is a simpler PR overall)

@pierrejeambrun pierrejeambrun added this to the Airflow 3.3.1 milestone Jul 1, 2026
@guan404ming guan404ming force-pushed the fix-67695-mark-dagrun-success-state branch from 723f40d to 6522569 Compare July 3, 2026 01:18
@guan404ming

guan404ming commented Jul 3, 2026

Copy link
Copy Markdown
Member Author

Updated as suggested. Success branch now mirrors failed (running → success, non-finished → skipped, teardowns untouched), sharing one implementation. Old approach reverted.

@guan404ming guan404ming force-pushed the fix-67695-mark-dagrun-success-state branch 3 times, most recently from 8ef31db to fbec914 Compare July 8, 2026 05:57
@guan404ming guan404ming force-pushed the fix-67695-mark-dagrun-success-state branch from fbec914 to 98abe20 Compare July 9, 2026 04:39
@pierrejeambrun pierrejeambrun added the backport-to-v3-3-test Backport to v3-3-test label Jul 9, 2026
@pierrejeambrun pierrejeambrun merged commit bb29517 into apache:main Jul 9, 2026
79 checks passed
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-3-test

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

Status Branch Result
✅ v3-3-test PR Link

@pierrejeambrun pierrejeambrun mentioned this pull request Jul 9, 2026
Closed
2 tasks
github-actions Bot pushed a commit to aws-mwaa/upstream-to-airflow that referenced this pull request Jul 9, 2026
@guan404ming
[v3-3-test] Allow keeping finished task states on Dag run patch (apac…
5cc3f50 
…he#68666)

(cherry picked from commit bb29517)

Co-authored-by: Guan-Ming Chiu <105915352+guan404ming@users.noreply.github.com>
aws-airflow-bot pushed a commit to aws-mwaa/upstream-to-airflow that referenced this pull request Jul 9, 2026
@guan404ming
[v3-3-test] Allow keeping finished task states on Dag run patch (apac…
d548532 
…he#68666)

(cherry picked from commit bb29517)

Co-authored-by: Guan-Ming Chiu <105915352+guan404ming@users.noreply.github.com>
@DonHaul DonHaul mentioned this pull request Jul 10, 2026
Open
pierrejeambrun pushed a commit that referenced this pull request Jul 10, 2026
@github-actions @guan404ming
[v3-3-test] Allow keeping finished task states on Dag run patch (#68666…
d7fd872 
…) (#69662)

(cherry picked from commit bb29517)

Co-authored-by: Guan-Ming Chiu <105915352+guan404ming@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pierrejeambrun pierrejeambrun pierrejeambrun approved these changes
@ephraimbuddy ephraimbuddy Awaiting requested review from ephraimbuddy ephraimbuddy is a code owner
@rawwar rawwar Awaiting requested review from rawwar rawwar is a code owner
@jason810496 jason810496 Awaiting requested review from jason810496 jason810496 is a code owner
@bugraoz93 bugraoz93 Awaiting requested review from bugraoz93
@shubhamraj-git shubhamraj-git Awaiting requested review from shubhamraj-git
@choo121600 choo121600 Awaiting requested review from choo121600
@bbovenzi bbovenzi Awaiting requested review from bbovenzi
@ryanahamilton ryanahamilton Awaiting requested review from ryanahamilton
@vatsrahul1001 vatsrahul1001 Awaiting requested review from vatsrahul1001
@potiuk potiuk Awaiting requested review from potiuk
@dheerajturaga dheerajturaga Awaiting requested review from dheerajturaga
+1 more reviewer
@yuseok89 yuseok89 yuseok89 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area:API Airflow's REST/HTTP API backport-to-v3-3-test Backport to v3-3-test

Projects

None yet

Milestone

Airflow 3.3.1

Development

Successfully merging this pull request may close these issues.

3 participants

@guan404ming @yuseok89 @pierrejeambrun