feat(examples): add derived sandbox-code image with run_code env inheritance - #1323
feat(examples): add derived sandbox-code image with run_code env inheritance#1323zyl1121 wants to merge 1 commit into
Conversation
![cubesandboxbot[bot]](https://avatars.githubusercontent.com/in/3653584?s=60&v=4){kind=small}
OverviewThis PR adds a derived The design is sound and well documented: one envd fetch per kernel, sandbox envs applied in a separate background execution before the first user cell (so a first-cell syntax error doesn't suppress initialization), per-call values snapshotted/restored in a background cleanup, and the next execution gated on cleanup. The 12 unit tests are focused and cover the important paths (envd caching, precedence, rollback on invalid values, cleanup waiting, first-cell failures, retry after envd failure). The E2E cases are correctly disabled by default and skip before sandbox creation. Overall this is a solid, well-tested PR. The findings below are robustness issues rather than blockers. Findings1.
|
28fa7a8 to
d1f486d
Compare
| env_vars.update(exec_request.env_vars) | ||
|
|
||
| context = contexts[context_id] | ||
| await context.load_sandbox_envs() |
There was a problem hiding this comment.
Medium — run_code now hard-depends on envd exposing GET /envs, with no graceful degradation.
Every /execute request unconditionally awaits load_sandbox_envs(), and _fetch_sandbox_envs() raises HTTP 502 on any failure (connect error, timeout, non-2xx, invalid payload). Because successes are cached but failures are not, a context whose envd is down — or whose base sandbox-code image predates the /envs endpoint — fails every run_code call with 502, including calls that pass no per-call envs and would work fine on the stock server. There is no runtime opt-out or fallback to the previous behavior.
This is a real behavior difference from the server this image replaces, and the README does not document the envd /envs requirement. Consider:
- documenting the envd version/
/envsrequirement in the README, and/or - degrading gracefully when the fetch fails (log + skip sandbox-env injection, preserving the base server's semantics) instead of failing the execution.
| ) | ||
| env_code = self._build_env_setup_code(env_vars, snapshot_name) | ||
| if env_code: | ||
| await self._run_background(env_code) |
There was a problem hiding this comment.
Medium — env setup runs outside the try/finally that schedules cleanup; a setup failure leaks per-call envs and wedges the context.
await self._run_background(env_code) executes before the try: whose finally: creates the per-call cleanup task. If the setup cell fails — e.g. envd returns an env key containing =/NUL, which makes os.environ[key] = value raise ValueError in the kernel, or the websocket drops mid-setup — the exception propagates out of execute() before the finally runs, so no cleanup is scheduled. Any per-call values already applied leak into the kernel and the _cube_lci_env_snapshot_* global lingers. Since sandbox_envs_applied stays False, the next request retries the same failing setup and 500s again — the context is effectively stuck until a new sandbox. A client disconnect (GeneratorExit) during setup has the same effect.
Consider moving the setup into the try so the finally always schedules cleanup when snapshot_name is set, or at minimum catch setup failures and run the cleanup for whatever partial state was applied.
…ritance Add a derived sandbox-code image because the official lightweight interpreter does not load create-time sandbox environments into `run_code` kernels. Load envd `/envs` once per kernel, apply sandbox and per-call values before unchanged user code, and restore the pre-call kernel environment after each temporary override. Roll back failed setup, keep initialization retryable, omit silent cells from Jupyter history, and expose the envd timeout as configuration. Verify 14 interpreter tests and four opt-in SDK compatibility variants across E2B and CubeSandbox, while the default gate skips all four variants before sandbox creation. Document the required fail-fast envd dependency in English and Chinese. Signed-off-by: zhengyilei <zheng_yilei@qq.com>
d1f486d to
240cd34
Compare
| code: str = Field(..., min_length=1) | ||
| context_id: str | None = None | ||
| language: str | None = None | ||
| timeout: float | None = None |
There was a problem hiding this comment.
ExecuteRequest.timeout is parsed but never used anywhere in this file: KernelContext.execute() has no deadline and _run_background() has no timeout, so the server never bounds an execution or interrupts the kernel. The SDK enforces a client-side read timeout, but the kernel keeps running the cell, so a runaway cell wedges the context — every later run_code queues behind it (this PR adds an extra background env-setup message per call, making that queueing more visible). Either enforce timeout server-side (e.g. asyncio.wait_for around the queue wait plus a kernel interrupt) or drop the dead field.
| env_vars: dict[str, str] | None = None, | ||
| timeout: int = 60, | ||
| ) -> CodeResult: | ||
| result = self._sandbox.run_code(code, envs=env_vars, timeout=timeout) |
There was a problem hiding this comment.
envs=env_vars is now passed unconditionally — even when env_vars is None — with no _accepts_keyword guard, unlike commands.run(user=...) a few lines above which guards against SDK version drift. If the installed e2b_code_interpreter version predates per-call envs support (e2b-dev/code-interpreter#141), every E2B run_code call raises TypeError, breaking the pre-existing non-opt-in run_code tests too, not just the new opt-in ones. Guard with _accepts_keyword(self._sandbox.run_code, "envs") or only pass envs when it's non-None.
| if context is None or client is None: | ||
| raise HTTPException(status_code=404, detail=f"context {context_id} not found") | ||
|
|
||
| await context.close() |
There was a problem hiding this comment.
context.close() runs before the restart POST is confirmed. If the POST fails, the context stays in contexts with a closed websocket, and since close() doesn't reset self.ws to None, the next /execute skips reconnection and send()s on the closed socket (HTTP 500) until the server restarts. Consider verifying the restart response first, or resetting ws/receive_task in close() so a subsequent execute reconnects.
| [ | ||
| f"def {restore_name}():", | ||
| " import os", | ||
| f" snapshot = globals().pop({json.dumps(snapshot_name)}, {{}})", |
There was a problem hiding this comment.
The per-call snapshot is stored as a __main__ global (_cube_lci_env_snapshot_<hex>), and cleanup depends on it surviving until the background cleanup executes. User code that clears the kernel namespace — most realistically IPython %reset — destroys it, making this globals().pop(..., {}) a no-op so the per-call env vars leak into every later execution, contradicting the README's "cannot leak into later executions" guarantee. The variable is also visible in globals() during the user's own execution. Consider tracking the snapshot outside the kernel namespace (e.g. embed the captured key/value pairs in the cleanup code, or keep the snapshot server-side).
3 participants
Motivation
PR #566 made sandbox create-time environment variables available to envd-backed command execution, so
commands.runnow observes create-time environment variables correctly.However,
run_codestill behaves differently. The bundled lightweight code interpreter does not load create-time values from envd, so code executed throughrun_codecannot see environment variables that were provided when the sandbox was created.This means
commands.runandrun_codecan observe different environments in the same sandbox. That is surprising for users who configure API keys, feature flags, or runtime settings at sandbox creation time.This PR adds a derived
sandbox-codeexample image that lets the lightweight code interpreter inherit sandbox create-time environment variables, while keeping per-callrun_codeenvironment overrides temporary and scoped to a single execution.The per-call lifecycle follows e2b-dev/code-interpreter#141: inject environment values into the user execution, clean them up in a background execution, and make the next request wait for cleanup.
Fixes #565.
Follow-up to #566.
What Changed
examples/run-code-env-inheritance/, a derived image based on the officialsandbox-codeimage./opt/lightweight-code-interpreter/server.pyin the derived image.envandenv_varsafter create-time values.ENVD_TIMEOUT, while preserving fail-fast HTTP 502 behavior for unavailable or invalid/envsresponses.run_codeenvironment support.SDK_E2E_RUN_CODE_ENV_INHERITANCE=trueis explicitly set for a compatible template.Validation
Derived-image interpreter tests: 14 passed.
Default SDK compatibility behavior without the opt-in flag: 4 skipped before sandbox creation.
Opt-in SDK compatibility E2E through E2B and CubeSandbox: 4 passed.
Built the derived image, created a template, and verified:
Scope
This PR adds an opt-in derived
sandbox-codeexample image and shared compatibility coverage.The replacement server is based on the lightweight code interpreter already bundled in the official
sandbox-codeimage. Existing execution, context, restart, and timeout behavior is intentionally preserved; this example changes only sandbox environment loading and the per-call environment overlay lifecycle.It does not modify the default
sandbox-codeimage or any existing templates. Users who want this behavior need to build the derived image and create a template from it.Create-time environment variables are read once per Jupyter kernel. This PR does not add generic environment refresh behavior. Per-call
run_codevalues remain temporary overlays for a single execution.The derived interpreter requires envd's
/envsendpoint and fails with HTTP 502 when that dependency is unavailable or returns an invalid payload; it does not fall back to the stock interpreter behavior.The shared environment-inheritance E2E case is disabled by default and skips before sandbox creation unless
SDK_E2E_RUN_CODE_ENV_INHERITANCE=trueis explicitly set for a compatible template.