Repository navigation
deps: migrate to httpx2 - #5704
Conversation
1d761ad to
f2100a4
Compare
collindutter
changed the title
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
62f0ea2 to
d0f051c
Compare
3cf7d29 to
f808816
Compare
f808816 to
f649e06
Compare
hutchinson
left a comment
hutchinson
left a comment
There was a problem hiding this comment.
These comments were generated with AI assistance and reviewed before posting.
Reviewed as an evidence review: claims below are measurements, run against head f649e06 in a scratch checkout.
What I ran
- Verified the mechanism in installed dependency source (
httpx22.12.0,httpx0.28.1,truststore0.10.4). - Mutation-tested the new tests: checked out
main'ssrc/, kept the newtests/unit/utils/test_http_file_patch.py, ran it. 5 of 11 fail against the base implementation, 6 pass, 0.42s. The 5 failures are exactly the[httpx2]parametrizations; the[httpx]andrequestsparams pass against both, which is the right shape for characterising preserved behavior. - Ran the affected suites at head: 958 passed, 3 skipped, 8.1s.
The migration checks out
httpx2 2.12.0 exports every symbol the diff references. No dangling references to the deleted _original_httpx_* globals remain. The dependency split in the description matches what resolves: mcp 12 files httpx2 / 0 httpx, openai 106 / 13, huggingface_hub 0 / 18, so keeping httpx is load-bearing.
On the CHANGELOG entry
The TLS claim is accurate, including the "matching the app" clause. Measured create_ssl_context(verify=True, trust_env=False):
| Context | httpx |
httpx2 |
|---|---|---|
Subprocess (no inject_into_ssl) |
ssl.SSLContext (certifi) |
truststore._api.SSLContext |
App (after inject_into_ssl) |
truststore._api.SSLContext |
truststore._api.SSLContext |
truststore.inject_into_ssl() lives in griptape-nodes-app (app.py:1285) and was removed from the engine in fbb89d4f. Engine subprocess entry points call install_file_url_support() but never inject_into_ssl, so httpx2's default closes that gap as a side effect.
Adjacent scope, not blocking this PR
Every request routes through _route_request twice: counting invocations for a single httpx2.get gives 2, and for a cloud asset URL that is two signing attempts per request (measured with a stubbed signer). This is pre-existing. The same probe against main with httpx.get also gives 2. Worth a follow-up ticket rather than a change here, though this PR doubles the number of patched modules, so the cost now applies on both paths.
| """Patched httpx.patch that handles file:// URLs.""" | ||
| return _patched_httpx_request("PATCH", url, **kwargs) | ||
| Client verb methods (`Client.get`, etc.) route through `Client.request`, so patching | ||
| `request` covers them. Module-level verbs call the unpatched internal `request`, so each |
There was a problem hiding this comment.
[question] This says module-level verbs call the unpatched internal request, but I think it is the other way round, and that would make the _HTTP_VERBS loop below redundant.
httpx2.head is still _api.head, and api.head.__globals__['request'] resolves to the patched object, so the module verbs pick up the patch from line 303 on their own. Measured, patching only Client.request and leaving the module verbs alone:
control (unpatched) api.request -> RAISED UnsupportedProtocol
Client.request patched only; api.request -> FileHttpxResponse 200
Client.request patched only; api.head -> FileHttpxResponse 200
The control raising is what makes the other two meaningful. This is also why HEAD and OPTIONS work despite being absent from _HTTP_VERBS.
Worth correcting mainly because this docstring is what the next reader will trust when deciding whether _HTTP_VERBS can be extended or dropped.
| return await original_async_client_request(self, method, target, **kwargs) | ||
|
|
||
| setattr(module, "request", patched_request) # noqa: B010 | ||
| for verb in _HTTP_VERBS: |
There was a problem hiding this comment.
[nit] Follows from the docstring comment above: if the module verbs already resolve request through the patched global, this loop and _HTTP_VERBS can both go. Happy to be wrong if there is a case I did not hit.
hutchinson
left a comment
hutchinson
left a comment
There was a problem hiding this comment.
Approving as nothing is blocking.
httpx2is Pydantic's drop-in fork ofhttpx0.28.1, and our dependencies are moving to it (openai, mcp, huggingface_hub). The migration guide supports installing both side by side, sohttpxstays for code that still deals in its types:huggingface-hub1.x errors and node libraries that import it.Next steps to drop
httpx:huggingface-hubto 2.0 and movemodel_download_errorstohttpx2.httpxin the standard library'spip_dependencies.httpxinhttp_file_patchonly when installed, then remove it frompyproject.toml.