Add sandbox and snapshot list APIs#14
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 1 minutes and 49 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR adds pagination and listing capabilities for sandboxes and snapshots, along with new methods to retrieve sandbox system information (user home directory and working directory). It includes new response schemas, model classes, client methods with parameter validation, and comprehensive test coverage for both synchronous and asynchronous APIs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
leap0/_sync/sandbox.py (1)
262-310: Consider conditionally includingstatein params.When
stateisNone, the params dict includes"state": None. While httpx typically omitsNonevalues from query strings, explicitly building the params dict withoutNoneentries is more defensive and clearer.♻️ Optional: Build params dict conditionally
+ params: dict[str, str | int] = { + "sort": sort, + "order-by": order_by, + "page": page, + "page-size": page_size, + } + if state is not None: + params["state"] = state + data = cast(ListSandboxesResponseDict, self._transport.request_json( "GET", "/v1/sandboxes", - params={ - "state": state, - "sort": sort, - "order-by": order_by, - "page": page, - "page-size": page_size, - }, + params=params, timeout=http_timeout, ))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/sandbox.py` around lines 262 - 310, The list method currently always includes "state": state in the params passed to self._transport.request_json causing a None value to be sent when state is None; change the params construction in list (and the call to self._transport.request_json) to build the params dict conditionally (e.g., start with params = {"sort": sort, "order-by": order_by, "page": page, "page-size": page_size} and only add "state": state if state is not None) so the request omits the state key when state is None; keep existing validation (valid_states) and preserve the same param keys used by _transport.request_json.leap0/_async/sandbox.py (1)
257-305: Same optional improvement as the sync version: consider conditionally includingstatein params.The validation logic and param handling correctly mirror the sync implementation.
♻️ Optional: Build params dict conditionally
+ params: dict[str, str | int] = { + "sort": sort, + "order-by": order_by, + "page": page, + "page-size": page_size, + } + if state is not None: + params["state"] = state + data = cast(ListSandboxesResponseDict, await self._transport.request_json( "GET", "/v1/sandboxes", - params={ - "state": state, - "sort": sort, - "order-by": order_by, - "page": page, - "page-size": page_size, - }, + params=params, timeout=http_timeout, ))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/sandbox.py` around lines 257 - 305, The list method currently always includes "state" in the params dict even when state is None; update async sandbox.List (the async method named list) to only include the "state" key when state is not None (keep the existing validation via valid_states), e.g. build params dict conditionally before calling self._transport.request_json so you only send "state" when provided while leaving "sort", "order-by", "page", and "page-size" unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@leap0/models/snapshot.py`:
- Around line 110-116: The from_dict classmethod on SnapshotListResponse
currently silently defaults missing "items" and "total_items" to [] and 0;
change it to treat those keys as required: check that "items" exists in the
input dict and is a list (and that each element can be passed to
Snapshot.from_dict), and that "total_items" exists and can be coerced to int; if
validation fails, raise a clear exception (ValueError/TypeError) rather than
using the defaults so malformed wire responses aren't silently accepted.
Reference the from_dict method, the "items" and "total_items" keys, and
Snapshot.from_dict when adding these checks.
In `@tests/_async/test_snapshots.py`:
- Around line 59-62: The test test_list_validates_page_size is currently
catching a broad Exception; update it to assert the specific Leap0Error thrown
by AsyncSnapshotsClient.list when page_size is invalid by replacing
pytest.raises(Exception, match="page_size") with pytest.raises(Leap0Error,
match="page_size") and ensure Leap0Error is imported into the test module so the
test mirrors the sync variant.
---
Nitpick comments:
In `@leap0/_async/sandbox.py`:
- Around line 257-305: The list method currently always includes "state" in the
params dict even when state is None; update async sandbox.List (the async method
named list) to only include the "state" key when state is not None (keep the
existing validation via valid_states), e.g. build params dict conditionally
before calling self._transport.request_json so you only send "state" when
provided while leaving "sort", "order-by", "page", and "page-size" unchanged.
In `@leap0/_sync/sandbox.py`:
- Around line 262-310: The list method currently always includes "state": state
in the params passed to self._transport.request_json causing a None value to be
sent when state is None; change the params construction in list (and the call to
self._transport.request_json) to build the params dict conditionally (e.g.,
start with params = {"sort": sort, "order-by": order_by, "page": page,
"page-size": page_size} and only add "state": state if state is not None) so the
request omits the state key when state is None; keep existing validation
(valid_states) and preserve the same param keys used by _transport.request_json.
🪄 Autofix (Beta)
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
Run ID: 5096f7e7-dcc8-4d28-accc-6bcb2cf1c6c6
📒 Files selected for processing (14)
leap0/__init__.pyleap0/_async/sandbox.pyleap0/_async/snapshots.pyleap0/_schemas/sandbox.pyleap0/_schemas/snapshot.pyleap0/_sync/sandbox.pyleap0/_sync/snapshots.pyleap0/models/sandbox.pyleap0/models/snapshot.pytests/_async/test_sandboxes.pytests/_async/test_snapshots.pytests/_sync/test_sandboxes.pytests/_sync/test_snapshots.pytests/test_import.py
|
@CodeRabbit review? |
|
✅ Actions performedReview triggered.
|
1 participant
Summary
Testing
Summary by CodeRabbit