Add pyreqwest-backed transports - #1088
Conversation
|
Docs preview: https://1d93fd0d-httpx2-docs.pydantic.workers.dev |
Merging this PR will not alter performance
Comparing 
Footnotes
|
| "AsyncByteStream", | ||
| "AsyncClient", | ||
| "AsyncHTTPTransport", | ||
| "AsyncPyreqwestTransport", |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8874107aa4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| def handle_request(self, request: Request) -> Response: | ||
| content = request.read() | ||
| builder = self._client.request(request.method, str(request.url)).headers(request.headers.multi_items()) |
There was a problem hiding this comment.
When a caller supplies a client- or request-level timeout, build_request stores it in request.extensions["timeout"], but this transport never reads that extension when constructing the pyreqwest request. Consequently values such as timeout=0.01 or timeout=None have no effect and the request instead uses whatever timeout was configured on the underlying pyreqwest client; the async path has the same omission.
Useful? React with 👍 / 👎.
| except BaseException: | ||
| exit_stack.close() | ||
| raise |
There was a problem hiding this comment.
When connection establishment or sending fails inside build_streamed(), this handler re-raises the pyreqwest exception unchanged. Callers that follow the documented HTTPX exception API and catch httpx2.RequestError or httpx2.TransportError therefore miss ordinary network failures when opting into this transport; the async implementation behaves identically, so both paths should map backend exceptions as the default HTTP transport does.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/httpx2/httpx2/_transports/pyreqwest.py">
<violation number="1" location="src/httpx2/httpx2/_transports/pyreqwest.py:77">
P2: Streaming uploads are fully buffered before the connection opens, so large or unbounded request iterators lose streaming/backpressure and can exhaust memory. Pass a streaming request body to pyreqwest rather than calling `Request.read()` here.</violation>
<violation number="2" location="src/httpx2/httpx2/_transports/pyreqwest.py:78">
P2: Configured `Client(timeout=...)` and per-request timeouts are ignored by this transport; stalled pyreqwest calls use pyreqwest's own defaults instead. Translate `request.extensions["timeout"]` into the pyreqwest request/client timeout configuration before starting the request.</violation>
<violation number="3" location="src/httpx2/httpx2/_transports/pyreqwest.py:93">
P2: Network failures from pyreqwest are re-raised as raw exceptions rather than being mapped to `httpx2.TransportError` subclasses (e.g., `ConnectError`, `ReadTimeout`). Callers relying on the standard httpx2 exception hierarchy—`except httpx2.TransportError`—will not catch connection failures or timeouts when using this transport, which is inconsistent with how the default `HTTPTransport` behaves.
Consider wrapping the `builder.build_streamed()` call (and equivalently in the async path) with a try/except that catches pyreqwest-specific exceptions and re-raises them as the appropriate `httpx2` transport error, e.g.:
```python
except SomePyreqwestConnectionError as exc:
raise httpx2.ConnectError(str(exc)) from exc
```</violation>
<violation number="4" location="src/httpx2/httpx2/_transports/pyreqwest.py:109">
P2: Async streaming uploads are fully buffered before the connection opens, so large or unbounded async iterators lose streaming/backpressure and can exhaust memory. Pass a streaming request body to pyreqwest rather than calling `Request.aread()` here.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ) | ||
| except BaseException: | ||
| exit_stack.close() | ||
| raise |
There was a problem hiding this comment.
P2: Network failures from pyreqwest are re-raised as raw exceptions rather than being mapped to httpx2.TransportError subclasses (e.g., ConnectError, ReadTimeout). Callers relying on the standard httpx2 exception hierarchy—except httpx2.TransportError—will not catch connection failures or timeouts when using this transport, which is inconsistent with how the default HTTPTransport behaves.
Consider wrapping the builder.build_streamed() call (and equivalently in the async path) with a try/except that catches pyreqwest-specific exceptions and re-raises them as the appropriate httpx2 transport error, e.g.:
except SomePyreqwestConnectionError as exc:
raise httpx2.ConnectError(str(exc)) from excPrompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/httpx2/httpx2/_transports/pyreqwest.py, line 93:
<comment>Network failures from pyreqwest are re-raised as raw exceptions rather than being mapped to `httpx2.TransportError` subclasses (e.g., `ConnectError`, `ReadTimeout`). Callers relying on the standard httpx2 exception hierarchy—`except httpx2.TransportError`—will not catch connection failures or timeouts when using this transport, which is inconsistent with how the default `HTTPTransport` behaves.
Consider wrapping the `builder.build_streamed()` call (and equivalently in the async path) with a try/except that catches pyreqwest-specific exceptions and re-raises them as the appropriate `httpx2` transport error, e.g.:
```python
except SomePyreqwestConnectionError as exc:
raise httpx2.ConnectError(str(exc)) from exc
```</comment>
<file context>
@@ -0,0 +1,129 @@
+ )
+ except BaseException:
+ exit_stack.close()
+ raise
+
+ def close(self) -> None:
</file context>
|
|
||
| def handle_request(self, request: Request) -> Response: | ||
| content = request.read() | ||
| builder = self._client.request(request.method, str(request.url)).headers(request.headers.multi_items()) |
There was a problem hiding this comment.
P2: Configured Client(timeout=...) and per-request timeouts are ignored by this transport; stalled pyreqwest calls use pyreqwest's own defaults instead. Translate request.extensions["timeout"] into the pyreqwest request/client timeout configuration before starting the request.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/httpx2/httpx2/_transports/pyreqwest.py, line 78:
<comment>Configured `Client(timeout=...)` and per-request timeouts are ignored by this transport; stalled pyreqwest calls use pyreqwest's own defaults instead. Translate `request.extensions["timeout"]` into the pyreqwest request/client timeout configuration before starting the request.</comment>
<file context>
@@ -0,0 +1,129 @@
+
+ def handle_request(self, request: Request) -> Response:
+ content = request.read()
+ builder = self._client.request(request.method, str(request.url)).headers(request.headers.multi_items())
+ if content:
+ builder = builder.body_bytes(content)
</file context>
| self._close_client = client is None or close_client | ||
|
|
||
| async def handle_async_request(self, request: Request) -> Response: | ||
| content = await request.aread() |
There was a problem hiding this comment.
P2: Async streaming uploads are fully buffered before the connection opens, so large or unbounded async iterators lose streaming/backpressure and can exhaust memory. Pass a streaming request body to pyreqwest rather than calling Request.aread() here.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/httpx2/httpx2/_transports/pyreqwest.py, line 109:
<comment>Async streaming uploads are fully buffered before the connection opens, so large or unbounded async iterators lose streaming/backpressure and can exhaust memory. Pass a streaming request body to pyreqwest rather than calling `Request.aread()` here.</comment>
<file context>
@@ -0,0 +1,129 @@
+ self._close_client = client is None or close_client
+
+ async def handle_async_request(self, request: Request) -> Response:
+ content = await request.aread()
+ builder = self._client.request(request.method, str(request.url)).headers(request.headers.multi_items())
+ if content:
</file context>
| self._close_client = client is None or close_client | ||
|
|
||
| def handle_request(self, request: Request) -> Response: | ||
| content = request.read() |
There was a problem hiding this comment.
P2: Streaming uploads are fully buffered before the connection opens, so large or unbounded request iterators lose streaming/backpressure and can exhaust memory. Pass a streaming request body to pyreqwest rather than calling Request.read() here.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/httpx2/httpx2/_transports/pyreqwest.py, line 77:
<comment>Streaming uploads are fully buffered before the connection opens, so large or unbounded request iterators lose streaming/backpressure and can exhaust memory. Pass a streaming request body to pyreqwest rather than calling `Request.read()` here.</comment>
<file context>
@@ -0,0 +1,129 @@
+ self._close_client = client is None or close_client
+
+ def handle_request(self, request: Request) -> Response:
+ content = request.read()
+ builder = self._client.request(request.method, str(request.url)).headers(request.headers.multi_items())
+ if content:
</file context>
1 participant
Summary
Adds optional sync and async transports backed by pyreqwest/reqwest:
httpx2.PyreqwestTransporthttpx2.AsyncPyreqwestTransporthttpx2[pyreqwest]extraThe transports disable pyreqwest redirect following and cookie storage so httpx2 keeps owning those higher-level behaviors.
Test plan
uv run --with pyreqwest ruff check src/httpx2/httpx2/_transports/pyreqwest.py tests/httpx2/test_pyreqwest_transport.pyuv run --with pyreqwest pytest tests/httpx2/test_pyreqwest_transport.py -q