feat(sdk): fork via v2 endpoint, leave the count limit to the API, add keepMemory - #1918
devin-ai-integration[bot] wants to merge 3 commits into
Conversation
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
🦋 Changeset detectedLatest commit: 57811c6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
Package ArtifactsBuilt from 401513d. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.52.1-devin-1790766635-fork-v2.0.tgzCLI ( npm install ./e2b-cli-2.21.1-devin-1790766635-fork-v2.0.tgzCode Interpreter JS SDK ( npm install ./e2b-code-interpreter-2.8.1-devin-1790766635-fork-v2.0.tgzDesktop JS SDK ( npm install ./e2b-desktop-2.4.1-devin-1790766635-fork-v2.0.tgzPython SDK ( pip install ./e2b-2.52.0+devin.1790766635.fork.v2-py3-none-any.whlCode Interpreter Python SDK ( pip install ./e2b_code_interpreter-2.10.1+devin.1790766635.fork.v2-py3-none-any.whlDesktop Python SDK ( pip install ./e2b_desktop-2.6.0+devin.1790766635.fork.v2-py3-none-any.whl |
There was a problem hiding this comment.
TASTE.md review of the fork v2 migration.
Checked: T-1/T-2 (JS, sync Python, and async Python stay in step: all three move to v2 and drop the cap together), T-3a, T-6, T-18 (SandboxForkRequestV2 stays internal), T-33, T-46 (signal is still passed through getSignal), T-52, T-59, T-60/T-61, T-62, T-69, T-74.
Violations: 1 (T-62/T-69), in 3 places. The count docs in JS and in both Python mirrors no longer say which error an over-limit count produces. Details are in the inline comments.
The main change fits T-52. Removing MAX_FORK_COUNT / validate_fork_count means the SDK no longer copies a backend business rule that the per-team max-fork-count flag has already outgrown. The CLI keeps its --count positive-integer parse, which counts as input parsing that produces a clear error (T-52b), not a mirrored limit.
Non-blocking note: the new timeout / timeoutMs docs state the server default ("300 seconds" / "5 minutes"). This isn't validation, so it's not counted as a violation. It is the same kind of copied server value that the count docs now avoid, though, and it will go stale if the API default changes.
|
check comments |
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Co-Authored-By: mish@e2b.dev <mish@e2b.dev>
Summary
Moves SDK/CLI fork to
POST /v2/sandboxes/{sandboxID}/fork(added in https://github.com/e2b-dev/belt/pull/4036) and removes the client-side fork count caps. The only limit left is the server's per-team fork limit (team limits: 20 on Hobby and Pro, custom on Enterprise). Adds akeepMemory/keep_memoryfork option that maps to the v2 request'smemoryfield.SandboxApi.forkSandbox:POST('/sandboxes/{sandboxID}/fork')becomesPOST('/v2/sandboxes/{sandboxID}/fork');MAX_FORK_COUNT/validateForkCountremoved._cls_fork:post_sandboxes_sandbox_id_fork+SandboxForkRequestbecomespost_v_2_sandboxes_sandbox_id_fork+SandboxForkRequestV2;MAX_FORK_COUNT/validate_fork_countremoved.keepMemory, named after the existingpause({ keepMemory })option:false, only the filesystem is captured: the forks cold-boot from disk and the source keeps running. When the option is omitted,memoryis left off the request and the forks restore the source's memory as before. Unsupported environments return an API error (400/409); the server never quietly turns the request into a memory fork.--count: still has to be a positive integer, but the<= 20check is removed. The CLI doesn't exposekeepMemory.spec/openapi.yml: v2 fork path +SandboxForkRequestV2(includingmemory) taken from belt#4036, v1 marked deprecated. JSschema.gen.tsand the Python client regenerated withgenerate:api/make generate-api..changeset/cli-fork-max-count.md(client-side cap at 20, from Cap sandbox fork count at 20 #1909) is replaced by.changeset/fork-v2-server-limit.md.Behavior change: omitting
timeout/timeoutMsnow gets the v2 default of 300s (v1 used 15s). Omittedcountis still left off the request.Tests:
apiDefaults.test.tsandtest_api_defaults.pycheck thatmemoryis absent when unset andfalsewhen passed, for sync and async.Risk & rollout
Blocked on https://github.com/e2b-dev/belt/pull/4036:
spec/openapi.ymlhas to matche2b-dev/runtimeat thespec/runtime-refpin, which doesn't include the v2 fork route yet. After belt#4036 is mirrored to runtime, bumpspec/runtime-refand re-runmake codegen. The regenerated spec should match what's here.forkintegration tests (production/staging) fail withno matching operation was foundbecause the v2 route isn't deployed yet. All other SDK tests pass.keepMemory: falsealso needs the server'sfilesystem-only-checkpointflag enabled for the team.Merge and release only after belt#4036 is deployed.
Link to Devin session: https://app.devin.ai/sessions/f375710619bb49c9b79e537736121bda
Open in Devin Desktop: https://app.devin.ai/desktop/session/f375710619bb49c9b79e537736121bda?variant=devin
Requested by: @mishushakov