From 14c6a40de767efec65358788973c23b0491590cf Mon Sep 17 00:00:00 2001 From: Eric Curtin Date: Sun, 18 Oct 2026 21:38:29 +0100 Subject: [PATCH 1/5] fix(python): add opt-in sandbox delete timeout Signed-off-by: Eric Curtin --- e2e/python/conftest.py | 3 +++ python/openshell/sandbox.py | 29 +++++++++++++++++---- python/openshell/sandbox_cleanup_test.py | 33 ++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/e2e/python/conftest.py b/e2e/python/conftest.py index 3c2bea236a..8e676dfeb0 100644 --- a/e2e/python/conftest.py +++ b/e2e/python/conftest.py @@ -95,6 +95,9 @@ def _create(*, spec: object | None = None, delete_on_exit: bool = True) -> Sandb delete_on_exit=delete_on_exit, # Allow time to pull an explicitly supplied workload fixture. ready_timeout_seconds=300.0, + # Kubernetes holds DeleteSandbox open until the pod terminates + # (30s grace plus API headroom), which can exceed the 30s default. + delete_timeout_seconds=120.0, ) return _create diff --git a/python/openshell/sandbox.py b/python/openshell/sandbox.py index e9c840e481..229d49cbfd 100644 --- a/python/openshell/sandbox.py +++ b/python/openshell/sandbox.py @@ -626,9 +626,14 @@ def exec_python( timeout_seconds=timeout_seconds, ) - def delete(self, *, allow_missing: bool = False) -> DeletionResult: + def delete( + self, *, allow_missing: bool = False, timeout: float | None = None + ) -> DeletionResult: return self._client.delete( - self.sandbox.name, workspace=self._workspace, allow_missing=allow_missing + self.sandbox.name, + workspace=self._workspace, + allow_missing=allow_missing, + timeout=timeout, ) def stop(self) -> SandboxRef: @@ -1072,15 +1077,21 @@ def list_ids_for_all_workspaces( ] def delete( - self, name: str, *, workspace: str, allow_missing: bool = False + self, + name: str, + *, + workspace: str, + allow_missing: bool = False, + timeout: float | None = None, ) -> DeletionResult: + """Delete a sandbox, waiting up to `timeout` (default: client timeout).""" response = self._stub.DeleteSandbox( openshell_pb2.DeleteSandboxRequest( workspace_scope=_workspace_scope(workspace), name=name, allow_missing=allow_missing, ), - timeout=self._timeout, + timeout=self._timeout if timeout is None else timeout, ) return DeletionResult( DeletionOutcome(response.outcome), response.sandbox_id or None @@ -1586,6 +1597,7 @@ def __init__( workload_template: str | None = None, timeout: float = 30.0, ready_timeout_seconds: float = 120.0, + delete_timeout_seconds: float | None = None, auto_refresh: bool = True, write_back: bool = True, insecure: bool = False, @@ -1593,6 +1605,10 @@ def __init__( ) -> None: """Bind a Sandbox context to the active gateway. + `delete_timeout_seconds` bounds the delete RPC on exit and defaults to + `timeout`. Raise it for drivers that hold the delete open while the + workload terminates. + OIDC kwargs (`auto_refresh`, `write_back`, `insecure`, and `client_credentials`) forward directly to `SandboxClient.from_active_cluster` and have the @@ -1612,6 +1628,7 @@ def __init__( self._workload_template = workload_template self._timeout = timeout self._ready_timeout_seconds = ready_timeout_seconds + self._delete_timeout_seconds = delete_timeout_seconds self._auto_refresh = auto_refresh self._write_back = write_back self._insecure = insecure @@ -1707,7 +1724,9 @@ def __exit__(self, *args: object) -> None: and self._session is not None and self._client is not None ): - result = self._session.delete(allow_missing=True) + result = self._session.delete( + allow_missing=True, timeout=self._delete_timeout_seconds + ) if result.outcome == DeletionOutcome.ACCEPTED: self._client.wait_deleted( self._session.sandbox.name, diff --git a/python/openshell/sandbox_cleanup_test.py b/python/openshell/sandbox_cleanup_test.py index 4cf558251c..47c4eb945f 100644 --- a/python/openshell/sandbox_cleanup_test.py +++ b/python/openshell/sandbox_cleanup_test.py @@ -3,6 +3,7 @@ """Cleanup status handling through the curated client's real gRPC interceptor.""" +import time from concurrent import futures from contextlib import nullcontext from types import SimpleNamespace @@ -20,6 +21,7 @@ def cleanup_client(): state = SimpleNamespace( exists=False, code=grpc.StatusCode.NOT_FOUND, + delete_delay=0.0, calls=[], closed=[], ) @@ -46,6 +48,7 @@ def delete(request, context): assert request.name == "cleanup-test" assert request.workspace_scope.workspace == "default" assert request.allow_missing + time.sleep(state.delete_delay) if state.code == grpc.StatusCode.NOT_FOUND: return openshell_pb2.DeleteSandboxResponse( outcome=openshell_pb2.DELETION_OUTCOME_ALREADY_ABSENT @@ -158,6 +161,36 @@ def test_context_cleanup_handles_absence_and_intercepted_errors( assert managed._session is None +@pytest.mark.parametrize( + ("delete_timeout_seconds", "expect_deadline"), [(None, True), (5, False)] +) +def test_context_cleanup_delete_timeout_is_opt_in( + cleanup_client, monkeypatch, delete_timeout_seconds, expect_deadline +): + client, state = cleanup_client + state.exists = True + state.delete_delay = 0.5 + client._timeout = 0.2 + monkeypatch.setattr( + SandboxClient, + "from_active_cluster", + classmethod(lambda _cls, **_kwargs: client), + ) + + managed = Sandbox( + workspace="default", + sandbox="cleanup-test", + delete_timeout_seconds=delete_timeout_seconds, + ) + expected = pytest.raises(GatewayError) if expect_deadline else nullcontext() + with expected as caught, managed: + state.exists = False + if caught is not None: + assert caught.value.code() == grpc.StatusCode.DEADLINE_EXCEEDED + assert state.closed == [True] + assert managed._client is None + + class _EntryFailureClient: """Fails the named client call and records close().""" From 42bcba0f068f9b1b0a9939f2067c0e4627ccb03d Mon Sep 17 00:00:00 2001 From: Eric Curtin Date: Tue, 20 Oct 2026 11:41:24 +0100 Subject: [PATCH 2/5] fix(python): omit delete timeout unless set Signed-off-by: Eric Curtin --- python/openshell/sandbox.py | 4 +++- python/openshell/sandbox_test.py | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/python/openshell/sandbox.py b/python/openshell/sandbox.py index 229d49cbfd..bba54996a8 100644 --- a/python/openshell/sandbox.py +++ b/python/openshell/sandbox.py @@ -629,11 +629,13 @@ def exec_python( def delete( self, *, allow_missing: bool = False, timeout: float | None = None ) -> DeletionResult: + # Forward `timeout` only on opt-in so older client subclasses still work. + extra = {} if timeout is None else {"timeout": timeout} return self._client.delete( self.sandbox.name, workspace=self._workspace, allow_missing=allow_missing, - timeout=timeout, + **extra, ) def stop(self) -> SandboxRef: diff --git a/python/openshell/sandbox_test.py b/python/openshell/sandbox_test.py index 019d3cc674..674fb484db 100644 --- a/python/openshell/sandbox_test.py +++ b/python/openshell/sandbox_test.py @@ -3090,3 +3090,25 @@ def test_sandbox_session_delete_passes_workspace() -> None: assert stub.delete_request is not None assert _request_workspace(stub.delete_request) == "staging" + + +def test_sandbox_session_delete_omits_timeout_for_legacy_clients() -> None: + from openshell.sandbox import SandboxSession + + calls: list[dict[str, Any]] = [] + + class LegacyClient: + def delete(self, name, *, workspace, allow_missing=False): + calls.append((name, workspace, allow_missing)) + + ref = SandboxRef( + id="sandbox-1", + name="job-1", + workspace="staging", + status=SandboxStatusRef(phase=2, current_policy_version=0), + ) + session = SandboxSession(cast("Any", LegacyClient()), ref) + + session.delete(allow_missing=True) + + assert calls == [("job-1", "staging", True)] From dd5dfad875d2fbed89a938f6ec72e1b7577752db Mon Sep 17 00:00:00 2001 From: Eric Curtin Date: Thu, 22 Oct 2026 10:22:47 +0100 Subject: [PATCH 3/5] fix(python): fix delete test typing Signed-off-by: Eric Curtin --- python/openshell/sandbox_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/openshell/sandbox_test.py b/python/openshell/sandbox_test.py index 674fb484db..8dcd59e4d1 100644 --- a/python/openshell/sandbox_test.py +++ b/python/openshell/sandbox_test.py @@ -3095,7 +3095,7 @@ def test_sandbox_session_delete_passes_workspace() -> None: def test_sandbox_session_delete_omits_timeout_for_legacy_clients() -> None: from openshell.sandbox import SandboxSession - calls: list[dict[str, Any]] = [] + calls: list[tuple[str, str, bool]] = [] class LegacyClient: def delete(self, name, *, workspace, allow_missing=False): From 59a7094a02227fee794d72fde37e941d52044318 Mon Sep 17 00:00:00 2001 From: Eric Curtin Date: Mon, 2 Nov 2026 21:33:18 +0000 Subject: [PATCH 4/5] test(python): tighten delete timeout coverage Signed-off-by: Eric Curtin --- docs/sdk/python.mdx | 4 ++++ python/openshell/sandbox.py | 6 +++++- python/openshell/sandbox_cleanup_test.py | 14 +++++++++++--- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/docs/sdk/python.mdx b/docs/sdk/python.mdx index 035e5e73e2..7edcef02ff 100644 --- a/docs/sdk/python.mdx +++ b/docs/sdk/python.mdx @@ -148,6 +148,10 @@ workspace for repeated exec, stop, start, and delete operations. List methods return lazy `Pager` instances; use `list_all()` only when you want to fetch the complete collection. +Delete calls use the client timeout, 30 seconds by default. Kubernetes gateways +can hold a delete open until the pod terminates, so pass a larger `timeout` to +`delete`, or `delete_timeout_seconds` (for example, 120) to `Sandbox`. + ## Implement a Credential Driver The SDK includes generated message classes, type stubs, and gRPC bindings for diff --git a/python/openshell/sandbox.py b/python/openshell/sandbox.py index bba54996a8..f45a3b4041 100644 --- a/python/openshell/sandbox.py +++ b/python/openshell/sandbox.py @@ -1086,7 +1086,11 @@ def delete( allow_missing: bool = False, timeout: float | None = None, ) -> DeletionResult: - """Delete a sandbox, waiting up to `timeout` (default: client timeout).""" + """Delete a sandbox. + + `timeout` is the gRPC deadline in seconds for this call (default: the + client timeout). + """ response = self._stub.DeleteSandbox( openshell_pb2.DeleteSandboxRequest( workspace_scope=_workspace_scope(workspace), diff --git a/python/openshell/sandbox_cleanup_test.py b/python/openshell/sandbox_cleanup_test.py index 47c4eb945f..0450877b51 100644 --- a/python/openshell/sandbox_cleanup_test.py +++ b/python/openshell/sandbox_cleanup_test.py @@ -162,15 +162,19 @@ def test_context_cleanup_handles_absence_and_intercepted_errors( @pytest.mark.parametrize( - ("delete_timeout_seconds", "expect_deadline"), [(None, True), (5, False)] + ("client_timeout", "delete_timeout_seconds", "expect_deadline"), + [(0.2, None, True), (0.2, 5, False), (5, 0.2, True)], ) def test_context_cleanup_delete_timeout_is_opt_in( - cleanup_client, monkeypatch, delete_timeout_seconds, expect_deadline + cleanup_client, + monkeypatch, + client_timeout, + delete_timeout_seconds, + expect_deadline, ): client, state = cleanup_client state.exists = True state.delete_delay = 0.5 - client._timeout = 0.2 monkeypatch.setattr( SandboxClient, "from_active_cluster", @@ -184,11 +188,15 @@ def test_context_cleanup_delete_timeout_is_opt_in( ) expected = pytest.raises(GatewayError) if expect_deadline else nullcontext() with expected as caught, managed: + # Only the exit delete runs under the short deadline. + client._timeout = client_timeout state.exists = False if caught is not None: assert caught.value.code() == grpc.StatusCode.DEADLINE_EXCEEDED + assert state.calls == ["GetSandbox", "GetSandbox", "DeleteSandbox"] assert state.closed == [True] assert managed._client is None + assert managed._session is None class _EntryFailureClient: From 91ff960da7a0ca605968590aa1c4f9b0d5acfeaf Mon Sep 17 00:00:00 2001 From: Eric Curtin Date: Fri, 6 Nov 2026 10:36:14 +0000 Subject: [PATCH 5/5] fix(python): rename delete timeout to timeout_seconds Signed-off-by: Eric Curtin --- docs/sdk/python.mdx | 5 +++-- e2e/python/conftest.py | 5 ++++- python/openshell/sandbox.py | 14 +++++++------- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/docs/sdk/python.mdx b/docs/sdk/python.mdx index 7e3d28e430..ac806ef496 100644 --- a/docs/sdk/python.mdx +++ b/docs/sdk/python.mdx @@ -149,8 +149,9 @@ return lazy `Pager` instances; use `list_all()` only when you want to fetch the complete collection. Delete calls use the client timeout, 30 seconds by default. Kubernetes gateways -can hold a delete open until the pod terminates, so pass a larger `timeout` to -`delete`, or `delete_timeout_seconds` (for example, 120) to `Sandbox`. +can hold a delete open until the pod terminates, so pass a larger +`timeout_seconds` to `delete`, or `delete_timeout_seconds` (for example, 120) to +`Sandbox`. ## Call Generated RPCs diff --git a/e2e/python/conftest.py b/e2e/python/conftest.py index 8e676dfeb0..fb27b1e810 100644 --- a/e2e/python/conftest.py +++ b/e2e/python/conftest.py @@ -56,7 +56,10 @@ def cluster_name() -> str | None: @pytest.fixture(scope="session") def sandbox_client(cluster_name: str | None) -> Iterator[SandboxClient]: - with SandboxClient.from_active_cluster(cluster=cluster_name) as client: + # Direct deletes can wait out the Kubernetes pod grace period. + with SandboxClient.from_active_cluster( + cluster=cluster_name, timeout=120.0 + ) as client: yield client diff --git a/python/openshell/sandbox.py b/python/openshell/sandbox.py index 37f5aebf9e..0670f1d3a6 100644 --- a/python/openshell/sandbox.py +++ b/python/openshell/sandbox.py @@ -681,10 +681,10 @@ def exec_python( ) def delete( - self, *, allow_missing: bool = False, timeout: float | None = None + self, *, allow_missing: bool = False, timeout_seconds: float | None = None ) -> DeletionResult: - # Forward `timeout` only on opt-in so older client subclasses still work. - extra = {} if timeout is None else {"timeout": timeout} + # Forward `timeout_seconds` only on opt-in so older clients still work. + extra = {} if timeout_seconds is None else {"timeout_seconds": timeout_seconds} return self._client.delete( self.sandbox.name, workspace=self._workspace, @@ -1150,11 +1150,11 @@ def delete( *, workspace: str, allow_missing: bool = False, - timeout: float | None = None, + timeout_seconds: float | None = None, ) -> DeletionResult: """Delete a sandbox. - `timeout` is the gRPC deadline in seconds for this call (default: the + `timeout_seconds` is the gRPC deadline for this call (default: the client timeout). """ response = self._stub.DeleteSandbox( @@ -1163,7 +1163,7 @@ def delete( name=name, allow_missing=allow_missing, ), - timeout=self._timeout if timeout is None else timeout, + timeout=self._timeout if timeout_seconds is None else timeout_seconds, ) return DeletionResult( DeletionOutcome(response.outcome), response.sandbox_id or None @@ -1896,7 +1896,7 @@ def __exit__(self, *args: object) -> None: and self._client is not None ): result = self._session.delete( - allow_missing=True, timeout=self._delete_timeout_seconds + allow_missing=True, timeout_seconds=self._delete_timeout_seconds ) if result.outcome == DeletionOutcome.ACCEPTED: self._client.wait_deleted(