Repository navigation
Migrate Task SDK to httpx2 - #73116
Merged
hussein-awala merged 19 commits intoOct 14, 2026
Merged
Conversation
1 task done
zach-overflow
marked this pull request as ready for review
September 14, 2026 11:29
zach-overflow
requested review from
amoghrajesh,
ashb,
bugraoz93,
gopidesupavan,
jason810496,
jscheffl,
kaxil and
potiuk
as code owners
September 14, 2026 11:29
potiuk
reviewed
Sep 14, 2026
zach-overflow
force-pushed
the
zach.gottesman/task-sdk-httpx2
branch
2 times, most recently
from
September 16, 2026 08:37
a6203f6 to
388ad9a
Compare
amoghrajesh
reviewed
Sep 20, 2026
amoghrajesh
left a comment
Contributor
There was a problem hiding this comment.
Good reason to support httpx2 since httpx is not actively maintained, most are nits, but the compat concern needs addressing.
Contributor
|
Quickest fix: git fetch upstream main && git rebase upstream/main
rm uv.lock && uv lock
git add uv.lock && git rebase --continue
git push --force-with-leaseAutomated nudge — ignore if you're not ready to rebase. This comment is updated in place on future |
zach-overflow
force-pushed
the
zach.gottesman/task-sdk-httpx2
branch
2 times, most recently
from
September 28, 2026 13:30
4e1b42c to
0c83ff2
Compare
zach-overflow
force-pushed
the
zach.gottesman/task-sdk-httpx2
branch
from
October 1, 2026 13:23
0c83ff2 to
87852a1
Compare
5 of 7 tasks
zach-overflow
force-pushed
the
zach.gottesman/task-sdk-httpx2
branch
from
October 2, 2026 13:42
87852a1 to
c992360
Compare
This was referenced Oct 3, 2026
Both HTTP stacks can coexist while compatibility jobs use older SDK distributions. The migration needs regression coverage for its logging behavior without exposing development-only helpers as public APIs.
Tests added on main for in-process shutdown still referenced httpx, which this branch no longer imports in the module.
httpx2 verifies against the system trust store via truststore, so keeping certifi for the Execution API client would add a direct dependency only to maintain a second, different set of trusted roots in the same worker.
Ruff already enforces banned imports, so a separate ast-grep hook and its extra pre-commit dependency are unnecessary. extend-banned-api adds to the root banned-api table instead of replacing it.
The test arrived on main after the Task SDK moved to httpx2 and still referenced httpx, which the module no longer imports.
Mocking load_verify_locations while also writing a CA file to disk tested neither path convincingly. Keeping the certificate-generation imports inside the fixture limits them to the one place that needs them.
Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com>
The helper arrived on main after the Task SDK moved to httpx2 and still referenced httpx, which the module no longer imports.
The test arrived on main after the Task SDK moved to httpx2 and still referenced httpx, which the module no longer imports.
The tests arrived on main after the Task SDK moved to httpx2; the client rejects httpx transports.
zach-overflow
force-pushed
the
zach.gottesman/task-sdk-httpx2
branch
from
October 13, 2026 13:24
c43ea9f to
b312d3d
Compare
1 task done
1 task done
Contributor
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.
|
ashb
pushed a commit
that referenced
this pull request
Oct 16, 2026
vatsrahul1001
pushed a commit
that referenced
this pull request
Oct 20, 2026
rjgoyln
added a commit
to rjgoyln/airflow
that referenced
this pull request
Oct 23, 2026
The Task SDK moved from httpx to httpx2 (apache#73116) after this branch was written, so the rebased helper and its tests referred to a module that is no longer imported there.
1 task done
henry3260
added a commit
that referenced
this pull request
Oct 25, 2026
* Fix airflowctl tracebacks on JSON errors behind a reverse proxy airflowctl compared the error response content-type against the exact string "application/json". Any reverse proxy, ingress, or API gateway that returns "application/json; charset=utf-8" — or emits its own "application/problem+json" error — fell outside that comparison, so ServerResponseError was never constructed and the friendly "not found" messages in the dags and tasks commands were skipped. The bare httpx.HTTPStatusError raised instead is not handled by safe_call_command, so users saw a raw traceback. Airflow's own API server does not send a charset, which is why this only surfaces once something sits in front of it. Media types are case-insensitive and may carry parameters (RFC 9110), so the header has to be parsed rather than compared whole. Matching more responses also routes bodies into the error path that are valid JSON but not objects, which dict() rejected with a ValueError from that same unprotected path, and bodies that do not decode at all. * Move media type rationale from docstring into an inline comment * Apply the same JSON media type handling to the Task SDK client The Task SDK client carries the identical exact-string content-type comparison, so a task talking to the API server through a proxy that declares a charset loses the server's error detail and, because the raised exception is then a plain httpx.HTTPStatusError, no longer hits the status-specific handling in the supervisor and the task runner. * Cross-reference the twin JSON media type checks The airflowctl and Task SDK copies are the same three lines with different comments, and they already drifted in the commits that created them. A pointer in each makes the next person notice there are two. * Add newsfragment for the API client error handling fix Requested during review: the Task SDK half of the change ships in airflow-core, and both effects are visible to users running behind a proxy. * Use httpx2 in the Task SDK JSON media type check The Task SDK moved from httpx to httpx2 (#73116) after this branch was written, so the rebased helper and its tests referred to a module that is no longer imported there. * Describe the airflowctl symptom as it is on main now Since #73307 airflowctl catches the bare HTTPStatusError and prints one line instead of a traceback, so the newsfragment overstated it; the server's own message is still what gets lost. * Keep only the airflowctl test cases the fix changes The bare media type and the 404/500 axis repeat what test_error_parsing_with_unread_body already covers, and dict() of an object body worked before; none of them fail without the fix. * Explain the non-object log guard by what it protects The previous comment described the dict() call the guard replaced, which is no longer in the code. * Limit the newsfragment to the Task SDK fix airflow-core's release notes ship only the Task SDK side; airflowctl's changelog is built from git log. The task runner never handled ServerResponseError, so it is no longer named. * Rename the newsfragment to this PR's number * Shorten the media type comments The longer versions restated what the diff and the PR already explain, and the two _has_json_body copies now carry the same note. --------- Co-authored-by: Henry Chen <henrychen@apache.org>
potiuk
pushed a commit
that referenced
this pull request
Oct 25, 2026
…reverse proxy (#75493) (#75622) * Fix airflowctl tracebacks on JSON errors behind a reverse proxy airflowctl compared the error response content-type against the exact string "application/json". Any reverse proxy, ingress, or API gateway that returns "application/json; charset=utf-8" — or emits its own "application/problem+json" error — fell outside that comparison, so ServerResponseError was never constructed and the friendly "not found" messages in the dags and tasks commands were skipped. The bare httpx.HTTPStatusError raised instead is not handled by safe_call_command, so users saw a raw traceback. Airflow's own API server does not send a charset, which is why this only surfaces once something sits in front of it. Media types are case-insensitive and may carry parameters (RFC 9110), so the header has to be parsed rather than compared whole. Matching more responses also routes bodies into the error path that are valid JSON but not objects, which dict() rejected with a ValueError from that same unprotected path, and bodies that do not decode at all. * Move media type rationale from docstring into an inline comment * Apply the same JSON media type handling to the Task SDK client The Task SDK client carries the identical exact-string content-type comparison, so a task talking to the API server through a proxy that declares a charset loses the server's error detail and, because the raised exception is then a plain httpx.HTTPStatusError, no longer hits the status-specific handling in the supervisor and the task runner. * Cross-reference the twin JSON media type checks The airflowctl and Task SDK copies are the same three lines with different comments, and they already drifted in the commits that created them. A pointer in each makes the next person notice there are two. * Add newsfragment for the API client error handling fix Requested during review: the Task SDK half of the change ships in airflow-core, and both effects are visible to users running behind a proxy. * Use httpx2 in the Task SDK JSON media type check The Task SDK moved from httpx to httpx2 (#73116) after this branch was written, so the rebased helper and its tests referred to a module that is no longer imported there. * Describe the airflowctl symptom as it is on main now Since #73307 airflowctl catches the bare HTTPStatusError and prints one line instead of a traceback, so the newsfragment overstated it; the server's own message is still what gets lost. * Keep only the airflowctl test cases the fix changes The bare media type and the 404/500 axis repeat what test_error_parsing_with_unread_body already covers, and dict() of an object body worked before; none of them fail without the fix. * Explain the non-object log guard by what it protects The previous comment described the dict() call the guard replaced, which is no longer in the code. * Limit the newsfragment to the Task SDK fix airflow-core's release notes ship only the Task SDK side; airflowctl's changelog is built from git log. The task runner never handled ServerResponseError, so it is no longer named. * Rename the newsfragment to this PR's number * Shorten the media type comments The longer versions restated what the diff and the PR already explain, and the two _has_json_body copies now carry the same note. --------- (cherry picked from commit 352ca60) Co-authored-by: rjgoyln <151457491+rjgoyln@users.noreply.github.com> Co-authored-by: Henry Chen <henrychen@apache.org>
1 task done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
apache-airflow-task-sdk's dependency onhttpx→httpx2. The Execution APIClientis now anhttpx2.Client.truststoreas an explicittask-sdkdependency — replacing the previously usedcertifiwhich was previously implicitly installed viahttpx, buthttpx2depends ontruststoreinstead. Details for this noted in news-fragment.InProcessExecutionAPI.transport/.atransportinairflow-coretohttpx2, plus the call sites that build or drive them. Objects don't cross thehttpx/httpx2boundary, and the only consumers of those transports are Task SDK clients. This addshttpx2as anairflow-coreruntime dependency; the rest ofairflow-corestays onhttpx.devel-common's in-process task-run helpers resolve the HTTP stack from the installed Task SDKClientrather than hardcoding one. Compat jobs pair this repo'sdevel-commonwith a released Airflow whose SDK is still onhttpx, and handing such a client anhttpx2transport trips httpx's own stream-type assertion.shared/loggingto suppress and mute both stacks, sinceairflow-coreand the Task SDK are now on different ones.ast-greppre-commit check to prevent re-introducinghttpxin the Task SDK, mirroring the providers one.Why?
httpxusage forhttpx2#70522httpx2#72111Was generative AI tooling used to co-author this PR?
task-sdkhttpxusage and it did the import replacements and dependency updates.devel-commontest helpers, prek scripts) and migrated those call sites, then made thedevel-commonhelpers stack-agnostic after the compat jobs caught the released-SDK case.ast-greprule and verified it fires.