environments: clearer setup-local --help and output (DECO-27977) - #6211
Merged
Conversation
rugpanov
force-pushed
the
setup-local/help-output
branch
from
August 10, 2026 11:40
fbf0eb7 to
ee7692c
Compare
Collaborator
Integration test reportCommit: 15bc2d7
11 interesting tests: 4 RECOVERED, 4 SKIP, 3 flaky
Top 17 slowest tests (at least 2 minutes):
|
misha-db
reviewed
Aug 10, 2026
| cmdio.LogString(ctx, "") | ||
| cmdio.LogString(ctx, "Next steps:") | ||
| if res.VenvPath != "" { | ||
| cmdio.LogString(ctx, " • Activate it: source "+res.VenvPath+"/bin/activate") |
There was a problem hiding this comment.
is res.VenvPath backward slash sepatated on Windows? if true broken path will be logged
misha-db
approved these changes
Aug 10, 2026
rugpanov
changed the base branch from
setup-local/skip-bundle-load-on-error
to
main
August 10, 2026 14:43
Co-authored-by: Isaac
…DECO-27977) Co-authored-by: Isaac
Co-authored-by: Isaac
Co-authored-by: Isaac
Co-authored-by: Isaac
Co-authored-by: Isaac
…-27977) Co-authored-by: Isaac
…e uv prompt (DECO-27977) *Why* The spinner was started before Pipeline.Run and set spinning "Checking your project…" at PhasePreflight. Preflight is also where EnsureAvailable can prompt (confirmUvInstall via cmdio.AskYesOrNo) when uv is not installed. That prompt is written straight to stderr (io.WriteString, not through the tea-program slot), while the spinner's Bubble Tea program repaints the same stderr line at ~5fps. Because SupportsPrompt is a strict superset of SupportsInteractive, whenever the prompt shows the spinner is always also active — so on a real TTY without uv the "[y/N]" line is reliably clobbered and the command looks hung. *What* - Drop the PhasePreflight entry from phaseMessages and start the spinner lazily, on the first phase that has a message (resolve), after the prompt is answered. - Extract a progressSpinner interface and inject the constructor so the lazy-start behavior is unit-testable; Close is now a no-op when no spinner was started. - Add progress_test.go covering "no spinner on preflight" and "starts once, reused". *Verification* go build ./..., go test ./cmd/environments ./libs/localenv, go test ./acceptance -run TestAccept/localenv, gofmt, go vet, and golangci-lint on the package all pass. Co-authored-by: Isaac
…27977) *Why* Follow-up review nits on the text output. Two doc comments no longer matched the code (text mode no longer prints phase headers; --debug logs entered phases, not the full list on dry runs). The failure line could read "Setup failed during setup." for an unknown phase, and a multi-line uv error only indented its first line, so continuation lines read as unrelated output. *What* - renderResult: fix the stale "prints phase headers" and "--debug" comments. - failureClause: return "" (leading-space clauses otherwise) for unknown/missing phase, so the line reads "Setup failed." instead of "Setup failed during setup." - Indent every line of the error message via a small indent() helper, keeping a folded multi-line uv stderr grouped under the failure line. *Verification* go test ./cmd/environments and go test ./acceptance -run TestAccept/localenv pass (failure goldens byte-identical — the change is a no-op for single-line messages); gofmt, go vet, and golangci-lint on the package are clean. Co-authored-by: Isaac
…-classic compute (DECO-27977) *Why* Two review findings on the new success summary. (1) The "Activate it" next-step hard-coded the Unix form `source .venv/bin/activate`; on Windows uv lays the venv out under Scripts\ and `source` is not a cmd/PowerShell builtin, so a Windows user copying the hint hits "source is not recognized" and a path that does not exist. A maintainer (@misha-db) flagged the same line. (2) ComputeInfo.Label() fell through to the raw "dbr/..." env key for a --job-task bound to classic compute (which carries SparkVersion but no ClusterID) — re-exposing exactly the internal detail the summary was meant to hide, only for that case. *What* - Add activateHint(venvPath), branching on runtime.GOOS the same way venvPython does: `<venv>\Scripts\activate` on Windows, `source <venv>/bin/activate` on Unix. - Add a SparkVersion case to Label() (after ClusterID, so --cluster-id/bundle-cluster targets still render "cluster <id>") that shows "DBR <version>" for job-classic. - Tests: TestActivateHint (per-OS), and a "job classic" case in TestComputeInfoLabel. *Verification* go build ./..., go test ./cmd/environments ./libs/localenv, go test ./acceptance -run TestAccept/localenv (success output is not golden-tested; no golden churn), gofmt, go vet, and golangci-lint on both packages all pass. Co-authored-by: Isaac
…y (DECO-27977)
*Why*
renderSuccess runs only on a non-dry-run success — renderResult returns earlier
for JSON output, failures, and dry runs. On that path res.VenvPath is always set:
the validate phase (the last step of a successful run) assigns res.VenvPath = venvDir
unconditionally. So the two `if res.VenvPath != ""` guards were always true and the
`else` branch ("Activate the .venv it created…") was unreachable — dead code and a
misleading suggestion that a success could leave VenvPath empty.
*What*
- Remove both VenvPath guards and the dead else branch; print the "Virtual env"
row and activation next-steps unconditionally.
- Document the invariant on renderSuccess so the removal is self-explanatory.
*Verification*
go build ./..., go test ./cmd/environments (existing render tests all set VenvPath,
still pass), gofmt, go vet, and golangci-lint on the package all pass.
Co-authored-by: Isaac
…ECO-27977) *Why* The acceptance test acceptance/localenv/merge-warnings landed on main with a golden that still carried the old per-phase log (preflight/resolve/... ok), which this PR removes from the default text output. After rebasing onto main the golden is stale and fails TestAccept/localenv/merge-warnings. *What* - Regenerate acceptance/localenv/merge-warnings/output.txt with -update: the --dry-run text now shows warnings + the Plan block without the phase log. merge-warnings-json is unchanged (JSON contract untouched). *Verification* go test ./acceptance -run TestAccept/localenv and go test ./cmd/environments ./libs/localenv all pass; only this one golden changed. Co-authored-by: Isaac
rugpanov
force-pushed
the
setup-local/help-output
branch
from
August 10, 2026 14:51
cc08376 to
15bc2d7
Compare
rugpanov
enabled auto-merge
August 10, 2026 14:53
Collaborator
Integration test reportCommit: 0807413
38 interesting tests: 24 flaky, 6 FAIL, 5 RECOVERED, 2 SKIP, 1 KNOWN
Top 50 slowest tests (at least 2 minutes):
|
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
3 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.
Changes
Makes
databricks environments setup-localexplain itself in plain language and produce clear output, addressing two usability problems from the M5 bug bash (DECO-27977):--helpcopy now leads with what the command does and when to use it ("get a local Python environment that matches your Databricks compute so local runs behave like Databricks"), instead of opening with implementation detail (environment keys, constraints, matched.venv). Added a usageExamplesblock.databricks-connectversions, the virtual env, whetherpyproject.tomlwas created or updated (with backup), and next steps. Runs show live per-phase progress via a spinner in interactive terminals.--debug/--output json.--debugand in--output json.Implementation notes:
localenv.Reporterhook lets the command show live progress without the pipeline depending oncmdio; it only notifies and does not change pipeline control flow or error codes.ComputeInfo.Label()renders a friendly compute label ("serverless 4", "cluster …").--output jsoncontract is unchanged (schemaVersion: 1); all JSON acceptance goldens are byte-identical. Text-mode goldens were regenerated.Why
The command is aimed at users setting up local development against Databricks compute, but its help and success output spoke in internal terms (phases, env keys,
fromCache) rather than telling the user what happened and what to do next. This reframes the default text experience around user value while preserving the phase detail for debugging and machine consumers.Tests
acceptance/localenv/*text goldens; confirmed JSON goldens unchanged.renderResult(success summary, constraints-only omission, failure, canceled, dry-run),ComputeInfo.Label(), and the pipeline phase-startReporter.go build ./...,go test ./cmd/environments ./libs/localenv,go test ./acceptance -run TestAccept/localenv, andgolangci-linton the affected packages all pass.--helpoutput.This pull request and its description were written by Isaac.