sandbox: Take timedelta for snapshot expiration#78
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Updates the sandbox API surface to accept datetime.timedelta for time-based parameters (notably snapshot expiration), centralizing duration normalization to millisecond integers.
Changes:
- Refactored
SnapshotExpiration/MIN_SNAPSHOT_EXPIRATION_MSinto internal helpers and updated public exports. - Added
normalize_duration_ms()and wired it into sandbox creation timeouts and extend-timeout requests. - Expanded unit + integration tests to cover
timedeltasnapshot expiration behavior and serialization.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/unit/test_sandbox_snapshot.py |
Adds unit/property tests for SnapshotExpiration accepting timedelta and rejecting invalid values. |
tests/integration/test_sandbox_sync_async.py |
Adds sync/async integration coverage for serializing timedelta snapshot expiration to milliseconds. |
src/vercel/sandbox/snapshot.py |
Re-exports SnapshotExpiration/MIN_SNAPSHOT_EXPIRATION_MS from internal module (removes previous local implementation). |
src/vercel/sandbox/sandbox.py |
Accepts timedelta for timeout, extend_timeout, and snapshot(expiration=...); normalizes timeout values. |
src/vercel/_internal/sandbox/time.py |
Introduces shared duration-to-milliseconds normalization helper. |
src/vercel/_internal/sandbox/snapshot.py |
Implements SnapshotExpiration supporting int or timedelta with validation. |
src/vercel/_internal/sandbox/core.py |
Normalizes timeout/extend-timeout durations; ensures snapshot expiration is serialized as an integer. |
Comments suppressed due to low confidence (1)
src/vercel/sandbox/sandbox.py:199
timeoutnow acceptstimedeltaand is serialized vianormalize_duration_ms, but existing integration tests only assert integer millisecond timeouts. Add a test that passes atimedeltatimeout and asserts the request body contains the expected millisecond value to prevent regressions.
@staticmethod
async def create(
*,
source: Source | None = None,
ports: list[int] | None = None,
timeout: int | timedelta | None = None,
resources: dict[str, Any] | None = None,
runtime: str | None = None,
token: str | None = None,
project_id: str | None = None,
team_id: str | None = None,
interactive: bool = False,
env: dict[str, str] | None = None,
network_policy: NetworkPolicy | None = None,
) -> AsyncSandbox:
"""Create a new sandbox.
Args:
source: Source to initialize the sandbox from (git, tarball, or snapshot).
ports: List of ports to expose.
timeout: Sandbox timeout in milliseconds or as a ``timedelta``.
resources: Resource configuration.
runtime: Runtime to use.
token: API token (uses OIDC if not provided).
project_id: Project ID (uses OIDC if not provided).
team_id: Team ID (uses OIDC if not provided).
interactive: Enable interactive shell support. When True, the sandbox
will have an interactive port for PTY connections.
env: Default environment variables for the sandbox. These are inherited
by all commands unless overridden per-command.
network_policy: Sandbox network policy. Accepts ``"allow-all"``,
``"deny-all"``, or ``NetworkPolicyCustom``. Omitted when ``None``.
Returns:
Created AsyncSandbox instance.
"""
creds: Credentials = get_credentials(token=token, project_id=project_id, team_id=team_id)
client = AsyncSandboxOpsClient(team_id=creds.team_id, token=creds.token)
resp: SandboxAndRoutesResponse = await client.create_sandbox(
project_id=creds.project_id,
source=_normalize_source(source),
ports=ports,
timeout=normalize_duration_ms(timeout),
resources=resources,
runtime=runtime,
interactive=interactive,
env=env,
network_policy=network_policy,
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from vercel._internal.sandbox.snapshot import ( | ||
| MIN_SNAPSHOT_EXPIRATION_MS as _MIN_SNAPSHOT_EXPIRATION_MS, | ||
| SnapshotExpiration as _SnapshotExpiration, | ||
| ) | ||
|
|
||
| from ..oidc import Credentials, get_credentials | ||
| from .page import AsyncSnapshotPage, AsyncSnapshotPager, SnapshotPage | ||
|
|
||
| MIN_SNAPSHOT_EXPIRATION_MS: Final[int] = 86_400_000 | ||
|
|
||
|
|
||
| class SnapshotExpiration(int): | ||
| """Snapshot expiration in milliseconds. | ||
|
|
||
| Valid values are ``0`` for no expiration or any value greater than or equal | ||
| to ``86_400_000`` (24 hours). | ||
| """ | ||
|
|
||
| def __new__(cls, value: int) -> SnapshotExpiration: | ||
| value = int(value) | ||
| if value != 0 and value < MIN_SNAPSHOT_EXPIRATION_MS: | ||
| raise ValueError( | ||
| "Snapshot expiration must be 0 for no expiration or >= 86400000 milliseconds" | ||
| ) | ||
| return int.__new__(cls, value) | ||
|
|
||
|
|
||
| def normalize_snapshot_expiration( | ||
| expiration: int | SnapshotExpiration | None, | ||
| ) -> SnapshotExpiration | None: | ||
| if expiration is None: | ||
| return None | ||
| if isinstance(expiration, SnapshotExpiration): | ||
| return expiration | ||
| return SnapshotExpiration(expiration) | ||
| MIN_SNAPSHOT_EXPIRATION_MS = _MIN_SNAPSHOT_EXPIRATION_MS | ||
| SnapshotExpiration = _SnapshotExpiration |
There was a problem hiding this comment.
normalize_snapshot_expiration was removed from vercel.sandbox.snapshot. Since this module is part of the public package, this is a potential breaking change for downstream callers importing it directly. Consider keeping a small wrapper (possibly deprecated) that forwards to SnapshotExpiration(...) / returns None when appropriate, so existing imports continue to work.
| duration: The duration in milliseconds or as a ``timedelta`` to | ||
| extend the timeout by. | ||
| """ | ||
| response = await self.client.extend_timeout(sandbox_id=self.sandbox.id, duration=duration) |
There was a problem hiding this comment.
extend_timeout now accepts timedelta, but the integration tests only cover integer millisecond durations. Add coverage that passes a timedelta and asserts the serialized request body uses millisecond integers, for both sync and async variants.
| response = await self.client.extend_timeout(sandbox_id=self.sandbox.id, duration=duration) | |
| normalized_duration = normalize_duration_ms(duration) | |
| response = await self.client.extend_timeout( | |
| sandbox_id=self.sandbox.id, | |
| duration=normalized_duration, | |
| ) |
No description provided.