[9/?] feat(python-setup): extension wiring & config-view dispatch (DRAFT — blocked) - #2054
Draft
rugpanov wants to merge 9 commits into
Draft
[9/?] feat(python-setup): extension wiring & config-view dispatch (DRAFT — blocked)#2054rugpanov wants to merge 9 commits into
rugpanov wants to merge 9 commits into
Conversation
Contributor
Author
|
🤖 Integration tests ❌ 1 of 35 test jobs failed for |
rugpanov
force-pushed
the
rugpanov/python-setup-wiring
branch
from
July 27, 2026 11:34
0254d5e to
6350a60
Compare
Contributor
Author
|
🤖 Integration tests triggered for |
rugpanov
force-pushed
the
rugpanov/python-setup-wiring
branch
from
July 27, 2026 11:46
6350a60 to
47e53f5
Compare
Contributor
Author
|
🤖 Integration tests ✅ all 35 test jobs passed for |
rugpanov
added a commit
that referenced
this pull request
Jul 28, 2026
#2055) ## Why After a successful uv-native Python environment setup, the extension needs to remember what environment was provisioned, so a later change of compute target can be detected as **drift** (the local env no longer matches the selected target) and surfaced to the user. This lands the persistence key. Scoped deliberately to the storage layer: it builds off `main` with no dependency on the (still-open) wiring stack. The **write site** — the orchestrator's `saveState` — is wired separately when the extension-wiring PR (#2054) lands; it currently holds a `TODO(DECO-27784)` no-op that this key will back. ## What - **`PythonSetupState`** type — `{envKey, pythonVersion, timestamp}`: - `envKey`: the CLI's resolved environment key for the target (e.g. `serverless/serverless-v5`). - `pythonVersion`: the provisioned interpreter minor version (e.g. `3.12`). - `timestamp`: ISO-8601 time of the successful setup. - **`"databricks.pythonSetup.setupState"`** `StorageConfigurations` entry, **workspace-scoped** (drift is per-project). Flows through the existing typed `get`/`set` surface — no new accessor code. - **First `StateStorage` unit test** (in-memory `Memento` fake): round-trip, undefined-before-write, clear-on-undefined, and workspace-not-global placement. ## Verification - `yarn --cwd packages/databricks-vscode build / test:lint / test:unit` — 329 passing (4 new). Build confirms the typed key resolves through `get`/`set`. This pull request and its description were written by Isaac.
rugpanov
added a commit
that referenced
this pull request
Jul 29, 2026
## Why **#2046 (DECO-27785, the setup orchestrator) shows as MERGED but its code never reached `main`.** It merged into the `rugpanov/python-setup-cli-client` branch *after* that branch's PR (#2039) had already been **squash-merged** to main — so the orchestrator landed on a now-stranded branch and `main` has no `PythonSetupEnvironmentSetup`. This re-lands the reviewed orchestrator directly on main. Without this, the feature's first real `PythonSetupCliClient` caller is missing from main, and the extension-wiring PR (#2054) has nothing to build on. ## What `PythonSetupEnvironmentSetup` + its test, **restored byte-identical** from #2046 as reviewed/approved. The orchestrator: - gates on `isVisible`, resolves the compute target, runs the CLI under a progress indicator (with cancellation), - on a ready real-run adopts the venv interpreter + persists setup state, - maps failures to `getPythonSetupErrorMessage`, - coalesces concurrent runs via an in-flight guard. The gate / compute-resolution / adopt / persist / progress collaborators are injected seams, wired to real implementations by the extension-wiring PR (#2054). ## Verification - `build` + `test:lint` — clean. - `test:unit` — **415 passing**. - Note: run with `VSCODE_TEST_VERSION=1.130.0`. The default `"stable"` now pulls VS Code **1.131.0**, whose macOS bundle renames the Electron executable to `Code` and breaks `@vscode/test-electron ^3.0.0` locally (`spawn … Electron ENOENT`). That's a repo-wide test-infra skew, unrelated to this change — flagging it separately. ## Context This is a process-recovery PR. Numbered `[6b/?]` since it restores what `[6/?]` (#2046) was meant to deliver. DECO-27785. This pull request and its description were written by Isaac.
*Why* The Python Environment section must show the single uv-native setup entry when the feature is visible, and the legacy pip checklist otherwise -- the two are mutually exclusive and never rendered together. *What* - Add buildPythonSetupEntry (pure): the single Python Environment row -- a rocket run CTA when not ready, a check status line once ready -- both running the setup command. Plus the PythonSetupEntry interface (isVisible/ready/ onDidChangeState), a narrow slice of the orchestrator so the component carries no controller/gateway dependency. - EnvironmentComponent takes an optional PythonSetupEntry; getChildren returns that single entry when isVisible(), else the unchanged checklist. Refreshes the view on its onDidChangeState. Optional, so the legacy construction is byte-for-byte unchanged when absent. *Verification* yarn --cwd packages/databricks-vscode test:unit (399 passing; 3 new entry-builder cases: run CTA, ready line, single entry). build clean. Co-authored-by: Isaac
*Why* The orchestrator's gate and compute-resolution seams shipped as stubs. This fills them with the real implementations, wiring the pieces from across the stack together for the extension. *What* - makePythonSetupVisibility: the config-view gate -- opted-in (flag on) AND the active project fits (shouldShowPythonSetup: clean uv/greenfield, no competing manager). No project / detection failure -> hidden. - resolveComputeFrom: map the attached compute to a setup-local target. A cluster wins outright; a serverless session resolves to its persisted serverlessVersion (from the compute picker, Diff A) -- no version -> undefined (abort silently, no guessing). - makePythonSetupDeps: assemble the full PythonSetupSetupDeps -- adoptInterpreter via venvInterpreterPath + the MS Python extension, progress/toasts via window. saveState is a documented no-op TODO(DECO-27784) until the storage key lands. *Verification* yarn --cwd packages/databricks-vscode test:unit (408 passing; 9 new: 4 visibility gate, 5 compute resolution incl. cluster-over-serverless and version-less-abort). build clean. Co-authored-by: Isaac
*Why* Final integration: construct the python-setup stack in activate(), register its command, and let the config view dispatch to it. This turns the feature on for opted-in users while leaving everyone else on the unchanged legacy flow. *What* - extension.ts: build PythonSetupManagerDetector (real collector via collectPackageManagerSignals + the active interpreter), PythonSetupCliClient (resolveCliPath: cliPathOverride else bundled), and PythonSetupEnvironmentSetup via makePythonSetupDeps -- projectRoot from workspaceFolderManager, gate from isPythonSetupEnabled + detection, compute from connectionManager (cluster / persisted serverlessVersion), interpreter adopted through the MS Python extension. Register databricks.environment.setupPythonEnv. - ConfigurationDataProvider: thread the (optional) PythonSetupEntry into EnvironmentComponent. - PythonSetupEnvironmentSetup: expose isVisible() (delegates to the gate seam) so it satisfies the component's PythonSetupEntry contract. - package.json: register the setupPythonEnv command. Backward compatibility: the stack is constructed unconditionally but inert -- the gate (flag off, or non-uv project) keeps the entry hidden and the command a no-op, so existing users see the identical legacy checklist. The dependency is optional end-to-end, so nothing changes when it is absent. *Verification* yarn --cwd packages/databricks-vscode build / test:lint / test:unit (408 passing; build type-checks the full activate() wiring). Manual dogfff smoke deferred to a custom-CLI environment. Co-authored-by: Isaac
*Why* The feature must stay hidden until a user opts in. A registered command is palette-invokable by default, which would expose "Set up Python environment (uv)" to every connected user regardless of the opt-in flag or project type. *What* - Add a commandPalette menu entry with "when": "false" for databricks.environment.setupPythonEnv, so it is reachable only via the gated config-view tree entry (which already checks the opt-in flag + package-manager gate), never the palette. Matches the repo's pattern for tree-only commands. *Verification* yarn --cwd packages/databricks-vscode build / test:lint (both clean). Co-authored-by: Isaac
rugpanov
force-pushed
the
rugpanov/python-setup-wiring
branch
from
July 29, 2026 15:07
47e53f5 to
2c6ecb4
Compare
Contributor
Author
|
🤖 Integration tests triggered for |
*Why* The extension-wiring PR left saveState as a TODO(DECO-27784) no-op and carried seam-doc comments claiming the visibility/compute seams were still stubbed. Both are now resolved: the storage key landed (#2055) and this PR wires the real implementations. *What* - Implement saveState (DECO-27784 write half): makePythonSetupDeps stamps the orchestrator's {envKey, pythonVersion} with an ISO-8601 timestamp and forwards it through a new injected persistSetupState seam; extension.ts wires that to stateStorage.set("databricks.pythonSetup.setupState", ...). Workspace-scoped, so drift detection is per-project. - Correct the now-stale PythonSetupSetupDeps seam docs on isVisible/resolveCompute -- they described "the extension passes a stub"; the real gate (flag + shouldShowPythonSetup) and compute resolution (cluster / persisted serverlessVersion) are now wired. - Tests: makePythonSetupDeps.saveState stamps+forwards a valid timestamp; adoptInterpreter routes through the seam with the OS-correct venv path. *Verification* yarn --cwd packages/databricks-vscode build / test:lint (clean); test:unit 429 passing (VSCODE_TEST_VERSION=1.130.0; default stable pulls VS Code 1.131.0 which breaks @vscode/test-electron locally -- repo-wide infra skew, unrelated). Co-authored-by: Isaac
Contributor
Author
|
🤖 Integration tests ✅ all 35 test jobs passed for |
…spatch *Why* Review of PR #2054 surfaced three defects confined to the opted-in uv path: the progress notification was not `cancellable`, so the token never fired and the whole downstream cancellation path (CLI teardown, PythonSetupCancelledError, the orchestrator's quiet-on-cancel branch) was dead code — a user could not cancel a minutes-long project-mutating run. The CLI's streamed output was piped to a no-op `onLog`, so failures showed only a one-line message with the real stderr discarded. And the `EnvironmentComponent` dispatch that guarantees the uv entry and legacy checklist are mutually exclusive had no direct test. *What* - `pythonSetupDeps.ts`: add a `log` seam (`append`/`show`); `withProgress` now passes `cancellable: true` and forwards chunks to `log.append`; `showError` reveals the log channel. Fix the stale visibility-gate comment to state the gate fails *open* on detection failure (matches `detect`'s swallow-to-unknown). - `extension.ts`: create a "Databricks Python Environment Setup" output channel (disposed via subscriptions) and wire it to the `log` seam; guard the fire-and-forget `persistSetupState` write with a `.catch` that logs so a rejected workspaceState update is not an unhandled rejection. - Tests: new `EnvironmentComponent.test.ts` covers all three dispatch branches (no entry / not visible → checklist; visible → uv entry only). Add `withProgress` tests (log forwarding + cancellation token) and a cluster-wins-over-serverless precedence case. *Verification* - `build` / `test:lint` — clean. - `test:unit` — 435 passing (VSCODE_TEST_VERSION=1.130.0), up from 427; 8 new. Co-authored-by: Isaac
Contributor
Author
|
🤖 Integration tests ❌ 1 of 35 test jobs failed for |
Merged
*Why* The Windows unit-test CI job was red: `adoptInterpreter` feeds the venv root through `Uri.file(root)`, whose `.fsPath` is `\proj` on Windows but `/proj` on POSIX. The test asserted the literal `"/proj"`, so it failed only on Windows (`AssertionError: expected '\proj' to equal '/proj'`). Pre-existing since 29e5435; surfaced here to unblock the Windows check. *What* - Assert `adopted[0].root` equals `Uri.file("/proj").fsPath` (the same platform-normalized value the seam receives) instead of the hard-coded literal, so the test holds on both Windows and POSIX. *Verification* - `build` / `test:lint` — clean. - `test:unit` — 435 passing (VSCODE_TEST_VERSION=1.130.0), incl. "adopts the venv interpreter through the injected seam". Co-authored-by: Isaac
Contributor
Author
|
🤖 Integration tests triggered for |
*Why* The uv-native "Set up Python environment" entry is visible whenever the project fits the gate (opt-in flag + uv/greenfield shape), independent of compute. So a user can click the CTA with no cluster attached, or on a serverless session before a version has been chosen — states `ConnectionManager` explicitly supports. In those cases `resolveCompute()` returns `undefined` and the orchestrator aborted with a bare `return`: no toast, no progress, no log. The feature's primary button appeared broken (dead button). Found in code review. *What* - `PythonSetupEnvironmentSetup.runSetup`: the no-compute branch now calls `showError(NO_COMPUTE_TARGET_MESSAGE)` before returning, telling the user to attach a cluster or pick a serverless version. Still runs no CLI / mutates nothing. - `errorMessages.ts`: extract the E_NO_TARGET copy into an exported `NO_COMPUTE_TARGET_MESSAGE` so the orchestrator's pre-flight check and the CLI-reported error surface identical wording (single-sourced, no drift). - Update the orchestrator test to assert the guidance is shown (and the CLI is still not run) instead of asserting a silent no-op. *Verification* - `build` / `test:lint` — clean. - `test:unit` — 435 passing (VSCODE_TEST_VERSION=1.130.0), incl. "guides the user (without running the CLI) when no compute could be resolved". Co-authored-by: Isaac
Contributor
Author
|
🤖 Integration tests ✅ all 35 test jobs passed for |
…y gate *Why* Follow-up review of the wiring surfaced a cross-project defect plus a few inertness/robustness gaps in the opted-in path (existing users stay unaffected). - Interpreter adoption re-read the *live* active project after the CLI await, so switching the active project during a multi-second `uv sync` pointed the newly active project's interpreter setting at the run's venv (config corruption of an unrelated project, announced as success). TOCTOU. - The visibility gate could throw: `activeProjectUri` throws when no project is active (the `?.` never applied), and a rejecting `detect` would propagate — either blanks the Environment section instead of showing the legacy checklist. - The output channel was created for every user on activation, so non-opted-in users got an empty "Databricks Python Environment Setup" entry in the Output dropdown, against the feature's "inert for existing users" goal. - Pre-flight no-compute guidance went through `showError`, which reveals the (empty, nothing-ran) output channel. *What* - `adoptInterpreter(venvPath, projectRoot)`: the orchestrator now passes the cwd captured at the start of the run; the wiring uses it instead of re-reading `projectRoot()`, so adoption always targets the project that was provisioned. - `makePythonSetupVisibility`: wrap the body so any failure degrades to `false` (keep the legacy checklist), and fix the `projectRoot` wiring to honour the `string | undefined` contract (catch the throwing getter). - Create the output channel lazily on first setup output. - Add a `notify` seam (plain warning, no channel reveal) for pre-flight guidance; route the no-compute message through it. *Verification* - `build` / `test:lint` — clean. - `test:unit` — 438 passing (VSCODE_TEST_VERSION=1.130.0), incl. new tests: mid-run project switch adopts the captured root; gate degrades to not-visible when detect/projectRoot throw; no-compute uses notify not showError. Co-authored-by: Isaac
Contributor
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
Contributor
Author
|
🤖 Integration tests ✅ all 35 test jobs passed for |
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
1 participant
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
Final integration for the uv-native Python environment setup: construct the stack in
activate(), register its command, and let the config view dispatch to it. Turns the feature on for opted-in users while leaving everyone else on the unchanged legacy flow.Rebased on fresh main now that all dependencies have merged (#2039 client, #2046/#2063 orchestrator, #2052 picker, #2053 compute-picker, #2055 state key). The diff is now just the wiring.
What
EnvironmentComponentdispatch —buildPythonSetupEntry(pure) + the component returns the single uv entry when visible, else the unchanged legacy checklist (mutually exclusive). AddsisVisible()to the orchestrator so the component can decide dispatch without knowing the gate's inputs.makePythonSetupDeps— fills the orchestrator's seams with real implementations: visibility gate (isPythonSetupEnabled+shouldShowPythonSetup), compute resolution (cluster / persistedserverlessVersion), interpreter adoption via the MS Python extension, progress/toasts viawindow. The progress notification is cancellable (so a user "Cancel" tears down the CLI), and the CLI's streamed output is forwarded to a dedicated "Databricks Python Environment Setup" output channel, which is revealed on failure.extension.ts— construct detector/client/orchestrator, registerdatabricks.environment.setupPythonEnv, create the output channel, and thread the entry throughConfigurationDataProvider.saveStateis wired to thedatabricks.pythonSetup.setupStatestorage key (workspace-scoped), guarded so a rejected write logs rather than surfacing as an unhandled rejection."when": "false"so the command is reachable only via the gated tree entry.Backward compatibility
The stack is constructed unconditionally but inert: the gate keeps the entry hidden (flag off, or non-uv project), the command is palette-hidden, and the
PythonSetupEntrydependency is optional end-to-end. Existing users see the identical legacy checklist — traced explicitly, and covered by the newEnvironmentComponentdispatch tests (no entry / not visible → checklist; visible → uv entry only).Verification
build/test:lint— clean.test:unit— 435 passing (run withVSCODE_TEST_VERSION=1.130.0; defaultstablepulls VS Code 1.131.0, which breaks@vscode/test-electronlocally — repo-wide infra skew, unrelated).This pull request and its description were written by Isaac.