Uh oh!
There was an error while loading. Please reload this page.
[VPEX][13] Document the error-code contract as source of truth - #5964
Merged
Conversation
Collaborator
Integration test reportCommit: 57eb340
8 interesting tests: 4 RECOVERED, 4 SKIP
Top 6 slowest tests (at least 2 minutes):
|
rugpanovforce-pushed
the
dbconnect/13-usage-error
branch
from
July 17, 2026 13:27
f20628a to
6b4a2a4Comparerugpanovforce-pushed
the
dbconnect/14-errcode-reconcile
branch
2 times, most recently
from
July 17, 2026 13:44
3f6b218 to
6ff8a75Compareanton-107
approved these changes
Jul 17, 2026
anton-107
left a comment
Contributor
There was a problem hiding this comment.
Reviewed as part of the full stack (#5960–#5965). The annotated error-code block reads well and makes the constant set a genuine source of truth; documenting why E_PYTHON_POLICY/E_AUTH are deliberately never emitted is a nice touch. Approving.
Since this is the comment-accuracy PR, two adjacent comment fixes it could sweep in while it's here:
[low] Stale
v4fallback comments. After #5962 bumped the default to v5,cmd/environments/compute.go:78and:116still describe the serverless empty-version fallback as v4. This PR touched onlyresult.go, so these were missed.[nit]
ResolveTargetprecedence comment still lists four sources. This PR updated theTargetInfodoc to acknowledge--cluster-name("now five flag sources"), but the sibling precedence comment atlibs/localenv/target.go:64still reads--cluster-id → --serverless-version → --job-id → bundle, omitting--cluster-name. Same class of inaccuracy this PR set out to fix.
Reviewed with AI assistance (build + unit tests + adversarial verification against the checked-out top of stack).
rugpanovforce-pushed
the
dbconnect/14-errcode-reconcile
branch
from
July 20, 2026 08:43
6ff8a75 to
a97a774Comparerugpanovforce-pushed
the
dbconnect/13-usage-error
branch
2 times, most recently
from
July 20, 2026 15:51
c0e7dea to
d5c4d6fCompareReconcile the error codes with the spec table (spec item #10). The code already emits the correct set; the spec's table was stale (it omitted E_NOT_WRITABLE and E_PYTHON_INSTALL and wrongly listed E_UV_MISSING as reserved). Rather than change behavior, annotate each ErrorCode constant with the phase that emits it and document the two codes the CLI deliberately never emits (E_PYTHON_POLICY, E_AUTH), making this block the authoritative reference the spec mirrors. Also fix a TargetInfo doc comment that still said 'four precedence sources' — with --cluster-name there are now five flag sources, though the JSON 'source' value is still one of cluster/serverless/job/bundle. Co-authored-by: Isaac
rugpanovforce-pushed
the
dbconnect/14-errcode-reconcile
branch
from
July 20, 2026 16:37
a97a774 to
57eb340CompareUh oh!
There was an error while loading. Please reload this page.
eng-dev-ecosystem-bot
commented
Jul 20, 2026
Collaborator
Integration test reportCommit: 1e02255
14 interesting tests: 6 FAIL, 5 RECOVERED, 2 SKIP, 1 KNOWN
Top 50 slowest tests (at least 2 minutes):
|
yansonggao-db pushed a commit
to yansonggao-db/cli
that referenced
this pull request
Jul 21, 2026
…icks#5965) Stacked on databricks#5964 → databricks#5963 → databricks#5962 → databricks#5961 → databricks#5960 → databricks#5959. ## What `--serverless-version` is now documented to take a **bare number** (e.g. `5`) rather than `v5`. - Input `5` is the documented form; `v5`/`V5` are still accepted (tolerant). - Both map to the same env key `serverless/serverless-vN`, so the **environments-repo layout is unchanged** (still `serverless/serverless-v5/…`). ## Changes - Flag help: `e.g. v4` → `e.g. 5`; the `E_ENV_UNSUPPORTED` hint suggests `--serverless-version 5`. - `defaultServerlessVersion` stored in bare form (`"5"`); still resolves to `serverless-v5` via `NormalizeServerless`. - Acceptance scripts pass the bare form; env-key goldens unchanged (`serverless-vN`). - Unit tests cover bare + v-prefixed input and the default constant. ## Testing `go build ./...`, lint (0 issues), deadcode clean, unit + acceptance green. 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
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.
Stacked on #5963 (E_USAGE) → #5962 → #5961 → #5960 → #5959.
What
Reconciles the error codes with the
[P0] CLI Changesspec table (spec item #10). No behavior change — the code already emits the correct set. The spec's table was stale:E_NOT_WRITABLE(preflight) andE_PYTHON_INSTALL(provision), andE_UV_MISSINGas "reserved / not emitted" when the CLI does emit it at preflight.Changes
ErrorCodeconstant with the phase that emits it, and document the two spec codes the CLI deliberately never emits:E_PYTHON_POLICY(no signal source yet) andE_AUTH(handled earlier byMustWorkspaceClient). This makes the constant block the authoritative reference the spec mirrors.TargetInfodoc comment that still said "four precedence sources" — with--cluster-namethere are now five flag sources (the JSONsourcevalue is still one of cluster/serverless/job/bundle).Note
The spec doc's error-code table itself (in the VPEX ERD, a Google Doc) has been updated separately to match — that's outside this repo.
Testing
go build ./..., lint (0 issues), deadcode clean, unit + acceptance green (comment-only code change).This pull request and its description were written by Isaac.