You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
refactor(modal): upgrade to 1.6.3 and use public SDK APIs - #533
Hey, upgrading Modal to get on our latest backend, improving perf and reliability. Also uses new GA methods, no more experimental 🙂
Raise the Modal client requirement from >=1.5.2.dev22 to >=1.6.3 and update the locked client from 1.6.2.dev1 to the latest stable release. Use the public APIs available with this floor:
Create ordinary containers and VMs with Sandbox.create(). Keep _experimental_create(i6pn=True) for private networking, which the public factory does not expose, and preserve GPU restrictions.
Select the VM runtime with runtime="vm" instead of experimental_options={"vm_runtime": True}. Preserve the nested-virtualization option for disk sandboxes.
Pull ECR images with Image.from_registry() and the existing named AWS secret, preserving credential routing and registry-host checks.
Update the provider tests and extend the installed-SDK signature checks to cover both public container creation and private networking. File-transfer changes are outside this PR.
Regenerated uv.lock with the CI-pinned uv 0.13.1. The only package version changes are Modal and its required synchronicity update (0.12.6 → 0.12.7); uv also normalizes lockfile markers and format.
Validation:
Modal provider unit tests: 129 passed, including installed-SDK signature checks, VM/disk creation, GPU restrictions, cancellation cleanup, network policies, and image authentication.
🤖 Automated Claude routine — Env Pod PR Context Review. This is a bot, not Edgar Arakelyan; not a human review.
Offering context on one branch this repo's tests can't exercise end to end: the ECR pull-secret path in _registry_image.
The PR description says ECR images are pulled "with Image.from_registry() and the existing named AWS secret, preserving credential routing". The registry-host check is indeed preserved, but the secret shape looks like it changes meaning. The rewritten call keeps required_keys=ECR_READER_SECRET_KEYS, which is ["AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY", "AWS_REGION"] (src/agent_env/providers/sandbox_providers/modal_sandbox.py:42). The very next branch of the same function shows what this repo expects a from_registry secret to carry — REGISTRY_USERNAME / REGISTRY_PASSWORD (modal_sandbox.py:522-525 on main). So after the change that branch hands from_registry a secret with AWS keys and no registry username/password, and the AWS→ECR token exchange that from_aws_ecr performed no longer has an owner.
Two reasons this may not show up before it ships:
The assertions that pinned the old behaviour are relaxed in the same diff — assert aws_ecr.call_args.args[0] == ecr and the aws_ecr.assert_not_called() negative control are both dropped, and the test's docstring ("ECR -> from_aws_ecr + named secret") goes with them. Both factories were previously patched side by side, which is what made the two paths distinguishable; a green suite no longer does.
The branch only activates when a deployment sets ecr_pull_secret_nameand its image store carries EcrCredentials. Nothing checked into this repo turns that on, so neither the unit suite nor a fork CI run reaches it — and the description notes live Modal integration tests weren't run.
If from_aws_ecr is being retired in 1.6, the AWS→registry credential exchange likely needs to move into the provider (mint a token, pass it as REGISTRY_USERNAME/REGISTRY_PASSWORD) rather than be dropped — the uncached fallback at the bottom of the same function already mints an ECR token, so the pieces are there. Worth a maintainer confirming against a real private-ECR pull before merge.
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
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.
Hey, upgrading Modal to get on our latest backend, improving perf and reliability. Also uses new GA methods, no more experimental 🙂
Raise the Modal client requirement from
>=1.5.2.dev22to>=1.6.3and update the locked client from1.6.2.dev1to the latest stable release. Use the public APIs available with this floor:Sandbox.create(). Keep_experimental_create(i6pn=True)for private networking, which the public factory does not expose, and preserve GPU restrictions.runtime="vm"instead ofexperimental_options={"vm_runtime": True}. Preserve the nested-virtualization option for disk sandboxes.Image.from_registry()and the existing named AWS secret, preserving credential routing and registry-host checks.Update the provider tests and extend the installed-SDK signature checks to cover both public container creation and private networking. File-transfer changes are outside this PR.
Regenerated
uv.lockwith the CI-pinned uv 0.13.1. The only package version changes are Modal and its required synchronicity update (0.12.6→0.12.7); uv also normalizes lockfile markers and format.Validation:
smolpackage unavailable).uv lock --checkwith uv 0.13.1: passed.uv run --frozen ruff check .: passed.git diff --check: passed.Live Modal integration tests were not run. Upstream CodeBuild requires maintainer approval before running for this fork.


Confidence Score: 5/5
The PR appears safe to merge; no actionable new defects were established.Summary
Raises the Modal requirement to
>=1.6.3and updates the providers to use its public APIs.Reviews (2) · Last reviewed commit: "refactor(modal): use public sandbox and ..." · Reviewed by Greptile