fix(rest): return immediately on volume create, drop synchronous wait - #1196
Draft
G4614 wants to merge 3 commits into
Draft
fix(rest): return immediately on volume create, drop synchronous wait#1196G4614 wants to merge 3 commits into
G4614 wants to merge 3 commits into
Conversation
POST /volumes blocked up to 30s polling for the volume to become ready, risking client/gateway timeouts around that same window for what is an inherently async operation (bucket provisioning runs on a 5s reconciler tick, decoupled from the request). Return 202 with state=pending_create as soon as the row exists, matching the pre-existing classic VolumeController's behavior (apps/api/src/box/controllers/volume.controller.ts) which never waited either. Callers learn readiness via GET or the existing volume.state.updated webhook (apps/api/src/box/subscribers/volume.subscriber.ts), not by polling inside a single held-open request. Removes VolumeService.waitForReady, now unused.
📦 BoxLite review — couldn't completepowered by BoxLite |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughVolume creation no longer waits for provisioning readiness. The controller returns HTTP 202 with the current volume state. The service polling method and related tests are removed. The OpenAPI contract documents asynchronous creation and removes timeout responses. ChangesAsynchronous volume creation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant BoxliteVolumeController
participant VolumeService
Client->>BoxliteVolumeController: POST /volumes
BoxliteVolumeController->>VolumeService: create()
VolumeService-->>BoxliteVolumeController: newly created volume
BoxliteVolumeController-->>Client: 202 Accepted with pending_create state
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
G4614
marked this pull request as draft
August 11, 2026 08:14
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openapi/box.openapi.yaml`:
- Around line 195-204: Update the createVolume controller to return HTTP 202,
matching the documented asynchronous creation contract and the OpenAPI response.
Regenerate the TypeScript and Go client artifacts from the updated
specification, then adjust related tests and documentation to expect 202.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ef3fa219-b516-4dd7-962e-0ee876df8949
📒 Files selected for processing (5)
apps/api/src/box/services/volume.service.spec.tsapps/api/src/box/services/volume.service.tsapps/api/src/boxlite-rest/boxlite-volume.controller.spec.tsapps/api/src/boxlite-rest/boxlite-volume.controller.tsopenapi/box.openapi.yaml
💤 Files with no reviewable changes (1)
- apps/api/src/boxlite-rest/boxlite-volume.controller.spec.ts
…t/CLI The REST volume create endpoint returns as soon as the volume is accepted (state=pending_create); provisioning happens asynchronously server-side. The Rust core's VolumeInfo/VolumeResponse never carried state at all, so there was no way for a client to observe this. Adds VolumeState (mirroring the API's enum) plus state/error_reason fields, and VolumeHandle::wait_until_ready — a client-side poll loop (mirrors ComputerBox/SkillBox's waitUntilReady in the Node SDK) that blocks until ready/error/timeout without the server holding a connection open. Wires it into the CLI: `boxlite volume create --wait [--wait-timeout secs]` restores the old synchronous-feeling UX for callers who want it, and `volume ls`/`get` now show a STATE column.
PyVolumeInfo/JsVolumeInfo only ever carried id/created_at/size_bytes, matching the pre-fix VolumeInfo — SDK callers had no way to see whether a volume they just created (state=pending_create) had actually become ready, short of raw-decoding the wire response themselves. Adds state (string, matching the API's snake_case wire values) and error_reason, and exposes VolumeHandle::wait_until_ready (added earlier in this PR for the Rust core/CLI) as create()/wait_until_ready() on both PyVolumeHandle and JsVolumeHandle, mirroring create/list/get/remove.
DorianZheng
pushed a commit
that referenced
this pull request
Aug 11, 2026
~~Expose existing S3 managed volumes through the BoxLite REST API and carry volume mounts into REST-created boxes.~~ **Rebuilt on top of main** now that this PR's backend content has been split out and merged separately: - #1191 — volume REST CRUD (`/v1/volumes` create/list/get/delete) - #1192 — REST box-create volume mount wiring (`VolumeSpec.source`) - (follow-up, still open) #1196 — makes `create` return immediately instead of blocking, adds client-side `wait_until_ready` What's left here is the remaining, still-unmerged piece: **CLI + Node/Python SDK support for attaching a managed volume to a box**, on top of the server-side contract #1191/#1192 already shipped. - CLI: `--mount src=volume://<id>,target=<path>` on `create`/`run` - Node/Python SDKs: accept a scheme-qualified volume source (`volume://<id>`) alongside the existing host-path volume option Test plan: - `cargo build -p boxlite-cli` / `cargo test -p boxlite-cli --bin boxlite volume` — 30 tests, all pass - `cargo check -p boxlite-node` / `cargo check -p boxlite-python` — clean - `cargo fmt --all -- --check` — clean - Fixed a pre-existing test bug found during verification: `test_volume_flags_managed_volume_rejects_missing_id`'s fixture never reached the code path it meant to exercise (see commit message for detail)
DorianZheng
reviewed
Aug 12, 2026
| /// enum (`apps/api/src/box/enums/volume-state.enum.ts`). | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] | ||
| #[serde(rename_all = "snake_case")] | ||
| pub enum VolumeState { |
Member
There was a problem hiding this comment.
3 states is good enough. and consistent with BoxState
DorianZheng
reviewed
Aug 12, 2026
| /// timeouts for what both `boxlite`'s runner and API treat as inherently | ||
| /// async). Mirrors `ComputerBox`/`SkillBox`'s `waitUntilReady` in the | ||
| /// Node SDK (`sdks/node/lib/computerbox.ts`, `skillbox.ts`). | ||
| pub async fn wait_until_ready(&self, id: &str, timeout: Duration) -> BoxliteResult<VolumeInfo> { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #1191 (merged).
createblocked up to 30s polling for the volume to become ready viaVolumeService.waitForReady, risking client/gateway timeouts around that same window for what is an inherently async operation — bucket provisioning runs on a 5s reconciler tick (VolumeManager.processPendingVolumes), fully decoupled from the request.API:
createnow returns 202 withstate=pending_createas soon as the row exists, matching what the pre-existing classicVolumeController(apps/api/src/box/controllers/volume.controller.ts) already did — it never waited either. RemovesVolumeService.waitForReady, now unused.Client-side wait: the Rust core's
VolumeInfo/VolumeResponsenever carriedstateat all, so there was no way for any caller to observe readiness short of re-deserializing raw JSON. Adds aVolumeStateenum (mirroring the API's) plusstate/error_reasonfields, andVolumeHandle::wait_until_ready— a client-side poll loop mirroringComputerBox/SkillBox'swaitUntilReadyin the Node SDK — that blocks until ready/error/timeout without the server holding a connection open. Wired into the CLI:boxlite volume create --wait [--wait-timeout secs]restores the old synchronous-feeling UX for callers who want it;volume ls/getnow show a STATE column.Callers who don't use
--waitlearn readiness viaGET /volumes/{id}(always authoritative, no external dependency) or the existingvolume.state.updatedwebhook if their org has one configured (best-effort, opt-in — not a delivery guarantee on its own).Test plan:
yarn nx run api:build/yarn nx run api:test -- --testPathPatterns=volume— 4 suites, 23 tests, all passcargo check -p boxlite --features rest/cargo build -p boxlite-cli— cleancargo test -p boxlite-cli volume::— 2 new/updated unit tests passcargo fmt --all -- --check— cleanboxlite volume createreturns in ~0.3s withstate=pending_create;boxlite volume create --waitblocks ~2s and returns oncestate=ready;boxlite volume lsshows the new STATE column throughout