Skip to content

feat(python-sdk): move the volume content client onto pyqwest#1602

Draft
mishushakov wants to merge 3 commits into
migrate-rest-to-pyqwestfrom
migrate-volume-to-pyqwest
Draft

feat(python-sdk): move the volume content client onto pyqwest#1602
mishushakov wants to merge 3 commits into
migrate-rest-to-pyqwestfrom
migrate-volume-to-pyqwest

Conversation

@mishushakov

Copy link
Copy Markdown
Member

What

Stacked on #1601. Migrates the volume content client (Volume/AsyncVolume file operations) onto pyqwest via its httpx-compatible transport adapter — the same ApiPyqwestTransport + connection-retry stack the REST API client uses after #1601.

Originally deferred from #1601 because Volume.read_file(format="stream") relied on httpx's per-read read timeout as an idle timeout, which the adapter can't express per request (it converts the httpx timeout dict into a whole-request deadline, and the sync adapter doesn't bound body reads at all). Unblocked by pyqwest's transport-constructor read_timeout, which maps to reqwest's ClientBuilder::read_timeout — verified behaviorally (local slow-chunk server, sync + async) to be a true per-read idle timeout: it resets after each successful read, covers body reads, and a healthy stream longer than the timeout completes untouched.

How

  • e2b/volume/client_sync/__init__.py / client_async/__init__.py move to the same ApiPyqwestTransport + connection-retry stack as the API client. Caches become process-global, keyed by (proxy, streaming) — previously one pool per thread (sync) / per event loop (async).
  • Streamed downloads go through a dedicated streaming transport with read_timeout=60s. It can't live on the shared transport: reqwest's read timer keeps running while a request body is sent and while waiting for the response head (verified empirically — a 2.4 s upload against a 0.5 s read_timeout dies mid-send), so a shared read_timeout would cut off write_file uploads and slow unary responses longer than the idle bound. Uploads and unary calls stay on a transport without it, bounded by their whole-request deadlines as before.
  • The 60 s default matches the JS SDK exactly: JS bounds stream start by requestTimeoutMs (60 s default) and idle gaps by streamIdleTimeoutMs ?? requestTimeoutMs; the Python streaming transport's read_timeout bounds the response head and each idle gap at 60 s, resetting on every chunk, wire-only (a slow consumer doesn't trip it — verified).
  • AsyncVolume.read_file keeps honoring an explicit stream_idle_timeout per call, the same way JS honors streamIdleTimeoutMs and feat(python-sdk): migrate envd RPC to the official connectrpc client #1558 bounds stream setup: asyncio.wait_for around each read (response head and every chunk). Explicit values run on the regular transport, so a value above the 60 s transport bound isn't capped by it and 0 disables idle bounding entirely, restoring the previous contract. The sync client keeps the parameter but ignores it — it has no way to interrupt a blocking read into the Rust transport, so its bound must live in the transport.
  • Streamed reads are sent without a per-request timeout so the adapter imposes no whole-request deadline on long downloads; an explicitly passed request_timeout becomes the total-transfer deadline.
  • pyqwest surfaces a stalled read as the builtin TimeoutError; the stream wrappers remap it to httpx.ReadTimeout, keeping the established contract.
  • Proxy narrowing follows feat(python-sdk): move the REST API client onto pyqwest's httpx transport adapter #1601: str, httpx.URL, and reducible httpx.Proxy values work; inexpressible extras raise InvalidArgumentException.

Testing

  • tests/test_volume_client.py rewritten: process-global transport caching (shared across threads and event loops), streaming vs regular transport separation, plus end-to-end streamed reads through Volume.read_file/AsyncVolume.read_file against a local chunked server — a healthy stream longer than the idle timeout completes (proves the timeout resets per read), a mid-body stall raises httpx.ReadTimeout, a slow response head on a non-streamed read is not cut off by the idle bound, and a slow response head on a streamed read is (JS handshake-timeout parity). Async stream_idle_timeout: an explicit value aborts a stall, a value above the transport bound isn't capped by it, and 0 disables idle bounding.
  • Volume content integration tests couldn't run end-to-end (the test team's key gets 403: use of volumes is not enabled); the mock-transport volume content tests and the local-server stream tests cover that path.
  • Lint (ruff), typecheck (ty), unit suite: green.

