apache
Add bulk TI deletion UI
Related PR
- this one is the follow up pr of #50443 to implement bulk TI deletion ui
How
- add multi-select in ti table
- minor fix in bulk TI deletion endpoint
https://github.com/user-attachments/assets/ed9455dd-ed94-44a0-a913-2641db5663dd
^ 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 airflow-core/newsfragments.
I have updated the translation and try reuse AffectedTasks
Current ui looks like:
https://github.com/user-attachments/assets/ed3a2b4d-b800-4443-81d5-664547bafe74
For mapped task instance the confirmation table is not correct, cf screenshot, actually 4 TIs are selected (3 mapped TI and 1 non mapped TI), and the modal only show 2 TIs)
The api needs some modification for handling ti with map_index as well. Let me mark this as draft.
The api needs some modification for handling ti with map_index as well. Let me mark this as draft.
Thanks @guan404ming, do not hesitate to create a different PR for the backend part so it's easier to review and merge. (and you can rebase this branch on top of the backend change to test it locally)
Thanks for the suggestion, I've implemented the backend part of this PR in https://github.com/apache/airflow/pull/51850.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Backend PR has been merged, I think we can proceed forward with this when we have time.
Backend PR has been merged, I think we can proceed forward with this when we have time.
I think frontend part is done. but I encounter some issues when deleting mapped ti and has submitted pr to fix it. I think I could mark this one as ready to review and would rebase after the fix got merged
Hang on. With the TI history, I'm not sure we want to ever delete TIs. Is this just a wording, or are we actually issuing a delete request on the TI row? (If so what happens to the history/tries table when the scheduler goes and recreates these rows?
This is actual deletion - we issue DELETE on the TI row. The TaskInstanceHistory table has CASCADE delete, so all history records are permanently lost when the TI is deleted. When the scheduler recreates the TI, it gets a new UUID and try_number resets to 0, losing all previous execution history.
-1. We should never delete TIs and TIH. This shouldn't even be a feature in the API!
-1. We should never delete TIs and TIH. This shouldn't even be a feature in the API!
It seems like the singular TI deletion is already in our UI and API. Should we need to also remove those related features?
I guess it's up to users if they want to do it, but we should probably make it 100% clear it will delete all history and audit records of the TI.
I don't personally like it, but now I think about it more I'm not sure that we should stop users from being able to do this if they want to.
Maybe a future change to add a config option to disable deleteing them at the deploy level
I guess it's up to users if they want to do it, but we should probably make it 100% clear it will delete all history and audit records of the TI.
I don't personally like it, but now I think about it more I'm not sure that we should stop users from being able to do this if they want to.
Maybe a future change to add a config option to disable deleteing them at the deploy level
How about we update the copy "This will remove all metadata related" and spell out what will be deleted?
Not being able to easily sort/search-by-params/delete runs/TIs in bulk is a big con of AF3 for our use case.
@guan404ming any progress on this one ? I think a UI warning could suffice as mentioned by Brent, and unblock this.
@guan404ming any progress on this one ? I think a UI warning could suffice as mentioned by Brent, and unblock this.
Sure! I'm also agree with this solution. I've update the warning to a more detailed version, please let me if it could be better. Thanks!