Add snapshot save command (local)#210
Conversation
anisaoshafi
force-pushed
the
drg-764-migrate-state-commands-from-cli-v1-export
branch
4 times, most recently
from
f6bbfbf to
6a24cdf
Compare
| return fmt.Errorf("no emulator configured") | ||
| } | ||
|
|
||
| c := appConfig.Containers[0] |
There was a problem hiding this comment.
issue: this could potentially panic. Snapshots are only for aws AFAIK so we could do this instead:
awsContainer := config.ContainerConfig{Type: config.EmulatorAWS, Port: config.DefaultAWSPort}
| dest = "ls-state-export" | ||
| } else if strings.Contains(dest, "://") { | ||
| return nil, fmt.Errorf("cloud destinations are not yet supported — use a file path like ./my-snapshot") | ||
| } else if !strings.HasPrefix(dest, ".") && !strings.HasPrefix(dest, "/") && !strings.HasPrefix(dest, "~") && !strings.Contains(dest, "/") { |
There was a problem hiding this comment.
issue: this looks like it would not work on Windows? Can we make unit-tests more OS-agnostic to verify? Instead of defining the paths as strings we could use filepath.Join("subdir", "state")
| return nil, fmt.Errorf("LocalStack returned status %d", resp.StatusCode) | ||
| } | ||
| return resp.Body, nil | ||
| } |
There was a problem hiding this comment.
question: we already have internal/snapshot/client.go for the aws emulator http api, can we add the new method there instead? Keeping a dedicated interface StateExporter just for the endpoints related to snapshots make sense to me though
There was a problem hiding this comment.
Sorry I meant to say internal/emulator/aws/client.go
| _ = resp.Body.Close() | ||
| return nil, fmt.Errorf("LocalStack returned status %d", resp.StatusCode) | ||
| } | ||
| return resp.Body, nil |
There was a problem hiding this comment.
question: instead of returning io.ReadCloser can we instead pass the io.Writer destination as argument?
This way we're sure the caller won't forget to close the returned body.
| } | ||
|
|
||
| func (d LocalDestination) Writer() (io.WriteCloser, error) { | ||
| return os.Create(d.Path) |
There was a problem hiding this comment.
issue: this abstraction won't work once we introduce pods because in the case of pods we are not saving to a file so we don't have a io.WriteCloser. We could just drop the interface for now?
|
I've posted a comment[1] in PRO-212 to discuss some potential changes, comments welcome! |
anisaoshafi
force-pushed
the
drg-764-migrate-state-commands-from-cli-v1-export
branch
from
19cab09 to
cc8b494
Compare
anisaoshafi
force-pushed
the
drg-764-migrate-state-commands-from-cli-v1-export
branch
3 times, most recently
from
cc4cd84 to
8ca807d
Compare
anisaoshafi
force-pushed
the
drg-764-migrate-state-commands-from-cli-v1-export
branch
from
c40edd6 to
e2a83f0
Compare
anisaoshafi
force-pushed
the
drg-764-migrate-state-commands-from-cli-v1-export
branch
from
e2a83f0 to
b09a963
Compare
Without this, a failed io.Copy left a corrupt/partial ZIP on disk. The user had no indication the file was incomplete. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Passing nil to runLstk inherited the developer's real $HOME, which could write to ~/.config/lstk/lstk.log and the file-keyring fallback. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Verify that ErrorEvent is routed to errOut and all other events go to out, and that nil writer arguments fall back safely without panicking. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
anisaoshafi
force-pushed
the
drg-764-migrate-state-commands-from-cli-v1-export
branch
from
36170a5 to
7e45925
Compare
anisaoshafi
changed the title
3 participants
Added a new snapshot subcommand with an initial save action that saves the current LocalStack state to a snapshot file.
Scope for this PR is saving a snapshot locally.
Saving remotely coming next.