Usage example

No API changes for the common path:

volume = Volume.connect(volume_id, token=token)
stream = volume.read_file("big.bin", format="stream")     # stalls bounded by the
for chunk in stream:                                      # transport-wide idle read
    ...                                                   # timeout (httpx.ReadTimeout)

volume.read_file("big.bin", format="stream", stream_idle_timeout=5)  # sync: accepted, ignored

async_volume = await AsyncVolume.connect(volume_id, token=token)
stream = await async_volume.read_file(
    "big.bin", format="stream", stream_idle_timeout=5     # async: honored per read,
)                                                         # 0 disables idle bounding

🤖 Generated with Claude Code

mishushakov and others added 3 commits July 24, 2026 01:19
Volume/AsyncVolume file operations join the API client on the
ApiPyqwestTransport + connection-retry stack; transport caches become
process-global keyed by proxy. httpx.Proxy values reduce to plain proxy
URLs; stream_idle_timeout is accepted but a no-op at this point (bounded
by the transport in the next commits).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n a dedicated transport

reqwest's read timer keeps ticking while a request body is sent and while
waiting for the response head, so a shared read_timeout would kill slow
uploads; streamed reads get their own transport with read_timeout=60s
(JS SDK parity), uploads and unary calls stay unbounded-per-read.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ead_file

asyncio.wait_for around each read (response head and every chunk); explicit
values run on the regular transport so values above the 60s transport bound
aren't capped and 0 disables idle bounding. Sync keeps the parameter but
ignores it — it cannot interrupt a blocking read into the Rust transport.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Jul 23, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 8c92831

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@e2b/python-sdk Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@cla-bot cla-bot Bot added the cla-signed label Jul 23, 2026
@cursor

cursor Bot commented Jul 23, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches volume file I/O, connection pooling, and timeout semantics (including async-only stream_idle_timeout behavior); well covered by new tests but behavior changes could affect long downloads and stalled streams.

Overview
Volume content HTTP (Volume / AsyncVolume file ops) now uses the same pyqwest httpx transport and connection-retry stack as the REST API client instead of per-thread or per-event-loop httpx pools. Transports are cached process-wide per proxy and a separate streaming pool that sets a 60s idle read_timeout (resets per chunk; stalls surface as httpx.ReadTimeout).

Streamed read_file no longer relies on httpx per-read timeouts: default idle bounding is on the streaming transport; explicit request_timeout on a stream becomes a whole-transfer deadline. AsyncVolume still honors per-call stream_idle_timeout via asyncio.wait_for (including 0 to disable), using the non-streaming transport when set; sync stream_idle_timeout is documented as ignored. Tests cover global transport sharing and local chunked-server stream timeout behavior.

Reviewed by Cursor Bugbot for commit 8c92831. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions

Copy link
Copy Markdown
Contributor

Package Artifacts

Built from c397d0f. Download artifacts from this workflow run.

JS SDK (e2b@2.35.4-migrate-volume-to-pyqwest.0):

npm install ./e2b-2.35.4-migrate-volume-to-pyqwest.0.tgz

CLI (@e2b/cli@2.15.1-migrate-volume-to-pyqwest.0):

npm install ./e2b-cli-2.15.1-migrate-volume-to-pyqwest.0.tgz

Python SDK (e2b==2.34.0+migrate.volume.to.pyqwest):

pip install ./e2b-2.34.0+migrate.volume.to.pyqwest-py3-none-any.whl

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 8c92831. Configure here.

break
yield chunk
finally:
await stream_cm.__aexit__(None, None, None)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stream enter timeout skips cleanup

Medium Severity

When an explicit stream_idle_timeout is set, asyncio.wait_for wraps stream_cm.__aenter__(). On timeout it cancels that enter before the inner try/finally, so __aexit__ never runs and the httpx stream generator is left suspended. That can leak a connection from the process-global pool on slow response heads.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 8c92831. Configure here.

@mishushakov

Copy link
Copy Markdown
Member Author

from other PR:

Non-stream AsyncVolume.read_file still passes FILE_TIMEOUT as the per-request timeout. Through the async pyqwest adapter that is a whole-transfer deadline, so a healthy large bytes/text download can be killed after one hour even though the stream path carefully avoids this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant