Conversation
Implements CON-1523, CON-1524, CON-1525, and CON-1526. Based on REST migration PR #584. Keep this draft local until that PR is merged. Includes managed ownership, bounded startup handling, closeable SSE streams, and cancellation-safe synchronous and asynchronous resource cleanup.
runpod-Henrik
left a comment
There was a problem hiding this comment.
QA review of the SDK surface, diff only.
Good call on ExecResult(output=data["output"], error=data.get("error")). A successful exec omits the error key entirely rather than sending null, despite the OpenAPI text describing it as "null when the command succeeded" — a strict is None comparison there reported every real success as a failure in another consumer, with green unit tests the whole time. Using .get() sidesteps it. Worth a comment on that line so nobody "tidies" it into an is None check later.
Four things.
1. AsyncioSandbox vs the specified AsyncSandbox
The design doc names the async entry point AsyncSandbox, and the stated reason is specific rather than aesthetic: it matches E2B's exact class name, so developers arriving from E2B or Daytona don't have to relearn a term. AsyncioSandbox gives up precisely that benefit.
The PR is also internally split on it — __all__ exports AsyncioSandbox alongside AsyncSandboxLogStream, so both prefixes ship in the same namespace. Whichever way this goes, the two should agree.
This is cheap to change now and a breaking change later, which is why it's worth settling before merge rather than after.
2. check: bool = False inverts the documented default
The design doc lists the raise-on-failure default as an open question and records the lean as raising. This ships the opposite: a command that exits non-zero returns an ExecResult and execution continues unless the caller opted in.
Two honest sides to this. check is the better name — it matches subprocess.run, and Python developers will read it correctly with no docs. But the default matters more than the name for this product: the entire use case is executing code a model just wrote, where silently proceeding past a failed command is how an agent ends up building on a step that didn't happen. E2B and the other SDKs in this category raise by default.
Not asking for a specific answer, just that the default is chosen deliberately and recorded, rather than inherited from subprocess.
3. This is the public repo
Shipping sandbox support into public runpod-python while the feature is email-gated and dark in prod means users can discover, install and build against an API that isn't generally available yet. That's happened before on another product and produced support load plus pressure not to change the surface — which is exactly the surface still being decided in points 1 and 2.
Options are a private repo until GA, or release tagging that keeps it off the default install. Either is fine; drifting into GA by publication is the one to avoid.
4. data["output"] is a hard index on a 200
output is required in the schema, so this is spec-correct. The failure mode it misses is a malformed 200 that isn't the API at all — a proxy error page, an HTML interstitial — which raises KeyError rather than anything a caller can act on. Another consumer of this endpoint hit exactly that shape on create. A .get("output", "") with an explicit error, or letting the existing response validation catch it, turns a confusing traceback into a real message.
Not gaps, for the record
No upload_file / download_file: there are no file-transfer endpoints in the REST contract yet, so that's blocked upstream rather than missing here. Same for anything depending on streaming exec.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
It introduces a couple of correctness/compatibility concerns in newly added public API and transport code that should be addressed before merging.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (2)
What changed in this PR
This PR introduces first-class sandbox support to the Runpod Python SDK by adding parallel sync (Sandbox) and async (AsyncioSandbox) APIs that share the same lifecycle semantics, command execution behavior, and log streaming facilities over REST API v2.
Changes:
- Added async sandbox handle (
AsyncioSandbox) with lifecycle management, exec semantics (check=True), and resumable SSE log streaming. - Added sync wrapper (
Sandbox) backed by a dedicated background asyncio loop to safely reuse aiohttp sessions while presenting a blocking API. - Added integration-style tests and README documentation for sandbox creation, borrowed handles (
get/list), exec retry rules, and log streaming.
| File | Description |
|---|---|
| tests/test_sandbox.py | Adds HTTP-level regression tests for sandbox lifecycle edge cases, exec retry semantics, and SSE log streaming behavior. |
| tests/test_init.py | Removes a brittle __all__ exact-match assertion that would conflict with newly exported sandbox symbols. |
| runpod/sandbox/sync.py | Implements the blocking Sandbox facade and sync log iterator over a stable background event loop. |
| runpod/sandbox/models.py | Introduces typed models for sandbox snapshots, exec results, log events, and sandbox-specific exceptions. |
| runpod/sandbox/asyncio.py | Implements AsyncioSandbox lifecycle, startup retry policy, exec behavior, and typed async log streaming. |
| runpod/sandbox/__init__.py | Exports sandbox public API surface from the runpod.sandbox package. |
| runpod/api/sandboxes.py | Adds aiohttp-based REST + SSE transport for sandboxes (create/get/list/terminate/exec/logs). |
| runpod/api/rest.py | Extends URL building to support custom base URLs and centralizes HTTP-status-to-error mapping for reuse. |
| runpod/__init__.py | Exposes Sandbox and AsyncioSandbox from the top-level runpod package and updates __all__. |
| README.md | Documents sync/async sandbox usage, lifecycle semantics, exec/check behavior, and log streaming/resume. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
runpod-Henrik
left a comment
There was a problem hiding this comment.
Delta review, since 2026-09-16 (re-checked at d5abe4b11)
I ran tests/test_sandbox.py on the new head (Python 3.12, aiohttp 3.14.3): 23 passed. The suite runs against a real aiohttp REST peer rather than mocking the SDK transport.
Earlier findings
AsyncioSandboxvsAsyncSandbox: resolved. The README now gives the reason: it follows the SDK's existingAsyncioEndpoint/AsyncioJobexports, whichrunpod/__init__.pydoes export. Consistent with the rest of the SDK. One piece is still open:runpod.sandbox.__all__still exportsAsyncSandboxLogStreamalongsideAsyncioSandbox, so both prefixes ship in the same namespace. There are also two classes namedAsyncSandboxLogStream, inrunpod/api/sandboxes.py:86andrunpod/sandbox/asyncio.py:441, and a user importing the wrong one getsisinstancemismatches. Renaming the exported one toAsyncioSandboxLogStreamwould settle both.check=Falsedefault: addressed as asked. The choice is now deliberate and documented in the README and theexecdocstring ("Likesubprocess.run…"), including that HTTP errors and malformed responses raise regardless ofcheck.- Public repo while the feature is gated: addressed by documentation. The README now says sandboxes are "an access-controlled preview … installing the SDK does not grant access". Whether that's enough before GA is a product decision; it's at least clear to anyone who finds it.
- Hard
data["output"]index: fixed. Non-JSON bodies, non-object JSON, and missing or non-stringoutputall raiseQueryError, without retrying the exec. The new tests cover{},{"output": null},[], and an HTML<html>proxy error</html>page returned with a 200, and all four pass.
New in this commit
Issue: SandboxCompute.vcpu_count is now typed int, but the API returns a number
The REST v2 spec (spec/openapi.yaml in runpod/rphttp2) declares SandboxCompute.vcpuCount as type: number, and the server builds it as float32. SandboxInfo assigns compute["vcpuCount"] without converting it, so at runtime callers get whatever the JSON contains. But type checkers and IDEs will now tell them it's an int. If fractional vCPU values are ever returned, the annotation will be wrong. Suggest keeping it float, as it was before this commit, or converting explicitly if an integer is really guaranteed.
Question: _retry_connection guard with no ceiling on aiohttp
if hasattr(self._session, "_retry_connection") prevents a crash if aiohttp removes this private attribute. But then the "don't replay ambiguous GET/DELETE" protection switches off without any warning. requirements.txt only sets a floor (aiohttp[speedups] >= 3.14.3). A small test asserting session._retry_connection is False would make an aiohttp change show up as a test failure rather than as silent retries.
Nits
- The CodeQL/code-quality annotations (mostly
except BaseExceptionin cleanup paths) look like intentional cleanup-then-reraise handling. A one-line comment at those sites, or resolving the alerts, would stop the next reviewer re-raising them.
Verdict: PASS WITH NITS. All four earlier points are addressed, and the new validation is tested against a real peer. The vcpu_count type is worth reverting before merge.
🤖 Reviewed by Henrik's AI-Powered Bug Finder


This brings sandbox support to the Python SDK, with a regular
SandboxAPI and an asyncAsyncioSandboxAPI. Both share the same lifecycle and execution logic.use a context manager for a short-lived sandbox:
The async version works the same way:
To work with a sandbox that's already running, use
get(sandbox_id)orlist(...). Those handles only close their local connections when you leave the context. they don't terminate the sandbox. You can callterminate()explicitly when you're finished with it.exec()returns command output, andcheck=Trueraises on command failure while keeping any partial output.logs()streams container or system logs and supports resuming with an event'sidthroughlast_event_id. Use a context manager around the log stream too if you might stop reading early.While working on this, I discovered an edge case in log streaming on host that should be addressed as well: CON-1557 / runpod/host#2820.
Linear: CON-1523, CON-1524, CON-1525, CON-1526