Skip to content

refactor(v1)!: make WaitReady succeed only on Ready - #18

Draft
elezar wants to merge 1 commit into
mainfrom
refactor/wait-ready-succeed-only-on-ready/elezar
Draft

elezar wants to merge 1 commit into
mainfrom
refactor/wait-ready-succeed-only-on-ready/elezar

Conversation

@elezar

@elezar elezar commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Summary

WaitReady returned the sandbox without error when it reached Completed. It now returns an error that names completion, like Stopped and Error. This keeps the Go SDK in line with the Rust and TypeScript SDKs, which make the same change in NVIDIA/OpenShell#5226 (tracked in NVIDIA/OpenShell#5225).

This PR is a draft until there is consensus on the breaking change in NVIDIA/OpenShell#5226.

Changes

  • checkTerminalPhase no longer returns success for Completed when the target is SandboxReady.
  • TestSandboxWaitReady_SandboxCompleted becomes TestSandboxWaitReady_RejectsCompleted and expects a completed before reaching Ready error.

Breaking change: callers that relied on WaitReady returning a Completed sandbox must poll the sandbox themselves.

Testing

Not yet run in this repository. The same change passes go test ./openshell/v1/... in NVIDIA/OpenShell sdk/go. mise run ci here needs to run on this PR.

WaitReady previously returned the sandbox without error when it reached
Completed. A caller waiting for a sandbox almost always needs a live one,
and the name suggests that, so a sandbox that completes before becoming
ready is now an error that fails fast, like Stopped and Error. This matches
the Rust and TypeScript SDKs in NVIDIA/OpenShell (see NVIDIA/OpenShell#5226).

BREAKING CHANGE: SandboxInterface.WaitReady returns an error instead of the
sandbox when it reaches the Completed phase. Callers that need to wait for a
finished workload should poll the sandbox.

Signed-off-by: Evan Lezar <[email protected]>

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant