Uh oh!
There was an error while loading. Please reload this page.
[VPEX][5/8] Add local-env pipeline, detection, and package-manager interface - #5828
Closed
rugpanov wants to merge 26 commits into
Closed
[VPEX][5/8] Add local-env pipeline, detection, and package-manager interface#5828rugpanov wants to merge 26 commits into
rugpanov wants to merge 26 commits into
Conversation
Contributor
Waiting for approvalCould not determine reviewers from git history. Eligible reviewers: Suggestions based on git history. See OWNERS for ownership rules. |
Collaborator
Integration test reportCommit: ac062fa
8 interesting tests: 4 SKIP, 2 KNOWN, 1 RECOVERED, 1 flaky
Top 10 slowest tests (at least 2 minutes):
|
First of a stacked series adding `databricks local-env python sync`, which provisions a local Python environment matched to a Databricks compute target. The feature lands across small, single-concern PRs; each layer is independently reviewable and adds no user-facing surface until the final PR wires the command in. This PR is the foundation the rest of the stack builds on: - result.go: the result types and the --json / E_* error contract that every phase reports through (Result, PipelineError, ErrorCode, PhaseName, PhaseStatus, Mode, TargetInfo, ResolvedInfo, Plan, Warning), plus the command-path constants (local-env / python / sync) defined once. - envkey.go: mapping a compute target to an environment key and parsing the Python minor from a requires-python specifier. Nothing imports this package yet, so the CLI is unchanged. The unexported filesystem/artifact constants and the canonical phase-order slice live with the pipeline that consumes them (a later PR in the stack). Co-authored-by: Isaac
rugpanovforce-pushed
the
dbconnect/05-pipeline
branch
from
July 3, 2026 13:20
6289ebf to
b94edd5Compare…pecifiers Review of the foundation layer flagged that PythonMinorFromRequires took the first MAJOR.MINOR in the string via first-match regex. For a multi-clause requires-python where the exclusive upper bound comes first — e.g. "<3.13,>=3.10" — it returned 3.13, the version the "<3.13" clause forbids, because PEP 440 clause order is arbitrary. The result feeds PM.EnsurePython, so the tool could target a Python the constraint excludes. Prefer a lower-bound / pinning clause (>=, >, ==, ~=, ===) and only fall back to the first version when none is present. Adds multi-clause test coverage; the prior tests exercised only single-bound specifiers. Co-authored-by: Isaac
rugpanovforce-pushed
the
dbconnect/05-pipeline
branch
from
July 3, 2026 15:26
b94edd5 to
dcdd6f3Comparerugpanovforce-pushed
the
dbconnect/05-pipeline
branch
from
July 3, 2026 15:31
dcdd6f3 to
aa3fa6fComparerugpanovforce-pushed
the
dbconnect/05-pipeline
branch
from
July 3, 2026 15:41
aa3fa6f to
965e3d0Comparerugpanovforce-pushed
the
dbconnect/05-pipeline
branch
from
July 3, 2026 17:55
965e3d0 to
d6aeed6Compare…dden version Round-2 review of the foundation layer noted that when a requires-python has no lower-bound/pin clause at all (only upper-bound or exclusion, e.g. "<3.13,!=3.12"), PythonMinorFromRequires fell back to the first number and returned 3.13 — a version the specifier forbids. Such a spec has no floor to install from, so it now errors rather than guessing. A bare "3.12" (no operator) is still accepted as a valid floor. Co-authored-by: Isaac
Fourth in the stacked local-env series. merge.go rewrites only the env-owned sections of a pyproject.toml and preserves every other byte (comments, ordering, whitespace, CRLF). It updates requires-python and the databricks-connect pin in place, and maintains a marker-bracketed managed [tool.uv] constraint block. The operation is idempotent: feeding its own output back in is byte-identical. RenderFreshPyproject produces a complete managed file for a greenfield project. Two correctness properties this file has to get right, both covered by tests that parse the result as TOML rather than asserting on strings: - When the user's pyproject.toml already has a [tool.uv] table with a non-constraint key, the managed constraint-dependencies nests header-less inside that table instead of emitting a second [tool.uv] header (two headers for one table is invalid TOML that uv rejects). - Single- vs multi-line constraint-dependencies detection tracks real bracket depth outside strings and comments, so an opening line that contains a "]" inside an element (e.g. "requests[security]~=2.0") or a trailing comment is not misread as single-line and mis-stripped. Depends on the constraints PR for the Constraints type. Still dormant. Co-authored-by: Isaac
Review of the merge layer found the databricks-connect rewrite was not scoped to [dependency-groups].dev: - It walked every line of [dependency-groups] and rewrote the first databricks-connect element found, so a pin in a sibling group (docs/test) was clobbered instead of the dev entry. It now locates the dev assignment and edits only within that array's line span. - The single-line branch replaced the databricks-connect token anywhere on the dev line, including inside a trailing comment (user content). Replacement is now confined to the array portion (through its closing "]"); the trailing comment is preserved byte-for-byte. - mergeRequiresPython replaced the whole line, dropping an inline comment such as `requires-python = ">=3.10" # maintained by platform team`. It now reattaches the trailing comment, honoring the byte-preservation contract for everything outside the managed value. Adds tests for each: sibling-group untouched, comment not clobbered, inline comment preserved. Co-authored-by: Isaac
Round-2 review found the merge did not tolerate a trailing comment on a table header line (e.g. "[project] # note"). Two consequences: mergeRequiresPython could not find a commented [project] header (managed value silently not updated), and worse, the [dependency-groups] end bound could run past a commented sibling header, so a dev key in a following table was mistaken for [dependency-groups].dev and rewritten. tableHeaderRe now allows a trailing comment and a new headerName helper matches a header by its bracketed name ignoring the comment; both the table lookup and the [tool.uv] attachment check use it. Also documents that line endings are a whole-file property (a CRLF-anywhere file is emitted entirely as CRLF), which is faithful for real single-ending pyproject.toml files. Co-authored-by: Isaac
…ldren Round-3 review found tableHeaderRe did not match TOML array-of-tables headers like "[[tool.uv.index]]". A [tool.uv] table's end bound therefore ran through its [[tool.uv.index]] children, and the header-less managed constraint block could be inserted inside the last index item instead of under [tool.uv], producing wrong or invalid uv config. tableHeaderRe now matches both "[...]" and "[[...]]"; headerName returns the full "[[...]]" token so an array-of-tables header is never treated as the same table as its "[...]" parent. Adds a merge test with a [[tool.uv.index]] child. Co-authored-by: Isaac
…oject files
Two correctness gaps in the line-based merge, from review:
- The scanner does not track TOML multi-line string state ("""...""" / '''...''')
across lines, so a line inside such a string that looks like a table header,
key, or bracket could mis-scope the managed-region edits and silently corrupt
the file. MergeManaged now detects a multi-line string delimiter and returns an
error (surfaced as E_MERGE) rather than risking corruption — the guarantee the
merge exists to uphold. Multi-line strings are rare in a pyproject.toml.
- A partial existing file with no [project] table would be "merged" with
requires-python silently skipped. MergeManaged now errors when [project] is
absent (greenfield goes through RenderFreshPyproject, which always writes it).
Adds tests for both bail-outs.
Co-authored-by: IsaacFifth in the stacked local-env series. This completes the libs/localenv engine; the uv backend and the CLI command follow in later PRs. - pipeline.go: the six-phase orchestrator (preflight → resolve → fetch → merge → provision → validate) that ties the earlier layers together and records per-phase status into the Result. It drives everything through the PackageManager interface, so it is unit-tested end to end against a fake package manager and a stub compute client. This file also owns the filesystem/artifact constants and the canonical phase-order slice, which the foundation PR intentionally left to their consumer. - pkgmanager.go: the PackageManager interface the pipeline provisions through (implemented by uv in the next PR). - detect.go: package-manager detection (uv vs conda vs pip) and the writability preflight, biased toward uv since uv's native project file is the pyproject.toml this command already merges. A read error on an existing pyproject.toml that is not a not-exist error is treated as a failure rather than as greenfield, so a permission or transient I/O error can never cause the project to be overwritten with a fresh file and no backup. Depends on the foundation, target, constraints, and merge PRs. The package is still dormant: nothing under cmd/ imports it. Co-authored-by: Isaac
Review of the pipeline layer found: - applyMerge treated every os.Stat error on the backup as "does not exist" and proceeded to copyFile over it. An existing-but-unstattable backup (permission or I/O error) would thus be overwritten with the already-merged content, destroying the canonical original the merge base relies on. It now only creates the backup on os.ErrNotExist and fails on any other stat error. - A backup-copy failure was reported with DiskMutated=false even though copyFile creates/truncates the .bak path and can leave a partial file. That path now reports DiskMutated=true. - majorVersion accepted any non-empty prefix before the first dot, so a malformed version like "bad.version" yielded major "bad" and could be compared as a real major. It now returns "" for a non-numeric major so the validate phase rejects malformed versions. Adds a direct applyMerge test for the unstattable-backup branch and extends the majorVersion cases with non-numeric inputs. Co-authored-by: Isaac
Round-4 review noted preflight ran ensureWritable even under --check, which creates and removes a temp file (a disk mutation in a dry run) and fails a read-only project the user only wants to inspect. The probe now runs only for a real (non-check) run; it exists to fail fast before a write that --check never performs. Co-authored-by: Isaac
Round-5 review noted that although --check now skips the writability probe, it still called PackageManager.EnsureAvailable, which may install the manager (uv) when missing — another disk mutation in a dry run. Preflight now performs neither write-side step under --check (neither is needed to compute the plan) and reports the phase as "check". Adds a test with a PackageManager that errors on every method, asserting --check still succeeds and produces a plan. Co-authored-by: Isaac
Use the NewResult constructor added in the foundation layer so the --json phases/warnings arrays are guaranteed non-nil (emit [] not null) at the real construction site, then override Phases with the canonical pending list. Co-authored-by: Isaac
rugpanovforce-pushed
the
dbconnect/05-pipeline
branch
from
July 7, 2026 08:35
de8581b to
be1b6b3CompareZaSkittles 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 was referenced Jul 8, 2026
bradleyjamrozik-origindigital pushed a commit
to Origin-Digital-LLC/databricks-cli
that referenced
this pull request
Jul 13, 2026
…eflight (databricks#5850) ## Why - PR databricks#5828 (`[VPEX][5/8]`) reads as +3,466 / 15 files on GitHub, but that is an artifact: PRs 1–4 were squash-merged into `main` while `dbconnect/05-pipeline` still sits on the pre-merge parent, so GitHub re-counts all of the already-merged code. The genuinely-new content in that layer is +1,165 / 5 files. - This splits that real delta at its one clean dependency seam. This PR is the leaf half: the `PackageManager` interface, manager detection, and the writability preflight — none of which reference the pipeline orchestrator. ## What - **`pkgmanager.go`** — the `PackageManager` interface the pipeline provisions through (implemented by the uv backend in a later PR). - **`detect.go`** — uv-vs-not-uv detection (biased toward uv, whose native project file is the `pyproject.toml` this command drives), the non-blaming guidance message shown for an unsupported manager, and the `ensureWritable` preflight. - The `pyprojectFile` constant moves here from `pipeline.go`, since detection is its first consumer. ## Testing strategy - `detect_test.go` covers the detection matrix (greenfield, uv lock, plain pyproject, conda/pip precedence), `ensureWritable` on a writable and a non-existent dir, and the unsupported-manager guidance message. - Gates: `go build`, `go test`, `golangci-lint` (0 issues), `deadcode`, `gofmt` — all green. --- ## About this stack This is one of a series of small, stacked PRs that together add the `databricks local-env python sync` command. It supersedes the pipeline half of databricks#5828, split further at the interface/detection seam. **Review bottom-up.** This PR and #5b (the six-phase pipeline orchestrator, stacked on this one) together replace databricks#5828. This pull request and its description were written by Isaac.
bradleyjamrozik-origindigital pushed a commit
to Origin-Digital-LLC/databricks-cli
that referenced
this pull request
Jul 13, 2026
…5851) ## Why - The earlier layers (resolve / fetch / merge) need an orchestrator that runs them in order, reports structured per-phase status, and provisions the environment — without mutating disk in `--check` dry-run mode. - This is the core half of the split described in databricks#5850: the six-phase pipeline itself, stacked on the package-manager/detection layer. ## What - **`pipeline.go`** — the six-phase orchestrator (`preflight → resolve → fetch → merge → provision → validate`). It records per-phase status into `Result` and returns typed `PipelineError`s carrying `FailurePhase` and `DiskMutated`. Under `--check` it computes and reports the plan (with a diff) and performs no writes, skipping the writability probe and package-manager availability (neither is needed to compute the plan). - The merge base is the **live `pyproject.toml`**. `MergeManaged` rewrites only the three managed regions and is idempotent on its own output, so a re-run preserves edits the user made between syncs. The `.bak` is a one-time safety copy of the pre-sync original (created only when none exists, on the first sync of an existing project); an unreadable or unstattable existing file fails loudly rather than being misread as greenfield and overwritten. - `constraints-only` stops *managing* the databricks-connect pin rather than removing it: a greenfield project renders `dev = []` (no databricks-connect), and an existing project that already pins databricks-connect keeps its pin untouched. ## Testing strategy - End-to-end unit tests of the phase machine against a fake `PackageManager` + stub compute + `httptest` server: `--check` mutates nothing (even on a read-only dir / without a package manager) and reports a plan that matches a real re-run (no spurious backup, empty diff when idempotent), greenfield vs. existing, the merge basing on the live file so between-sync edits survive, backup safety on unreadable/unstattable files, constraints-only behavior, preflight exits (`E_MANAGER_UNSUPPORTED` / `E_UV_MISSING`), and phase/error attribution (`pipeline_test.go`). - Gates: `go build`, `go test`, `golangci-lint` (0 issues), `deadcode`, `gofmt` — all green. --- ## About this stack **Review bottom-up.** This PR targets databricks#5850 as its base, so its diff shows only the pipeline layer. This PR and databricks#5850 together supersede databricks#5828. This pull request and its description were written by Isaac.
rugpanov
commented
Jul 16, 2026
ContributorAuthor
Superseded by the split PRs #5850 (5a), #5851 (5b), and #5854 (5c), all merged to main. Closing with no unlanded content: the diff between |
swearyangupta pushed a commit
to swearyangupta/cli
that referenced
this pull request
Jul 17, 2026
…ks#5832) ## Why - The engine needs a real `PackageManager` implementation (uv) and a CLI entry point so a user can actually run the feature. - Wiring it in while keeping it `Hidden` lets the command be dogfooded and exercised by acceptance tests without becoming user-visible until the stack is complete. - This is the first layer reachable from `main`, so it also makes the whole `libs/localenv` package live for the `deadcode` checker. ## What - **`libs/localenv/uv.go`** — the uv implementation of `PackageManager`: discover/install uv, install the Python minor, `uv sync`, seed pip into the venv, validate; plus the `pip.conf` → `UV_INDEX_URL` bridge for Databricks-managed machines. - **`cmd/localenv/`** — the command tree matching `local-env python sync`: a `local-env` group (`Hidden: true`), a `python` subgroup, and the `sync` verb. Parent nodes use `root.ReportUnknownSubcommand`; `sync` uses `cobra.NoArgs`, resolves flags/bundle target, builds the `Pipeline` with the uv manager, and renders text or `--json`. All Cobra `Use` values + the `--json` command field come from the `libs/localenv` constants. - **`cmd/cmd.go`** — registers the group. ## Testing strategy - Unit tests for uv helper logic (discovery, `pip.conf` index-url, arg builders, stderr surfacing) (`uv_test.go`). - Runtime smoke test of the hidden 3-level command: absent from top-level help; help works at each level; unknown subcommand exits non-zero; bare group shows help; flags + mutual-exclusion + `NoArgs` behave. - Gates: `go build ./...`, `go test`, `golangci-lint`, `deadcode` (whole tree, no pragmas), `gofmt` — all green. - `cmd/localenv/` unit tests are intentionally deferred to the acceptance PR (databricks#5833), per the repo convention that user-visible CLI output is covered by acceptance tests. --- ## 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 (this PR targets `main`), so its diff shows only that layer. Layers 1–5 have merged; the original layer-5 PR (databricks#5828) was split into 5a/5b/5c during review. | # | PR | What | Status | |---|----|------|--------| | 1 | databricks#5823 | foundation: result types + env-key mapping | merged | | 2 | databricks#5824 | compute-target resolution | merged | | 3 | databricks#5826 | constraint fetch + offline cache | merged | | 4 | databricks#5827 | formatting-preserving pyproject.toml merge | merged | | 5a | databricks#5850 | package-manager interface + detection | merged | | 5b | databricks#5851 | six-phase pipeline orchestrator | merged | | 5c | databricks#5854 | --check cache purity, greenfield name, dbc insertion | merged | | 6 | **databricks#5832 ← you are here** | 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.
swearyangupta pushed a commit
to swearyangupta/cli
that referenced
this pull request
Jul 17, 2026
## Why - The command's user-visible behavior — text and `--json` output, and every error path — needs end-to-end coverage against the real CLI. - `cmd/localenv/` carries no unit tests by design, so acceptance tests are where that surface is verified (repo convention: user-visible CLI output is covered by acceptance tests). ## What - **`acceptance/localenv/`** — 9 scenarios driven through the (hidden) command against the in-process fake server: `help` (three-level tree), `no-target` (`E_NO_TARGET`), `flag-conflict` (Cobra mutual-exclusion), `manager-unsupported` (conda project → clean P1 exit), `env-unsupported` (404 → `E_ENV_UNSUPPORTED` at fetch), `json-error` (`--output json` error object), `serverless-check` (dry-run plan), `serverless-json` (`--json` plan), and `constraints-only`. - Scripts use `local-env python sync` and the `DATABRICKS_LOCALENV_CONSTRAINT_SOURCE` override; goldens show the `local-env python sync` command field and managed marker. No source changes. ## Testing strategy - Goldens generated with `-update` and verified **stable on a clean re-run** (no `-update`); all 9 subtests pass. - `musterr` guards the five expected-failure scenarios; `trace` shows the three output-producing ones. - Full acceptance suite run to confirm no regressions elsewhere (only pre-existing, environment-specific failures unrelated to this change). - Diff confined to `acceptance/localenv/`. - Independently verified by a review subagent (PASS — goldens, scripts, stubs, stale-refs, hygiene) and by codex (no issues). --- ## 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.** Layers 1–5 have merged (the original layer-5 PR databricks#5828 was split into 5a/5b/5c during review). | # | PR | What | Status | |---|----|------|--------| | 1 | databricks#5823 | foundation: result types + env-key mapping | merged | | 2 | databricks#5824 | compute-target resolution | merged | | 3 | databricks#5826 | constraint fetch + offline cache | merged | | 4 | databricks#5827 | formatting-preserving pyproject.toml merge | merged | | 5a | databricks#5850 | package-manager interface + detection | merged | | 5b | databricks#5851 | six-phase pipeline orchestrator | merged | | 5c | databricks#5854 | --check cache purity, greenfield name, dbc insertion | merged | | 6 | databricks#5832 | uv backend + CLI command (registered hidden) | | | 7 | **databricks#5833 ← you are here** | 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 freeto 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
--checkdry-run mode.libs/localenvengine.What
pipeline.go— the six-phase orchestrator (preflight → resolve → fetch → merge → provision → validate), recording per-phase status intoResultand returning typedPipelineErrors (withFailurePhase+DiskMutated). Under--checkit computes/reports the plan and performs no writes (skips both the writability probe and package-manager availability). Backups are created only onos.ErrNotExist(an unreadable existing file fails rather than being overwritten).pkgmanager.go— thePackageManagerinterface the pipeline provisions through.detect.go— package-manager detection (uv vs conda vs pip) + the writability preflight.Testing strategy
PackageManager+ stub compute +httptestserver:--checkmutates nothing (even on a read-only dir / without a package manager), greenfield vs existing, backup safety on unreadable/unstattable files, and phase/error attribution (pipeline_test.go,detect_test.go).go build,go test,golangci-lint,deadcode,gofmt— all green.--check-purity 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 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 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.
This pull request and its description were written by Isaac.