Skip to content

[6/?] feat(python-setup): setup orchestrator (first CLI-client caller)#2046

Open
rugpanov wants to merge 1 commit into
rugpanov/python-setup-cli-clientfrom
rugpanov/python-setup-orchestrator
Open

[6/?] feat(python-setup): setup orchestrator (first CLI-client caller)#2046
rugpanov wants to merge 1 commit into
rugpanov/python-setup-cli-clientfrom
rugpanov/python-setup-orchestrator

Conversation

@rugpanov

Copy link
Copy Markdown
Contributor

Stacked on #2039 (base = rugpanov/python-setup-cli-client). Review after #2039; the diff of interest is the single controllers/ commit on top. Once #2039 merges, GitHub retargets this to main automatically.

Why

#2039 adds PythonSetupCliClient with no call site — as raised in review, the API is hard to judge without seeing it used. This PR adds the orchestrator that actually drives the client end-to-end, so the client's usage ships alongside it.

What

PythonSetupEnvironmentSetup (python-setup/controllers/) — the first real caller of PythonSetupCliClient:

  1. gate → resolve compute → cli.run(invocation, {cwd, onLog}) under a progress indicator,
  2. on success: adopt the provisioned venv interpreter + persist {envKey, pythonVersion} for drift detection,
  3. on failure: surface the mapped getPythonSetupErrorMessage copy,
  4. PythonSetupCancelledError → treated as a silent user action (no error toast).

It consumes the real client surface (run(invocation{compute}, {cwd, onLog}), isPythonSetupSuccess, getPythonSetupErrorMessage), and depends on the client structurally (CliRunner) so the flow is unit-testable without spawn/vscode.

The two not-yet-built pieces are injected seams, not hard deps

Per the agreed approach, the gate and serverless-version selection are stubbed behind injected functions with explicit TODOs, so this PR stacks only on the CLI-client change and doesn't pull in unbuilt tickets:

  • isVisible()TODO(DECO-27781 / [4/?] feat(python-setup): package-manager gating & live detection #2044): shouldShowPythonSetup(...). The wiring stub returns false, so the feature is inert end-to-end for now.
  • resolveCompute()TODO(DECO-27782): serverless-version picker (scoreServerlessVersions + pickServerlessVersion). The wiring stub returns undefined, so a serverless setup is a no-op until that ticket; the cluster case can already resolve.

The real gate and picker drop in through these seams in their own tickets (and the extension.ts wiring lands in DECO-27786). Nothing is wired into the extension in this PR.

Verification

  • yarn --cwd packages/databricks-vscode test:unit — 344 passing (6 new orchestrator cases: success→ready+adopt+saveState, gate-closed→no run, no-compute→no run, CLI failure→mapped error, cancel→silent, onDidChangeState fires).
  • test:lint — clean · build — clean.

This pull request and its description were written by Isaac.

@rugpanov

Copy link
Copy Markdown
Contributor Author

🤖 Integration tests triggered for 3508259b — ⏳ running.
View run

@rugpanov
rugpanov force-pushed the rugpanov/python-setup-orchestrator branch from 3508259 to b48d2ae Compare July 24, 2026 13:01
@rugpanov
rugpanov temporarily deployed to test-trigger-is July 24, 2026 13:01 — with GitHub Actions Inactive
@rugpanov

Copy link
Copy Markdown
Contributor Author

🤖 Integration tests triggered for b48d2ae8 — ⏳ running.
View run

*Why*
The CLI client (PythonSetupCliClient) shipped with no call site, which makes its
API hard to review in isolation. This adds the orchestrator that actually
drives it end-to-end, so the client's usage lands alongside the client.

*What*
- Add PythonSetupEnvironmentSetup (python-setup/controllers): decide whether to
  run, resolve the compute target, invoke cli.run() under a progress indicator,
  then on success adopt the provisioned venv interpreter and persist
  {envKey, pythonVersion} for drift detection; on failure surface the mapped
  getPythonSetupErrorMessage copy; treat PythonSetupCancelledError as a silent
  user action.
- Consumes the real client surface: run(invocation{compute}, {cwd, onLog, token}),
  isPythonSetupSuccess, getPythonSetupErrorMessage. The client is depended on
  structurally (CliRunner) so the flow is unit-testable without spawn/vscode.
- The two not-yet-built pieces are injected seams with explicit TODOs, not hard
  dependencies:
    * isVisible()  -> TODO(DECO-27781/#2044): shouldShowPythonSetup(...). Wiring
      stub returns false, so the feature is inert end-to-end for now.
    * resolveCompute() -> TODO(DECO-27782): serverless-version picker. Wiring
      stub returns undefined, so a serverless setup is a no-op until then.

Review fixes (Claude + Codex, two rounds):
- Cancellation is no longer dead code: ProgressTask now exposes a
  CancellationLike token alongside the log sink, and setup() threads it into
  cli.run({cwd, onLog, token}). The production withProgress passes through the
  progress notification's own token, so a user "Cancel" tears down the
  project-mutating CLI instead of being a silent no-op.
- Re-entrancy guard: overlapping setup() calls coalesce onto a single in-flight
  run (cleared in finally, incl. on rejection) so two setup-local processes
  can't race the same cwd's writes/backup.
- Strict real-run success: an ok:true result must carry venvPath AND
  target/resolved. Missing any (dry-run shape / CLI-schema drift) is treated as
  a failure instead of flipping `ready` with no interpreter adopted or with no
  drift baseline persisted.

*Verification*
yarn --cwd packages/databricks-vscode test:unit (351 passing; 13 orchestrator
cases, incl. no-open-project, raw-error-reject branch, success-without-venvPath,
success-missing-target/resolved, token threaded to cli.run, re-entrancy
coalescing, and in-flight guard cleared on rejection). test:lint and build
(tsc --build) clean.

Co-authored-by: Isaac
@rugpanov
rugpanov force-pushed the rugpanov/python-setup-orchestrator branch from b48d2ae to fa992f3 Compare July 24, 2026 14:23
@rugpanov
rugpanov temporarily deployed to test-trigger-is July 24, 2026 14:23 — with GitHub Actions Inactive
@rugpanov
rugpanov temporarily deployed to test-trigger-is July 24, 2026 14:23 — with GitHub Actions Inactive
@rugpanov

Copy link
Copy Markdown
Contributor Author

🤖 Integration tests triggered for fa992f32 — ⏳ running.
View run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant