[VPEX][8/8] Unveil local-env: make the command visible, add help + changelog#5835
Open
rugpanov wants to merge 1 commit into
Open
[VPEX][8/8] Unveil local-env: make the command visible, add help + changelog#5835rugpanov wants to merge 1 commit into
rugpanov wants to merge 1 commit into
Conversation
Contributor
Waiting for approvalBased on git history, these people are best suited to review:
Eligible reviewers: Suggestions based on git history. See OWNERS for ownership rules. |
This was referenced Jul 6, 2026
Collaborator
Integration test reportCommit: 27f8064
11 interesting tests: 4 SKIP, 3 FAIL, 3 RECOVERED, 1 KNOWN
Top 9 slowest tests (at least 2 minutes):
|
This was referenced Jul 6, 2026
rugpanov
force-pushed
the
dbconnect/07-acceptance
branch
from
6c2fadc to
c45ca0c
Compare
rugpanov
force-pushed
the
dbconnect/08-unveil
branch
from
3123548 to
8008a65
Compare
Eighth and final PR in the stacked series. Flips the feature from implemented-but-hidden to released: - cmd/localenv/localenv.go: remove Hidden:true so `local-env` appears in `databricks --help` and completion. - acceptance/help/output.txt: regenerated global-help golden now lists local-env under Developer Tools. - NEXT_CHANGELOG.md: announce `databricks local-env python sync`. This is the only PR that changes user-visible surface; everything it exposes was reviewed and tested in PRs 1-7. Reverting this PR alone re-hides the feature without touching any implementation. Co-authored-by: Isaac
rugpanov
force-pushed
the
dbconnect/07-acceptance
branch
from
c45ca0c to
25d07fe
Compare
rugpanov
force-pushed
the
dbconnect/08-unveil
branch
from
8008a65 to
27f8064
Compare
ZaSkittles
pushed a commit
to ZaSkittles/cli
that referenced
this pull request
Jul 7, 2026
databricks#5823) ## Why - The `local-env` feature needs a shared vocabulary before any behavior can be built: the result shape, the error taxonomy, and how a compute target maps to an environment key. - Landing these contract types first lets every later layer (resolve / fetch / merge / pipeline / command) depend on stable, reviewed definitions. - Kept deliberately minimal and dependency-free so it reviews on its own and stays `unused`/`deadcode`-clean with no consumers yet. ## What - **`result.go`** — the `--json` / `E_*` output contract: `Result`, `PipelineError`, `ErrorCode`, `PhaseName`, `PhaseStatus`, `Mode`, `TargetInfo`, `ResolvedInfo`, `Plan`, `Warning`; plus the command-path constants (`local-env` / `python` / `sync`) defined in one place. - **`envkey.go`** — `EnvKeyForServerless` / `EnvKeyForSparkVersion` / `NormalizeServerless`, and `PythonMinorFromRequires` (clause-aware: returns the effective highest lower bound of a `requires-python`). - No wiring into `cmd/`, so the CLI is unchanged. Filesystem/artifact constants and the phase-order slice deliberately live with their consumer (PR 5). ## Testing strategy - Unit tests for the error/type contract (`result_test.go`) and env-key mapping incl. multi-clause / strict-`>` / no-floor `requires-python` cases (`envkey_test.go`). - Gates: `go build`, `go test`, `golangci-lint`, `deadcode`, `gofmt` — all green. - Reviewed with codex across several rounds to convergence (all findings fixed or explicitly rejected as speculative). --- ## About this stack This is one of a series of small, stacked PRs that together add the `databricks local-env python sync` command — it provisions a local Python environment (Python version, `databricks-connect` pin, and dependency constraints) matched to a selected Databricks compute target. The work was split from one large branch into single-concern layers so each is independently reviewable; the command is kept hidden until the final PR so nothing is user-visible mid-stack. **Review bottom-up.** Each PR targets the previous one as its base branch, so its diff shows only that layer. | # | PR | What | |---|----|------| | 1 | **databricks#5823 ← you are here** | foundation: result types + env-key mapping | | 2 | databricks#5824 | compute-target resolution | | 3 | databricks#5826 | constraint fetch + offline cache | | 4 | databricks#5827 | formatting-preserving pyproject.toml merge | | 5 | databricks#5828 | six-phase pipeline + detection + package-manager interface | | 6 | databricks#5832 | uv backend + CLI command (registered hidden) | | 7 | databricks#5833 | acceptance tests | | 8 | databricks#5835 | unveil (unhide + help + changelog) | This pull request and its description were written by Isaac.
ZaSkittles
pushed a commit
to ZaSkittles/cli
that referenced
this pull request
Jul 7, 2026
## Why - `local-env` must turn the user's compute selection into a single environment key before it can fetch anything, and the selection can come from several places with a defined precedence. - Isolating resolution behind a narrow seam keeps it testable without a live workspace and keeps SDK details out of the engine. ## What - **`target.go`** — `ResolveTarget` with ordered precedence `--cluster` → `--serverless` → `--job` → bundle target, producing a `TargetInfo` + env key. - Compute lookups go through the narrow `ComputeClient` interface (stubbable in tests). - `ValidateTargetFlags` rejects more than one target flag; `ResolveTarget` runs it up front so a non-Cobra caller can't silently resolve the wrong target. - Classic-compute jobs read the Spark version from the documented first return of `GetJobSparkVersion` (not the recorded-version third return). ## Testing strategy - Unit tests against a stub `ComputeClient` covering each precedence branch, the mutually-exclusive-flags error, and the job classic-compute contract (`target_test.go`). - Gates: `go build`, `go test`, `golangci-lint`, `deadcode`, `gofmt` — all green. - Reviewed with codex to a clean pass. --- ## About this stack This is one of a series of small, stacked PRs that together add the `databricks local-env python sync` command — it provisions a local Python environment (Python version, `databricks-connect` pin, and dependency constraints) matched to a selected Databricks compute target. The work was split from one large branch into single-concern layers so each is independently reviewable; the command is kept hidden until the final PR so nothing is user-visible mid-stack. **Review bottom-up.** Each PR targets the previous one as its base branch, so its diff shows only that layer. | # | PR | What | |---|----|------| | 1 | databricks#5823 | foundation: result types + env-key mapping | | 2 | **databricks#5824 ← you are here** | compute-target resolution | | 3 | databricks#5826 | constraint fetch + offline cache | | 4 | databricks#5827 | formatting-preserving pyproject.toml merge | | 5 | databricks#5828 | six-phase pipeline + detection + package-manager interface | | 6 | databricks#5832 | uv backend + CLI command (registered hidden) | | 7 | databricks#5833 | acceptance tests | | 8 | databricks#5835 | unveil (unhide + help + changelog) | This pull request and its description were written by Isaac.
ZaSkittles
pushed a commit
to ZaSkittles/cli
that referenced
this pull request
Jul 7, 2026
…icks#5826) ## Why - Once a target resolves to an env key, `local-env` needs the pinned Python version, `databricks-connect` version, and dependency constraints published for that key. - The fetch must degrade gracefully offline and distinguish “this environment isn't published” from “the network is down,” because those call for different user action. - The artifact host must be a Databricks-owned, access-controlled location and must never default to a personal repo — whoever controls the host controls what the CLI installs. ## What - **`constraints.go`** — fetches the per-environment `pyproject.toml`, parses `requires-python`, the `databricks-connect` pin, and `[tool.uv]` `constraint-dependencies`, and caches it on disk. - **Host parameterization** — no host is hardcoded: `RepoConstraintBaseURL` reads the repo (`owner/name`) from the temporary `DATABRICKS_LOCALENV_CONSTRAINT_REPO` env var and builds a `raw.githubusercontent.com/<repo>/main` URL; the built-in default is empty. When unset it returns `""` and `FetchConstraints` reports the missing source as a fetch-phase `E_FETCH` error (so there is no untrusted default, and the failure flows through the normal phase/JSON reporting). Once `databricks/environments` can publish, that becomes the hardcoded default and the env var is no longer required. - Failure classification: **404** → `E_ENV_UNSUPPORTED` (no cache fallback — a distinct non-transient condition); **transport / non-404** → `E_FETCH` with fallback to the last-good cached copy. - Robustness: validate the body (parse + require `requires-python`) **before** caching so a bad 2xx can't poison the cache; atomic cache write (mkdir + temp-file + rename); dedicated `http.Client` with a 30s timeout; body read bounded by `io.LimitReader` at 1 MiB; `databricks-connect` matched by leading package name under PEP 503 normalization (so `Databricks_Connect` matches, `databricks-connectors` does not); cache filename = readable slug + sha256 suffix to prevent collisions. ## Testing strategy - Unit tests with an `httptest` server: 200-parse, 404 → `E_ENV_UNSUPPORTED`, transport failure + cache fallback, missing-`requires-python` rejection, PEP 503 name matching, cache-dir creation, collision-free filenames, oversized-body rejection (`constraints_test.go`). - Host resolution: `TestRepoConstraintBaseURL` (env var → URL, unset → `""`, whitespace treated as unset) and `TestFetchConstraintsNoSourceConfigured` (empty host → `E_FETCH` naming the env var). - Gates: `go build`, `go test`, `golangci-lint`, `deadcode`, `gofmt` — all green. - Reviewed with codex to a clean pass (several fetch/cache edge-case fixes landed from review). --- ## About this stack This is one of a series of small, stacked PRs that together add the `databricks local-env python sync` command — it provisions a local Python environment (Python version, `databricks-connect` pin, and dependency constraints) matched to a selected Databricks compute target. The work was split from one large branch into single-concern layers so each is independently reviewable; the command is kept hidden until the final PR so nothing is user-visible mid-stack. **Review bottom-up.** Each PR targets the previous one as its base branch, so its diff shows only that layer. | # | PR | What | |---|----|------| | 1 | databricks#5823 | foundation: result types + env-key mapping | | 2 | databricks#5824 | compute-target resolution | | 3 | **databricks#5826 ← you are here** | constraint fetch + offline cache | | 4 | databricks#5827 | formatting-preserving pyproject.toml merge | | 5 | databricks#5828 | six-phase pipeline + detection + package-manager interface | | 6 | databricks#5832 | uv backend + CLI command (registered hidden) | | 7 | databricks#5833 | acceptance tests | | 8 | databricks#5835 | unveil (unhide + help + changelog) | This pull request and its description were written by Isaac.
ZaSkittles
pushed a commit
to ZaSkittles/cli
that referenced
this pull request
Jul 7, 2026
…atabricks#5827) ## Why - `local-env` must apply the resolved Python version and constraints to the user's `pyproject.toml` without disturbing their own content — comments, ordering, formatting, and unrelated config must survive untouched. - Re-running must be safe and idempotent, and a greenfield project needs a sensible file created from scratch. - This is the most intricate logic in the feature, so it lands in its own PR for focused review. ## What - **`merge.go`** — a formatting-preserving merge that rewrites only the env-owned regions (`requires-python`, the `databricks-connect` entry in `[dependency-groups].dev`, and a marker-bracketed managed `[tool.uv]` block) and preserves every other byte incl. CRLF; idempotent. `RenderFreshPyproject` builds a complete managed file for a greenfield project. - Scoping/robustness: managed `constraint-dependencies` nests header-less inside an existing user `[tool.uv]` (never a duplicate header); single- vs multi-line array detection tracks real bracket depth outside strings/comments; the `databricks-connect` rewrite is confined to `dev` and leaves trailing comments alone; `requires-python`'s inline comment is preserved; table-header parsing tolerates inline comments and recognizes `[[array.of.tables]]`. ## Testing strategy - Unit tests that parse the merged output as TOML (not just substring checks), covering idempotency, CRLF preservation, user-key preservation, the duplicate-`[tool.uv]` case, bracket-in-element arrays, sibling-group/comment non-clobbering, and `[[tool.uv.index]]` children (`merge_test.go`). - Gates: `go build`, `go test`, `golangci-lint`, `deadcode`, `gofmt` — all green. - Reviewed with codex to a clean pass (multiple TOML-corruption edge cases were caught and fixed). --- ## About this stack This is one of a series of small, stacked PRs that together add the `databricks local-env python sync` command — it provisions a local Python environment (Python version, `databricks-connect` pin, and dependency constraints) matched to a selected Databricks compute target. The work was split from one large branch into single-concern layers so each is independently reviewable; the command is kept hidden until the final PR so nothing is user-visible mid-stack. **Review bottom-up.** Each PR targets the previous one as its base branch, so its diff shows only that layer. | # | PR | What | |---|----|------| | 1 | databricks#5823 | foundation: result types + env-key mapping | | 2 | databricks#5824 | compute-target resolution | | 3 | databricks#5826 | constraint fetch + offline cache | | 4 | **databricks#5827 ← you are here** | formatting-preserving pyproject.toml merge | | 5 | databricks#5828 | six-phase pipeline + detection + package-manager interface | | 6 | databricks#5832 | uv backend + CLI command (registered hidden) | | 7 | databricks#5833 | acceptance tests | | 8 | databricks#5835 | unveil (unhide + help + changelog) | This pull request and its description were written by Isaac.
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.
Why
local-env python syncwas built, reviewed, and tested behindHidden: trueacross PRs 1–7. This PR makes it visible — the single, deliberate “release” step.What
cmd/localenv/localenv.go— removeHidden: true(and the now-stale “hidden while landing” comment) solocal-envappears indatabricks --helpand completion.acceptance/help/output.txt— regenerated global-help golden now listslocal-envunder Developer Tools (betweenbundleandsync).NEXT_CHANGELOG.md— announcedatabricks local-env python syncunder### CLI.Testing strategy
go build ./...,golangci-lint,deadcode,gofmt— all green.helpandlocalenvacceptance suites pass without-update(golden matches reality).local-envnow appears in top-level help; sub-help still works; behavior otherwise unchanged (e.g. no-target still exits 1) — only visibility flipped.Hidden, changelog accuracy) and by codex (no issues).About this stack
This is the final PR of a series of small, stacked PRs that together add the
databricks local-env python synccommand — it provisions a local Python environment (Python version,databricks-connectpin, and dependency constraints) matched to a selected Databricks compute target. The work was split from one large branch into single-concern layers so each is independently reviewable; the command was kept hidden until this PR so nothing was user-visible mid-stack.Review bottom-up. Each PR targets the previous one as its base branch, so its diff shows only that layer.
This pull request and its description were written by Isaac.