Repository navigation
Clear language SDK task XComs in the coordinator when a try starts - #74221
jason810496 merged 3 commits into
Conversation
14c70d1 to
1da3011
Compare
1da3011 to
b7191c6
Compare
There was a problem hiding this comment.
Thanks for the fix.
I pushed 34bbaee on top with a few fixes from review:
- Rethrow
CancellationExceptionwhile clearing XComs, so a cancelled task doesn't report a failure or use up a retry. - Corrected the comment on why the worker clears XComs, and added a TODO: Python's
XCom.deletealso purges custom XCom backends, which the Java runtime can't do yet. ServerTest: the fake supervisor thread is now a daemon and is always stopped, so a failing case doesn't leave a thread running.- Moved the throwing test transport to a shared
NoOpTransport. The test fakes now useby NoOpTransportand override only what they use.
|
Hm, should this be done in the coordinator instead of reimplemented in every sdk? |
jason810496
left a comment
jason810496
left a comment
There was a problem hiding this comment.
Hm, should this be done in the coordinator instead of reimplemented in every sdk?
Yes, it would be better to cover this in the core layer instead of having it in each SDK.
Marked the PR as draft for now.
Agree, it will be more clear. I will do it in this PR and close TS SDK one #74220. |
34bbaee to
ed8efe4
Compare
FrankYang0529
changed the title
ed8efe4 to
2f45b3a
Compare
jason810496
left a comment
jason810496
left a comment
There was a problem hiding this comment.
Nice, thanks. The change matches my imagination and LGTM, one more thing before merge.
Could you help keep the go-sdk change out of this PR and keep them in a new follow-up PRs please? Thanks.
The Python task-sdk part is ready to go and it's ready to backport as well.
However, on the language SDK side, we need to wait for several backport PRs get merged first before the go-sdk change in one.
8f5255d to
a76390e
Compare
Signed-off-by: PoAn Yang <payang@apache.org>
Signed-off-by: PoAn Yang <payang@apache.org>
Signed-off-by: PoAn Yang <payang@apache.org>
a76390e to
dfa034c
Compare
Thanks for the review. I moved the go-sdk change out of this PR. |
jason810496
added
area:coordinator
Backport successfully created: v3-4-testNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
|
…ry starts (#74221) (#74936) * Clear language SDK task XComs in the coordinator when a try starts * Keep XCom deletion in the Go runtime until backports are merged * Use httpx2 in the new XCom clearing tests --------- (cherry picked from commit 4b464ee) Signed-off-by: PoAn Yang <payang@apache.org> Co-authored-by: PoAn Yang <payang@apache.org>
…ry starts (#74221) (#74936) * Clear language SDK task XComs in the coordinator when a try starts * Keep XCom deletion in the Go runtime until backports are merged * Use httpx2 in the new XCom clearing tests --------- (cherry picked from commit 4b464ee) Signed-off-by: PoAn Yang <payang@apache.org> Co-authored-by: PoAn Yang <payang@apache.org>
…pache#74221) * Clear language SDK task XComs in the coordinator when a try starts Signed-off-by: PoAn Yang <payang@apache.org> * Keep XCom deletion in the Go runtime until backports are merged Signed-off-by: PoAn Yang <payang@apache.org> * Use httpx2 in the new XCom clearing tests Signed-off-by: PoAn Yang <payang@apache.org> --------- Signed-off-by: PoAn Yang <payang@apache.org>
Follow-up for #74178 (review). The coordinator now deletes the XComs once when a try starts, instead of each language SDK runtime deleting them.
Why
xcom_keys_to_clear, the list of XComs that the worker must delete before the task runs. The Python task runner and the Go runtime delete these XComs. The Java and TypeScript runtimes do not.How
_PopenActivitySubprocess, which the Java, Go and TypeScript coordinators share, now deletes the listed XComs after the server marks the try as running and before sendingStartupDetailsto the runtime.Was generative AI tooling used to co-author this PR?