Skip to content

feat(sdks/kotlin): add SandboxManager.waitForSnapshotReady#1051

Open
ferponse wants to merge 4 commits into
opensandbox-group:mainfrom
ferponse:feat/sdk-kotlin-wait-for-snapshot-ready
Open

feat(sdks/kotlin): add SandboxManager.waitForSnapshotReady#1051
ferponse wants to merge 4 commits into
opensandbox-group:mainfrom
ferponse:feat/sdk-kotlin-wait-for-snapshot-ready

Conversation

@ferponse

Copy link
Copy Markdown

Summary

Snapshot creation is asynchronous: Sandbox.createSnapshot / SandboxManager.createSnapshot return as soon as the snapshot record exists (typically in the Creating state). Today the Kotlin SDK gives callers no way to wait for the snapshot to actually become usable — they have to hand-roll a polling loop over getSnapshot.

This PR adds a small, client-side convenience that polls until the snapshot is ready:

  • SandboxState-style SnapshotState constants (Creating / Ready / Failed / Deleting / Unknown) so callers can compare states without magic strings.
  • SnapshotFailedException + a SNAPSHOT_FAILED error code for the terminal-failure case.
  • SandboxManager.waitForSnapshotReady(snapshotId, timeout = 5m, pollingInterval = 2s) — polls getSnapshot until the snapshot reaches Ready, throws SnapshotFailedException on Failed, and throws SandboxReadyTimeoutException on timeout. Annotated @JvmOverloads so Java callers can omit the defaults.

It mirrors the existing Sandbox.checkReady polling style (deadline + fixed interval, java.time.Duration, SDK exception hierarchy).

This is pure client-side glue: no changes to the OpenAPI specs, the server, or generated code, and it is purely additive (no breaking changes).

SnapshotInfo snap = manager.createSnapshot(sandboxId, "my-snap");
SnapshotInfo ready = manager.waitForSnapshotReady(snap.getId());

Testing

  • ./gradlew spotlessApply :sandbox:test — green (full :sandbox suite: 180 tests, 0 failures).
  • Added 4 unit tests in SandboxManagerTest covering: becomes-ready after polling, fail-fast on Failed, timeout, and rejection of a non-positive polling interval.

Breaking Changes

None. Additive, backward-compatible API.

Checklist

  • Conventional Commits
  • Unit tests added
  • Java interop preserved (java.time.Duration, @JvmOverloads, no coroutines)
  • No generated code edited
  • E2E (tests/java/) — happy to add a SnapshotE2ETest assertion if maintainers prefer

Co-authored-by: Atenea Agent srv_atenea_gitlab@ofidona.net

Snapshot creation is asynchronous: createSnapshot returns while the
snapshot is still in the Creating state. This adds a client-side poller
that waits until the snapshot reaches Ready, fails fast on Failed, and
times out after a configurable deadline.

- Add SnapshotState constants (Creating/Ready/Failed/Deleting/Unknown),
  mirroring SandboxState.
- Add SnapshotFailedException and the SNAPSHOT_FAILED error code.
- Add SandboxManager.waitForSnapshotReady(snapshotId, timeout,
  pollingInterval) with @jvmoverloads for Java callers.

Pure client-side glue: no spec, server, or generated-code changes.

Co-authored-by: Atenea Agent <srv_atenea_gitlab@ofidona.net>
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ This PR has no labels. Please add one based on the changes.

Changed directories: sdks.

📋 Recommended labels (based on changed files):

  • sdks ⬅️

Other available labels:

  • bug - Something isn't working
  • dependencies - Pull requests that update a dependency file
  • documentation - Improvements or additions to documentation
  • feature - New feature or request
  • packages - Changes for package, image and configuration

💡 Tip: Use feature for new functionality or improvements, bug for fixes.

cc @ferponse

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8a2590207d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8a2590207d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

waitForSnapshotReady gave up early when the remaining window was smaller
than pollingInterval (and timed out after the first poll when
pollingInterval >= timeout). Sleep for min(pollingInterval, remaining)
and keep polling until the deadline so a snapshot that becomes Ready
within the window is not reported as a false timeout.

Co-authored-by: Atenea Agent <srv_atenea_gitlab@ofidona.net>

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7533540dc6

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Check the deadline at the top of the wait loop so a snapshot that only
turns Ready after the timeout is reported as a SandboxReadyTimeoutException
instead of a late success, and never sleep past the deadline. Adds a
regression test for the late-ready case and keeps the within-window
polling test.

Co-authored-by: Atenea Agent <srv_atenea_gitlab@ofidona.net>

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d62d7c7fca

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

…ault

- Re-check the deadline after getSnapshot returns READY: a poll that starts
  before the deadline but blocks past it (slow server) now surfaces a
  SandboxReadyTimeoutException instead of a late success.
- Raise the default timeout from 5m to 900s to cover the server's
  snapshot_create_timeout_seconds (Kubernetes commit jobs may run up to the
  controller commitJobTimeout, 10m by default), so the default no longer
  times out while the backend is still within its normal snapshot window.

Co-authored-by: Atenea Agent <srv_atenea_gitlab@ofidona.net>
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