sandbox: dynamic directory mount & snapshot - Go#243
Open
ehdr wants to merge 3 commits intoehdr/sb-command-router-gofrom
Open
sandbox: dynamic directory mount & snapshot - Go#243ehdr wants to merge 3 commits intoehdr/sb-command-router-gofrom
ehdr wants to merge 3 commits intoehdr/sb-command-router-gofrom
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
thomasjpfan
approved these changes
Dec 23, 2025
Contributor
thomasjpfan
left a comment
thomasjpfan
left a comment
There was a problem hiding this comment.
Minor nits, otherwise LGTM
Comment on lines
833
to
841
|
|
||
| if image != nil && image.ImageID == "" { | ||
| return InvalidError{Exception: "Image must be built before mounting. Call `image.Build(app)` first."} | ||
| } | ||
|
|
||
| imageID := "" | ||
| if image != nil { | ||
| imageID = image.ImageID | ||
| } |
Contributor
There was a problem hiding this comment.
Nit:
imageId := ""
if image != nil {
if image.ImageID == "" {
return InvalidError{...}
}
imageId = image.ImageID
}| _, err = echoProc.Wait(ctx) | ||
| g.Expect(err).ShouldNot(gomega.HaveOccurred()) | ||
|
|
||
| mountImage, err := sb1.SnapshotFilesystem(ctx, 55*time.Second) |
Contributor
There was a problem hiding this comment.
Are we going to advertise that you can snapshot the whole filesystem and then mounting it into /mnt/data in another sandbox?
Contributor
Author
There was a problem hiding this comment.
Yeah I think it's alright? Used it mostly as a convenient way to make an image not from a directory snapshot. But this test perhaps doesn't add much in addition to the next one, so could skip it entirely I suppose.
Contributor
There was a problem hiding this comment.
I think it's okay to test as long as this is a supported feature.
ehdr
force-pushed
the
ehdr/sb-command-router-go
branch
3 times, most recently
from
edbb15d to
ac41d27
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Port of a2d11ac and thus modal-labs/modal-client#3791.
Note
Adds experimental sandbox directory mount/snapshot in Go
Sandbox.ExperimentalMountImage(path, image)andSandbox.ExperimentalSnapshotDirectory(path)inmodal-go/sandbox.go, wired through the command routerexamples/sandbox-image-mountdemonstrating mounting a repo dir, cloning, snapshotting, and remountingsandbox-image-mount.tsto install git in base image and show mount/snapshot usageCHANGELOG.mdto note experimental support for single-directory snapshot and image mountWritten by Cursor Bugbot for commit 4337fc6. This will update automatically on new commits. Configure here.