diff --git a/NOTICE b/NOTICE index 01f9df98246..fb80040b33a 100644 --- a/NOTICE +++ b/NOTICE @@ -151,6 +151,10 @@ mattn/go-isatty - https://github.com/mattn/go-isatty Copyright (c) Yasuhiro MATSUMOTO License - https://github.com/mattn/go-isatty/blob/master/LICENSE +mattn/go-runewidth - https://github.com/mattn/go-runewidth +Copyright (c) 2016 Yasuhiro Matsumoto +License - https://github.com/mattn/go-runewidth/blob/master/LICENSE + sabhiram/go-gitignore - https://github.com/sabhiram/go-gitignore Copyright (c) 2015 Shaba Abhiram License - https://github.com/sabhiram/go-gitignore/blob/master/LICENSE diff --git a/cmd/lakebox/config.go b/cmd/lakebox/config.go index f5ad8ad8081..6ca1d6350ce 100644 --- a/cmd/lakebox/config.go +++ b/cmd/lakebox/config.go @@ -38,7 +38,7 @@ Three knobs are independent — pass any combination: the sandbox after this much idle time. Pass 0 (or 0s) to clear and revert to the manager's global default (10m). Valid range when set: - 60s to 24h. + 1m to 24h. --no-autostop[=true|false] When true, the sandbox is exempt from idle-driven auto-stop entirely. The @@ -157,9 +157,15 @@ func checkIdleSecs(secs int64) (int64, error) { return 0, nil // clear / revert to global default } if secs < minIdleTimeoutSecs || secs > maxIdleTimeoutSecs { + // Format both the bounds and the offending value as Go-style + // durations to match the input form the user typed and the + // flag's --help text (Anwell flagged the prior `86400s` / + // `90000s` echoes as confusing — same unit as input now). return 0, fmt.Errorf( - "idle-timeout must be 0 (clear) or between %ds and %ds, got %ds", - minIdleTimeoutSecs, maxIdleTimeoutSecs, secs, + "idle-timeout must be 0 (clear) or between %s and %s, got %s", + formatDurationSecs(minIdleTimeoutSecs), + formatDurationSecs(maxIdleTimeoutSecs), + formatDurationSecs(secs), ) } return secs, nil diff --git a/cmd/lakebox/list.go b/cmd/lakebox/list.go index 2bcef9c4243..7e951ffd7be 100644 --- a/cmd/lakebox/list.go +++ b/cmd/lakebox/list.go @@ -8,6 +8,7 @@ import ( "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/cmdio" + "github.com/mattn/go-runewidth" "github.com/spf13/cobra" ) @@ -101,21 +102,27 @@ Example: // rendered only when at least one entry sets a display name // different from the ID — there's no point in a column of // pet-names that duplicate the ID column. + // All column widths are measured in *terminal cells*, not + // bytes or runes — emoji and CJK glyphs render as 2 cells + // despite being 1 rune / multi-byte, and using len() here + // (which counts bytes) misaligns the row whenever a `--name` + // includes wide characters. runewidth.StringWidth gives the + // East-Asian-Width-corrected cell count. idCol := 10 autostopCol := 8 nameCol := 4 showName := false for _, e := range entries { - if l := len(e.SandboxID); l > idCol { + if l := runewidth.StringWidth(e.SandboxID); l > idCol { idCol = l } - if l := len(e.autoStopLabel()); l > autostopCol { + if l := runewidth.StringWidth(e.autoStopLabel()); l > autostopCol { autostopCol = l } if e.Name != "" && e.Name != e.SandboxID { showName = true } - if l := len(e.Name); l > nameCol { + if l := runewidth.StringWidth(e.Name); l > nameCol { nameCol = l } } @@ -152,11 +159,11 @@ Example: } // Pad each cell manually so visible-width alignment is // preserved after the helpers wrap them with ANSI escapes. - idPad := max(idCol-len(id), 0) + idPad := max(idCol-runewidth.StringWidth(id), 0) st := status(ctx, e.Status) - stPad := max(statusCol-len(e.Status), 0) + stPad := max(statusCol-runewidth.StringWidth(e.Status), 0) as := e.autoStopLabel() - asPad := max(autostopCol-len(as), 0) + asPad := max(autostopCol-runewidth.StringWidth(as), 0) idStr := cmdio.Bold(ctx, id) if strings.EqualFold(e.Status, "running") { idStr = cmdio.Bold(ctx, cmdio.Cyan(ctx, id)) @@ -166,7 +173,7 @@ Example: if nm == "" || nm == id { nm = "-" } - nmPad := max(nameCol-len(nm), 0) + nmPad := max(nameCol-runewidth.StringWidth(nm), 0) nmStr := nm if nm == "-" { nmStr = cmdio.Faint(ctx, "-") diff --git a/cmd/lakebox/resolve_test.go b/cmd/lakebox/resolve_test.go index 761bf9cdb6b..9a8595191da 100644 --- a/cmd/lakebox/resolve_test.go +++ b/cmd/lakebox/resolve_test.go @@ -126,3 +126,27 @@ func TestRemoveSandboxMissingIsNoop(t *testing.T) { require.NoError(t, removeSandbox(ctx, "p", "nope")) assert.Equal(t, []cachedSandbox{{ID: "keep-me"}}, getSandboxes(ctx, "p")) } + +// Removing the last sandbox for a profile must also drop the profile's +// cached gateway host — otherwise lakebox.json accumulates orphan +// gatewayHosts entries that no longer correspond to any sandbox. +func TestRemoveSandboxClearsOrphanGatewayHost(t *testing.T) { + ctx, _ := stateCtx(t) + require.NoError(t, setSandboxes(ctx, "p", []cachedSandbox{{ID: "only-one"}})) + require.NoError(t, setGatewayHost(ctx, "p", "gw.example.test")) + require.Equal(t, "gw.example.test", getGatewayHost(ctx, "p")) + + require.NoError(t, removeSandbox(ctx, "p", "only-one")) + assert.Empty(t, getSandboxes(ctx, "p")) + assert.Empty(t, getGatewayHost(ctx, "p"), "gateway host must be cleared when the last sandbox is removed") +} + +// Removing one of many sandboxes must NOT touch the gateway host — it +// still applies to the remaining sandboxes on the profile. +func TestRemoveSandboxKeepsGatewayHostWhileSandboxesRemain(t *testing.T) { + ctx, _ := stateCtx(t) + require.NoError(t, setSandboxes(ctx, "p", []cachedSandbox{{ID: "keep"}, {ID: "drop"}})) + require.NoError(t, setGatewayHost(ctx, "p", "gw.example.test")) + require.NoError(t, removeSandbox(ctx, "p", "drop")) + assert.Equal(t, "gw.example.test", getGatewayHost(ctx, "p")) +} diff --git a/cmd/lakebox/sshkey.go b/cmd/lakebox/sshkey.go index c0a5d915315..5b7f3829aa9 100644 --- a/cmd/lakebox/sshkey.go +++ b/cmd/lakebox/sshkey.go @@ -10,6 +10,7 @@ import ( "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/cmdio" + "github.com/mattn/go-runewidth" "github.com/spf13/cobra" ) @@ -115,9 +116,11 @@ Examples: out := cmd.OutOrStdout() blank(out) + // Measure in terminal cells (runewidth) so wide / emoji + // glyphs in `--name` don't misalign the row. nameCol := 4 for _, k := range keys { - if l := len(k.Name); l > nameCol { + if l := runewidth.StringWidth(k.Name); l > nameCol { nameCol = l } } @@ -136,10 +139,10 @@ Examples: for _, k := range keys { // Pad NAME manually from the raw width because cmdio.Faint // wraps the cell in ANSI escapes that throw off `%-*s`. - displayName, visibleNameLen := k.Name, len(k.Name) + displayName, visibleNameLen := k.Name, runewidth.StringWidth(k.Name) if displayName == "" { displayName = cmdio.Faint(ctx, "(unset)") - visibleNameLen = len("(unset)") + visibleNameLen = runewidth.StringWidth("(unset)") } namePad := max(nameCol-visibleNameLen, 0) gutter := " " diff --git a/cmd/lakebox/start.go b/cmd/lakebox/start.go index 5b97145b098..dd6b7a5dd40 100644 --- a/cmd/lakebox/start.go +++ b/cmd/lakebox/start.go @@ -1,7 +1,10 @@ package lakebox import ( + "context" "fmt" + "strings" + "time" "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/libs/cmdctx" @@ -9,16 +12,31 @@ import ( "github.com/spf13/cobra" ) +// Bounds for `start`'s "wait until Running" loop. The server's StartSandbox +// RPC returns immediately with status="Creating" (reused for cold start — +// see F10), so the CLI polls until it actually reaches Running. Matches +// `create`'s blocking semantics so scripts can chain start → ssh / start → +// config without racing the cold boot. The 10-minute timeout covers +// Mitch's observed cold-start range (5–13 minutes) for the common case; +// truly stuck sandboxes still surface as a timeout rather than hanging +// the script forever. +const ( + startPollInterval = 2 * time.Second + startWaitTimeout = 10 * time.Minute +) + func newStartCommand() *cobra.Command { cmd := &cobra.Command{ Use: "start ", Short: "Start a stopped Lakebox environment", Long: `Start a stopped Lakebox environment. -Boots the backing microVM and brings the sandbox to Running. -'databricks lakebox ssh' already auto-starts a stopped sandbox on -connection, so this command is mostly useful for pre-warming an -environment without immediately connecting. +Boots the backing microVM and blocks until the sandbox reaches +Running (or up to 5 minutes). 'databricks lakebox ssh' already +auto-starts a stopped sandbox on connection, so this command is +mostly useful for pre-warming an environment without immediately +connecting, or when a script needs to be sure the sandbox is up +before continuing. Starting an already-running sandbox is a no-op. @@ -57,10 +75,56 @@ Example: _ = setGatewayHost(ctx, profile, updated.GatewayHost) _ = upsertSandbox(ctx, profile, updated.SandboxID, updated.Name) - s.ok("Started " + cmdio.Bold(ctx, updated.SandboxID)) + // Already up — short-circuit so we don't pretend to wait + // when there's nothing to wait for. + if strings.EqualFold(updated.Status, "running") { + s.ok("Already running " + cmdio.Bold(ctx, updated.SandboxID)) + return nil + } + + final, err := waitForRunning(ctx, api, s, updated.SandboxID) + if err != nil { + s.fail("Failed to start " + lakeboxID) + return err + } + _ = upsertSandbox(ctx, profile, final.SandboxID, final.Name) + + s.ok("Started " + cmdio.Bold(ctx, final.SandboxID)) return nil }, } return cmd } + +// waitForRunning polls the sandbox until it reaches Running or the timeout +// elapses. The spinner is updated with the elapsed seconds each poll so the +// user can tell progress is happening, and a transition to an unexpected +// terminal state (Stopped / Terminated / Failed) short-circuits with a +// useful error rather than waiting out the full timeout. +func waitForRunning(ctx context.Context, api *lakeboxAPI, s *spinner, id string) (*sandboxEntry, error) { + start := time.Now() + deadline := start.Add(startWaitTimeout) + for { + sb, err := api.get(ctx, id) + if err != nil { + return nil, fmt.Errorf("polling status of %s: %w", id, err) + } + switch strings.ToLower(sb.Status) { + case "running": + return sb, nil + case "stopped", "terminated", "failed": + return nil, fmt.Errorf("sandbox %s reached unexpected state %q while starting", id, sb.Status) + } + elapsed := time.Since(start).Round(time.Second) + s.Update(fmt.Sprintf("Starting %s… (%s)", id, elapsed)) + if time.Now().After(deadline) { + return nil, fmt.Errorf("sandbox %s did not reach Running within %s (last seen %s)", id, startWaitTimeout, sb.Status) + } + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-time.After(startPollInterval): + } + } +} diff --git a/cmd/lakebox/state.go b/cmd/lakebox/state.go index 7e388684378..e30160563bc 100644 --- a/cmd/lakebox/state.go +++ b/cmd/lakebox/state.go @@ -219,6 +219,12 @@ func upsertSandbox(ctx context.Context, profile, id, name string) error { // removeSandbox drops a single cached entry for `profile`. Called from // `delete` so the cache doesn't keep referencing sandboxes that no // longer exist server-side. +// +// When the removal empties the sandbox list for a profile, also drop +// the profile's `GatewayHosts` entry — there is nothing for the +// gateway hostname to apply to until the user creates a new sandbox, +// and leaving the entry behind accumulates orphan state across the +// lifecycle of a profile (per Mitch's "Delete cleanup" CUJ). func removeSandbox(ctx context.Context, profile, id string) error { state, err := loadState(ctx) if err != nil { @@ -228,6 +234,10 @@ func removeSandbox(ctx context.Context, profile, id string) error { for i, s := range existing { if s.ID == id { state.Sandboxes[profile] = append(existing[:i], existing[i+1:]...) + if len(state.Sandboxes[profile]) == 0 { + delete(state.Sandboxes, profile) + delete(state.GatewayHosts, profile) + } return saveState(ctx, state) } } diff --git a/go.mod b/go.mod index f35dadb31ce..ddeceadd2f1 100644 --- a/go.mod +++ b/go.mod @@ -24,6 +24,7 @@ require ( github.com/hexops/gotextdiff v1.0.3 // BSD-3-Clause github.com/jackc/pgx/v5 v5.9.2 // MIT github.com/mattn/go-isatty v0.0.22 // MIT + github.com/mattn/go-runewidth v0.0.23 // MIT github.com/palantir/pkg/yamlpatch v1.5.0 // BSD-3-Clause github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // BSD-2-Clause github.com/quasilyte/go-ruleguard/dsl v0.3.22 // BSD-3-Clause @@ -80,7 +81,6 @@ require ( github.com/lucasb-eyer/go-colorful v1.4.0 // indirect github.com/mattn/go-colorable v0.1.14 // indirect github.com/mattn/go-localereader v0.0.1 // indirect - github.com/mattn/go-runewidth v0.0.23 // indirect github.com/mitchellh/hashstructure/v2 v2.0.2 // indirect github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 // indirect github.com/muesli/cancelreader v0.2.2 // indirect