Add bulk XCom deletion to Execution API#59874
Conversation
80df148 to
5eef562
Compare
|
Just following up here - I implemented the suggestions about the XCom.delete() parameters. Let me know if the approach looks sound. |
|
This will need a migration to move existing calls to the new endpoint (since the path has changed). |
5eef562 to
e2e8422
Compare
f7c9646 to
dae61bb
Compare
8ef2514 to
1c60599
Compare
1c60599 to
f104dfb
Compare
|
Updated the |
f104dfb to
ffaddec
Compare
|
@amoghrajesh, not usually one to tag but I couldn't re-request review. Do you mind taking a look at the updates whenever you have a chance. Thanks. |
|
Please rebase. You have conflicts. |
ffaddec to
387db67
Compare
a0cd44c to
7198380
Compare
Thanks for the review! Addressed all the comments. The bulk delete endpoint now returns the count of deleted rows instead of the generic message. The client then returns a |
7198380 to
9c1e1b9
Compare
76887d5 to
f877346
Compare
|
@amoghrajesh Can you give this another review when you have a chance? Addressed all of your most recent comments. Thanks. |
f877346 to
7421401
Compare
|
@amoghrajesh — @justinpakzad addressed your feedback on this PR back in May (most recent push: 2026-05-14) and the CHANGES_REQUESTED state has now been blocking the PR for ~5 months. Could you either re-review and dismiss / re-approve, or explicitly dismiss the request-changes if your concerns are addressed? @eladkal mentioned earlier we wanted this in 3.2.0 — happy to have someone else take a look if you don't have bandwidth, just let us know. Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting |
f0d8d48 to
e66f192
Compare
|
@amoghrajesh do you mind giving this another review? Would love to get this merged. I addressed all of your previous comments and I also made updates to follow the new request_handlers pattern, so that it's aligned with all the rest. Thanks. |
e66f192 to
e9078b6
Compare
|
@justinpakzad — Changes have been requested on this PR, so I've removed the Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you. |
@potiuk The changes requested had been addressed quite a while ago and I have consistently pushed fixes for all other review comments. Been awaiting for another review on this. Would appreciate if this was added back to the maintainer queue. |
|
cc: @ashb |
c8bb987 to
668ff4a
Compare
|
Would really appreciate another review on this. |
668ff4a to
b093bf7
Compare
Add a DELETE /xcoms/{dag_id}/{run_id} endpoint that
deletes all XCom entries for a Dag run, with optional
task_id, key, and map_index filters. Returns the count
of deleted rows.
b093bf7 to
988b548
Compare
|
@ashb would you mind giving this a review when you have a moment? |
This PR implements bulk XCom deletion functionality. The issue was that it's not possible to delete all XComs for a dag run using the
XCom.delete()method since task_id and key are required arguments. Given there is no direct database access, adding bulk deletion functionality would allow users to run clean up tasks at the end of their Dags, which can help prevent db bloat when the Xcoms don't really have much archival value.I added a new
bulk_delete_xcoms()endpoint to the execution API with task_id and key as optional query parameters. This allows a user to just pass in dag_id and run_id to delete all XComs associated with that run.A
delete_all()method was added to the BaseXCom class as well as the associated method in the client which calls the execution API. New message type was added to the supervisor and trigger job runner.I created tests to cover the different deletion patterns: deleting all XComs for a run, deleting all keys for a specific task, and deleting a specific key across all tasks. Cadwyn migration file was also updated with the new change.
closes: #57812
^ 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.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.