Skip to content

Conversation

@peloyeje
Copy link
Contributor

@peloyeje peloyeje commented Sep 21, 2023

Adds a new "clear (including downstream)" action in the "List Task Instances" view
Also a bit of refactoring so that "clear" / "clear including downstream" share the same logic
Added a unit test to make sure clearing logic is "run-aware"

closes: #33014


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborg boring-cyborg bot added area:webserver Webserver related Issues kind:documentation labels Sep 21, 2023
@hussein-awala hussein-awala self-requested a review September 21, 2023 17:54
@hussein-awala hussein-awala added the type:new-feature Changelog: New Features label Sep 21, 2023
@hussein-awala hussein-awala added this to the Airflow 2.8.0 milestone Sep 21, 2023
@peloyeje peloyeje marked this pull request as ready for review September 21, 2023 19:10
Copy link
Member

@hussein-awala hussein-awala left a comment

Choose a reason for hiding this comment

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

Nice work!

@peloyeje peloyeje force-pushed the feature/clear-with-downstream branch 2 times, most recently from 3977962 to 4ad02d1 Compare September 22, 2023 00:52
Copy link
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

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

Tested locally, looking nice

@peloyeje peloyeje force-pushed the feature/clear-with-downstream branch from ca8cfaa to 9e5a466 Compare September 22, 2023 16:53
@peloyeje
Copy link
Contributor Author

peloyeje commented Sep 22, 2023

Just pushed yet another tweaks to the initial implementation so that models.clear_task_instances is called only once per DAG and batches DAG run operations; sorry for all these iterations, but I've run out of ideas now 😄

Copy link
Member

@hussein-awala hussein-awala left a comment

Choose a reason for hiding this comment

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

Looks good! LGTM

task_id for task_id in partial_dag.task_dict if task_id not in task_ids_to_clear
]

# dag.clear returns TIs when in dry run mode
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

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

Looking good!

@hussein-awala hussein-awala merged commit 5b0ce3d into apache:main Sep 22, 2023
@boring-cyborg
Copy link

boring-cyborg bot commented Sep 22, 2023

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

@hussein-awala
Copy link
Member

Congrats Jean-Eudes on your first commit 🎉

@peloyeje
Copy link
Contributor Author

Thanks everyone for your feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:webserver Webserver related Issues kind:documentation type:new-feature Changelog: New Features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clearing task from List Task Instance page in UI does not also clear downstream tasks?

4 participants