[ericcurtin]

Summary

Python E2E cleanup no longer fails when Kubernetes pod termination outlasts the 30s delete deadline.

Related Issue

Closes #4685

Changes

• Add opt-in delete_timeout_seconds to Sandbox. The default is unchanged.
• The E2E fixture sets it to 120s.

Testing

• Checks appropriate to the affected code and behavior pass
• Unit tests added/updated (if applicable)
• E2E tests added/updated (if applicable)

Ran pytest python/, ruff and ty. Not run against a Kubernetes gateway.

Checklist

• Follows Conventional Commits
• Commits are signed off (DCO)
• Architecture docs updated (if applicable)

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[ericcurtin]

@krishicks @matthewgrossman could you please triage #4685 so the issue check can pass? @drew @sjenning PTAL, and /ok to test 14c6a40de767efec65358788973c23b0491590cf if it looks good. Thank you!

[TaylorMutch]

/ok to test 14c6a40

[github-actions]

Compatibility review (advisory)

New or updated breaking changes; see the full report for all findings.

1. python-sdk — No explicit risk acceptance found

SandboxSession.delete() changed from calling SandboxClient.delete(..., allow_missing=...) to always adding timeout=None when the argument is omitted.

• Impact: Existing SandboxClient subclasses that retain the baseline delete signature now raise TypeError on an unchanged SandboxSession.delete() call.
• Action: When timeout is None, call SandboxClient.delete without the new keyword and forward it only when a caller explicitly opts in.
• PR: fix(python): add opt-in sandbox delete timeout #4697

Review details

• Execution: complete
• Candidate: PR #4697 (dac79c70d5557ecf41c9cae7e4d873dc5dffb552)
• Baseline: 55bf786453395f3fe1858a187152537b457de77d (55bf786453395f3fe1858a187152537b457de77d)
• Reviewed: rust-sdk, python-sdk, go-sdk, typescript-sdk, api-behavior, cli, configuration, policy, helm, state

Advisory review; publication gates are unchanged.

Branch check run

[TaylorMutch]

@ericcurtin please see the compatibility review and address the concern. Thanks!

[ericcurtin]

@TaylorMutch Fixed: timeout is now forwarded only when set. @krishicks @matthewgrossman PTAL, and /ok to test 42bcba0f0 if it looks good. Thank you!

[TaylorMutch]

/ok to test 42bcba0

[copy-pr-bot]

/ok to test 42bcba0

@TaylorMutch, there was an error processing your request: E2

See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/2/

[TaylorMutch]

/ok to test 42bcba0

[TaylorMutch]

@TaylorMutch Fixed: timeout is now forwarded only when set. @krishicks @matthewgrossman PTAL, and /ok to test 42bcba0f0 if it looks good. Thank you!

Just FYI we can't use short sha's anymore on /ok to test.

[TaylorMutch]

@ericcurtin please see the branch check failures

[ericcurtin]

@TaylorMutch Fixed the typecheck failure. /ok to test dd5dfad875d2fbed89a938f6ec72e1b7577752db if it looks good. Thank you!

[JohnnyWilson16]

Tested commit dd5dfad locally:

Backward compatibility verified: SandboxSession.delete() correctly omits the keyword when timeout=None, preserving compatibility with legacy client subclasses.
Defaults preserved: standard callers remain bound to 30s; delete_timeout_seconds=120.0 in e2e/python/conftest.py resolves the Kubernetes pod termination race.
Passing unit tests verified in python/openshell/sandbox_cleanup_test.py and python/openshell/sandbox_test.py.
Looks good to land.

[TaylorMutch]

/ok to test dd5dfad