Repository navigation
Lower severity of handled XCom misses - #73198
Conversation
|
Reviewed the XCom 404 handling and both new regression cases. VPS validation: the two new warning-level tests pass in an isolated task-SDK harness, and Ruff format/check plus git diff --check pass. The full repository test command was not rerun because the Airflow test plugin requires uv, which is not installed on this VPS. The existing Airflow-required generative-AI disclosure remains in the PR body. |
|
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. |
|
Hi! We’re also affected by #73175 and would love to see this merged. We run Airflow 3.3.2 in production, and these handled 404s on XCom lookups show up as ERROR in our task logs, which adds noise and makes real failures harder to spot. Since the client already handles the 404 with a fallback response, logging it as a warning seems like the right behavior. I tested this change in our environment and it works as expected: the misses are now logged at warning level and task behavior is unchanged. Could a maintainer take a look when there’s a chance? Thanks! |
LazyXComIterator reads offsets until the server returns 404, so every iteration over a mapped upstream ends in a handled miss. Log both XCom not-found branches at debug, like the Connection and Variable lookups. Move the new tests to httpx2, use the server's real 404 body, and name them after the fallback they cover.
b269cfc to
bdd61c8
Compare
kaxil
left a comment
There was a problem hiding this comment.
Rebased onto main and pushed a commit that moves both branches to debug and the tests to httpx2. Thanks @axelray-dev!
|
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
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.
|
* Lower log level for handled XCom misses * Log handled XCom misses at debug level LazyXComIterator reads offsets until the server returns 404, so every iteration over a mapped upstream ends in a handled miss. Log both XCom not-found branches at debug, like the Connection and Variable lookups. Move the new tests to httpx2, use the server's real 404 body, and name them after the fallback they cover. --------- (cherry picked from commit f05c32e) Co-authored-by: AxelRay <raynaldo.axel@gmail.com> Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
* Lower log level for handled XCom misses * Log handled XCom misses at debug level LazyXComIterator reads offsets until the server returns 404, so every iteration over a mapped upstream ends in a handled miss. Log both XCom not-found branches at debug, like the Connection and Variable lookups. Move the new tests to httpx2, use the server's real 404 body, and name them after the fallback they cover. --------- (cherry picked from commit f05c32e) Co-authored-by: AxelRay <raynaldo.axel@gmail.com> Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
* Lower log level for handled XCom misses * Log handled XCom misses at debug level LazyXComIterator reads offsets until the server returns 404, so every iteration over a mapped upstream ends in a handled miss. Log both XCom not-found branches at debug, like the Connection and Variable lookups. Move the new tests to httpx2, use the server's real 404 body, and name them after the fallback they cover. --------- (cherry picked from commit f05c32e) Co-authored-by: AxelRay <raynaldo.axel@gmail.com> Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
Lower the severity of handled missing XCom responses in the Task SDK client.
Both XCom lookup paths already return handled fallback responses for HTTP 404:
get()returns aNonevalue andget_sequence_item()returnsXCOM_NOT_FOUND. Log these expected misses at debug level, like the Connection and Variable not-found branches, so task logs do not report them as errors. Debug rather than warning becauseLazyXComIteratorkeeps requesting the next offset until it gets a 404, so every iteration over a mapped upstream ends in one of these misses.The new tests cover both 404 fallbacks, which had no client-level test before.
closes: #73175
Was generative AI tooling used to co-author this PR?
Generated-by: Codex CLI following the guidelines