-
Notifications
You must be signed in to change notification settings - Fork 0
Replace profile option with loading spinner #176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| package cmd | ||
|
|
||
| import ( | ||
| "github.com/localstack/lstk/internal/awscli" | ||
| "github.com/localstack/lstk/internal/config" | ||
| "github.com/localstack/lstk/internal/endpoint" | ||
| "github.com/localstack/lstk/internal/env" | ||
| "github.com/localstack/lstk/internal/telemetry" | ||
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| func newAWSCmd(cfg *env.Env, tel *telemetry.Client) *cobra.Command { | ||
| return &cobra.Command{ | ||
| Use: "aws [args...]", | ||
| Short: "Run AWS CLI commands against LocalStack", | ||
| Long: `Proxy AWS CLI commands to LocalStack with endpoint, credentials, and region pre-configured. | ||
|
|
||
| Equivalent to running: | ||
| aws --endpoint-url http://localhost:4566 <args> | ||
| with AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, and AWS_DEFAULT_REGION set automatically. | ||
|
|
||
| Examples: | ||
| lstk aws s3 ls | ||
| lstk aws sqs list-queues | ||
| lstk aws s3 mb s3://my-bucket`, | ||
| DisableFlagParsing: true, | ||
| RunE: commandWithTelemetry("aws", tel, func(cmd *cobra.Command, args []string) error { | ||
| port := resolveAWSPort() | ||
| host, _ := endpoint.ResolveHost(port, cfg.LocalStackHost) | ||
| return awscli.Exec(cmd.Context(), "http://"+host, args) | ||
| }), | ||
| } | ||
| } | ||
|
|
||
| func resolveAWSPort() string { | ||
| const defaultPort = "4566" | ||
| if err := config.Init(); err != nil { | ||
| return defaultPort | ||
| } | ||
| appCfg, err := config.Get() | ||
| if err != nil { | ||
| return defaultPort | ||
| } | ||
| for _, c := range appCfg.Containers { | ||
| if c.Type == config.EmulatorAWS { | ||
| return c.Port | ||
| } | ||
| } | ||
| return defaultPort | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| package awscli | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "fmt" | ||
| "io" | ||
| "os" | ||
| "os/exec" | ||
| "strings" | ||
| "sync" | ||
|
|
||
| "github.com/localstack/lstk/internal/output" | ||
| ) | ||
|
|
||
| // stopOnWriteWriter wraps a writer and stops the spinner on first write | ||
| type stopOnWriteWriter struct { | ||
| w io.Writer | ||
| spinner *spinner | ||
| once sync.Once | ||
| } | ||
|
|
||
| func (s *stopOnWriteWriter) Write(p []byte) (int, error) { | ||
| s.once.Do(func() { | ||
| s.spinner.Stop() | ||
| }) | ||
| return s.w.Write(p) | ||
|
Comment on lines
+17
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix spinner lifecycle: possible panic + leaked spinner when no output. Current logic allows two independent stop paths (stdout/stderr wrappers), so Proposed fix type stopOnWriteWriter struct {
- w io.Writer
- spinner *spinner
- once sync.Once
+ w io.Writer
+ stop func()
}
func (s *stopOnWriteWriter) Write(p []byte) (int, error) {
- s.once.Do(func() {
- s.spinner.Stop()
- })
+ s.stop()
return s.w.Write(p)
}
@@
var s *spinner
if isTerminal(os.Stderr) {
s = newSpinner(os.Stderr, "Loading...")
s.Start()
+ var stopOnce sync.Once
+ stopSpinner := func() {
+ stopOnce.Do(func() { s.Stop() })
+ }
+ defer stopSpinner()
// Wrap stdout/stderr to stop spinner on first output
- stopWriter := &stopOnWriteWriter{w: os.Stdout, spinner: s}
- cmd.Stdout = stopWriter
- cmd.Stderr = &stopOnWriteWriter{w: os.Stderr, spinner: s}
+ cmd.Stdout = &stopOnWriteWriter{w: os.Stdout, stop: stopSpinner}
+ cmd.Stderr = &stopOnWriteWriter{w: os.Stderr, stop: stopSpinner}
} else {
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
}Also applies to: 44-56, 58-61 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| func Exec(ctx context.Context, endpointURL string, args []string) error { | ||
| awsBin, err := exec.LookPath("aws") | ||
| if err != nil { | ||
| return fmt.Errorf("aws CLI not found in PATH — install it from https://aws.amazon.com/cli/") | ||
| } | ||
|
|
||
| cmdArgs := make([]string, 0, len(args)+2) | ||
| cmdArgs = append(cmdArgs, "--endpoint-url", endpointURL) | ||
| cmdArgs = append(cmdArgs, args...) | ||
|
|
||
| cmd := exec.CommandContext(ctx, awsBin, cmdArgs...) | ||
| cmd.Stdin = os.Stdin | ||
| cmd.Env = BuildEnv(os.Environ()) | ||
|
|
||
| var s *spinner | ||
| if isTerminal(os.Stderr) { | ||
| s = newSpinner(os.Stderr, "Loading...") | ||
| s.Start() | ||
|
|
||
| // Wrap stdout/stderr to stop spinner on first output | ||
| stopWriter := &stopOnWriteWriter{w: os.Stdout, spinner: s} | ||
| cmd.Stdout = stopWriter | ||
| cmd.Stderr = &stopOnWriteWriter{w: os.Stderr, spinner: s} | ||
| } else { | ||
| cmd.Stdout = os.Stdout | ||
| cmd.Stderr = os.Stderr | ||
| } | ||
|
|
||
| err = cmd.Run() | ||
|
|
||
| if err == nil { | ||
| return nil | ||
| } | ||
|
|
||
| var exitErr *exec.ExitError | ||
| if errors.As(err, &exitErr) { | ||
| return output.NewSilentError(output.NewExitCodeError(exitErr.ExitCode(), err)) | ||
| } | ||
| return err | ||
| } | ||
|
|
||
| func BuildEnv(base []string) []string { | ||
| env := make([]string, len(base), len(base)+3) | ||
| copy(env, base) | ||
|
|
||
| setIfAbsent(&env, "AWS_ACCESS_KEY_ID", "test") | ||
| setIfAbsent(&env, "AWS_SECRET_ACCESS_KEY", "test") | ||
| setIfAbsent(&env, "AWS_DEFAULT_REGION", "us-east-1") | ||
|
|
||
| return env | ||
| } | ||
|
|
||
| func setIfAbsent(env *[]string, key, value string) { | ||
| prefix := key + "=" | ||
| for _, e := range *env { | ||
| if strings.HasPrefix(e, prefix) { | ||
| return | ||
| } | ||
| } | ||
| *env = append(*env, prefix+value) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| package awscli | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| func TestBuildEnvSetsDefaultsWhenAbsent(t *testing.T) { | ||
| base := []string{"PATH=/usr/bin", "HOME=/home/user"} | ||
| env := BuildEnv(base) | ||
|
|
||
| assert.Contains(t, env, "AWS_ACCESS_KEY_ID=test") | ||
| assert.Contains(t, env, "AWS_SECRET_ACCESS_KEY=test") | ||
| assert.Contains(t, env, "AWS_DEFAULT_REGION=us-east-1") | ||
| assert.Contains(t, env, "PATH=/usr/bin") | ||
| assert.Contains(t, env, "HOME=/home/user") | ||
| } | ||
|
|
||
| func TestBuildEnvPreservesExistingValues(t *testing.T) { | ||
| base := []string{ | ||
| "AWS_ACCESS_KEY_ID=custom-key", | ||
| "AWS_SECRET_ACCESS_KEY=custom-secret", | ||
| "AWS_DEFAULT_REGION=eu-west-1", | ||
| } | ||
| env := BuildEnv(base) | ||
|
|
||
| assert.Contains(t, env, "AWS_ACCESS_KEY_ID=custom-key") | ||
| assert.Contains(t, env, "AWS_SECRET_ACCESS_KEY=custom-secret") | ||
| assert.Contains(t, env, "AWS_DEFAULT_REGION=eu-west-1") | ||
| assert.NotContains(t, env, "AWS_ACCESS_KEY_ID=test") | ||
| assert.NotContains(t, env, "AWS_SECRET_ACCESS_KEY=test") | ||
| assert.NotContains(t, env, "AWS_DEFAULT_REGION=us-east-1") | ||
| } | ||
|
|
||
| func TestBuildEnvDoesNotMutateInput(t *testing.T) { | ||
| base := []string{"PATH=/usr/bin"} | ||
| original := make([]string, len(base)) | ||
| copy(original, base) | ||
|
|
||
| BuildEnv(base) | ||
|
|
||
| assert.Equal(t, original, base) | ||
| } | ||
|
|
||
| func TestBuildEnvPartialOverride(t *testing.T) { | ||
| base := []string{"AWS_ACCESS_KEY_ID=custom-key"} | ||
| env := BuildEnv(base) | ||
|
|
||
| assert.Contains(t, env, "AWS_ACCESS_KEY_ID=custom-key") | ||
| assert.Contains(t, env, "AWS_SECRET_ACCESS_KEY=test") | ||
| assert.Contains(t, env, "AWS_DEFAULT_REGION=us-east-1") | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| package awscli | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "io" | ||
| "os" | ||
| "strings" | ||
| "sync" | ||
| "time" | ||
| ) | ||
|
|
||
| var dotFrames = []string{"⣾", "⣽", "⣻", "⢿", "⡿", "⣟", "⣯", "⣷"} | ||
|
|
||
| // ANSI color codes matching lstk's spinner style (color 69 blue) and secondary (color 241 gray) | ||
| const ( | ||
| spinnerColor = "\033[38;5;69m" | ||
| secondaryColor = "\033[38;5;241m" | ||
| resetColor = "\033[0m" | ||
| ) | ||
|
Comment on lines
+7
to
+19
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: didn't we have these properties defined somewhere for Nimbo? I think they're duplicated. Also, In components package, we have spinner defined. Why are we redefining it again in this PR, can you try reuse them? Btw 🎗️, we have this |
||
|
|
||
| type spinner struct { | ||
| out io.Writer | ||
| label string | ||
| stop chan struct{} | ||
| done chan struct{} | ||
| mu sync.Mutex | ||
| } | ||
|
|
||
| func newSpinner(out io.Writer, label string) *spinner { | ||
| return &spinner{ | ||
| out: out, | ||
| label: label, | ||
| stop: make(chan struct{}), | ||
| done: make(chan struct{}), | ||
| } | ||
| } | ||
|
|
||
| func (s *spinner) Start() { | ||
| go func() { | ||
| defer close(s.done) | ||
| tick := time.NewTicker(100 * time.Millisecond) | ||
| defer tick.Stop() | ||
|
|
||
| i := 0 | ||
| for { | ||
| s.mu.Lock() | ||
| _, _ = fmt.Fprintf(s.out, "\r%s%s%s %s%s%s", spinnerColor, dotFrames[i%len(dotFrames)], resetColor, secondaryColor, s.label, resetColor) | ||
| s.mu.Unlock() | ||
|
|
||
| select { | ||
| case <-s.stop: | ||
| s.clearLine() | ||
| return | ||
| case <-tick.C: | ||
| i++ | ||
| } | ||
| } | ||
| }() | ||
| } | ||
|
|
||
| func (s *spinner) Stop() { | ||
| close(s.stop) | ||
| <-s.done | ||
| } | ||
|
|
||
| func (s *spinner) clearLine() { | ||
| s.mu.Lock() | ||
| defer s.mu.Unlock() | ||
| width := len(s.label) + 10 | ||
| _, _ = fmt.Fprintf(s.out, "\r%s\r", strings.Repeat(" ", width)) | ||
| } | ||
|
|
||
| // isTerminal returns true if the writer is a terminal | ||
| func isTerminal(w io.Writer) bool { | ||
| f, ok := w.(*os.File) | ||
| if !ok { | ||
| return false | ||
| } | ||
| stat, err := f.Stat() | ||
| if err != nil { | ||
| return false | ||
| } | ||
| return (stat.Mode() & os.ModeCharDevice) != 0 | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard against empty AWS port config before returning it.
If the AWS container exists but
c.Portis empty, Line 46 returns"", which can yield an invalid endpoint URL. Fall back to default in that case.Proposed fix
func resolveAWSPort() string { const defaultPort = "4566" @@ for _, c := range appCfg.Containers { if c.Type == config.EmulatorAWS { - return c.Port + if c.Port != "" { + return c.Port + } + return defaultPort } } return defaultPort }📝 Committable suggestion
🤖 Prompt for AI Agents