Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions acceptance/localenv/cluster-name-ambiguous-json/out.test.toml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

42 changes: 42 additions & 0 deletions acceptance/localenv/cluster-name-ambiguous-json/output.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
{
"schemaVersion": 1,
"command": "environments setup-local",
"ok": false,
"mode": "default",
"dryRun": true,
"greenfield": false,
"phases": [
{
"phase": "preflight",
"status": "ok"
},
{
"phase": "resolve",
"status": "error"
},
{
"phase": "fetch",
"status": "pending"
},
{
"phase": "merge",
"status": "pending"
},
{
"phase": "provision",
"status": "pending"
},
{
"phase": "validate",
"status": "pending"
}
],
"warnings": [],
"error": {
"code": "E_RESOLVE",
"failurePhase": "resolve",
"message": "resolving cluster name \"dup\": there are 2 active clusters named \"dup\"; use --cluster-id to disambiguate",
"diskMutated": false
},
"durationMs": 0
}
1 change: 1 addition & 0 deletions acceptance/localenv/cluster-name-ambiguous-json/script
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
musterr $CLI environments setup-local --cluster-name dup --dry-run --output json
20 changes: 20 additions & 0 deletions acceptance/localenv/cluster-name-ambiguous-json/test.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
EnvMatrix.DATABRICKS_BUNDLE_ENGINE = []

[Env]
DATABRICKS_LOCALENV_CONSTRAINT_SOURCE = "$DATABRICKS_HOST"

# Two active clusters share the same name, so name→ID resolution is genuinely
# ambiguous and errors as an actionable E_RESOLVE at the resolve phase.
# (A terminated cluster sharing the name would be filtered out server-side via
# ListClustersFilterBy, so it does not cause a spurious collision — see
# GetClusterByName.)
[[Server]]
Pattern = "GET /api/2.1/clusters/list"
Response.Body = '''
{
"clusters": [
{"cluster_id": "cid-123", "cluster_name": "dup", "state": "RUNNING", "spark_version": "15.4.x-scala2.12"},
{"cluster_id": "cid-999", "cluster_name": "dup", "state": "RUNNING", "spark_version": "14.3.x-scala2.12"}
]
}
'''
3 changes: 3 additions & 0 deletions acceptance/localenv/flag-conflict-json/out.test.toml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

42 changes: 42 additions & 0 deletions acceptance/localenv/flag-conflict-json/output.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
{
"schemaVersion": 1,
"command": "environments setup-local",
"ok": false,
"mode": "default",
"dryRun": false,
"greenfield": false,
"phases": [
{
"phase": "preflight",
"status": "error"
},
{
"phase": "resolve",
"status": "pending"
},
{
"phase": "fetch",
"status": "pending"
},
{
"phase": "merge",
"status": "pending"
},
{
"phase": "provision",
"status": "pending"
},
{
"phase": "validate",
"status": "pending"
}
],
"warnings": [],
"error": {
"code": "E_USAGE",
"failurePhase": "preflight",
"message": "invalid compute target flags: flags --cluster-id and --serverless-version are mutually exclusive; specify at most one",
"diskMutated": false
},
"durationMs": 0
}
1 change: 1 addition & 0 deletions acceptance/localenv/flag-conflict-json/script
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
musterr $CLI environments setup-local --cluster-id abc --serverless-version v4 --output json
1 change: 1 addition & 0 deletions acceptance/localenv/flag-conflict-json/test.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
EnvMatrix.DATABRICKS_BUNDLE_ENGINE = []
8 changes: 7 additions & 1 deletion acceptance/localenv/flag-conflict/output.txt
Original file line number Diff line number Diff line change
@@ -1 +1,7 @@
Error: if any flags in the group [cluster-id cluster-name serverless-version job-id] are set none of the others can be; [cluster-id serverless-version] were all set
preflight error invalid compute target flags: flags --cluster-id and --serverless-version are mutually exclusive; specify at most one
resolve pending
fetch pending
merge pending
provision pending
validate pending
For more detail, re-run with --debug, or --output json to share a structured report.
14 changes: 7 additions & 7 deletions cmd/environments/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ func addTargetFlags(cmd *cobra.Command) {
cmd.Flags().String("constraint-source-url", "", "URL for the constraint source (overrides "+envConstraintSource+")")
// Hide constraint-source-url from casual --help output; it is a power-user escape hatch.
_ = cmd.Flags().MarkHidden("constraint-source-url")
cmd.MarkFlagsMutuallyExclusive("cluster-id", "cluster-name", "serverless-version", "job-id")
// The mutual exclusivity of the target flags is enforced in the pipeline's
// preflight (as E_USAGE) rather than via cmd.MarkFlagsMutuallyExclusive, so
// the conflict is reported through the phase/JSON contract the --output json
// consumer relies on, instead of a bare pre-RunE Cobra error.
}

// runPipeline builds and runs the setup-local Pipeline.
Expand All @@ -77,12 +80,9 @@ func runPipeline(cmd *cobra.Command) error {
Serverless: serverless,
Job: job,
}
// ValidateTargetFlags is kept despite MarkFlagsMutuallyExclusive above:
// it also validates the library path (no Cobra equivalent) and guards
// non-Cobra call paths such as tests that invoke runPipeline directly.
if err := libslocalenv.ValidateTargetFlags(targetFlags); err != nil {
return err
}
// Flag validation (including mutual exclusivity) happens in the pipeline's
// preflight, so a conflict is reported as E_USAGE through the phase/JSON
// contract rather than as a bare error here.

mode := libslocalenv.ModeDefault
if constraintsOnly {
Expand Down
11 changes: 10 additions & 1 deletion libs/localenv/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,16 @@ func (p *Pipeline) Run(ctx context.Context) (*Result, error) {
// run drives the phases and returns the first phase error. Result bookkeeping
// (phase status, error object) is handled by fail / markOK.
func (p *Pipeline) run(ctx context.Context) error {
// Phase: preflight — manager detection, writability, package-manager availability.
// Phase: preflight — flag validation, manager detection, writability,
// package-manager availability.
//
// Incompatible target flags are a usage error (E_USAGE), reported at preflight
// before any other work so the failure flows through the phase/JSON reporting
// (a plain Cobra mutual-exclusion error would print no command JSON object,
// which the --output json consumer needs).
if err := ValidateTargetFlags(p.Flags); err != nil {
return p.fail(PhasePreflight, false, NewError(ErrUsage, err, "invalid compute target flags"))
}
// P0 supports only uv; any other detected manager is a clean, non-blaming exit.
if m := detectManager(p.ProjectDir); m != managerUv {
return p.fail(PhasePreflight, false, NewError(ErrManagerUnsupported, nil, "%s", managerGuidance(m)))
Expand Down
19 changes: 19 additions & 0 deletions libs/localenv/pipeline_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,25 @@ func newTestServer(t *testing.T) *httptest.Server {
}))
}

func TestPipelineRejectsConflictingTargetFlagsAtPreflight(t *testing.T) {
// Incompatible target flags are a usage error surfaced as E_USAGE at
// preflight, before any manager/writability/fetch work.
dir := writeProject(t)
p := &Pipeline{
Mode: ModeDefault, Check: true, ProjectDir: dir, CacheDir: t.TempDir(),
Flags: TargetFlags{Cluster: "abc", Serverless: "v4"},
Compute: stubCompute{}, PM: fakePM{py: "3.12", dbc: "17.2.0"},
}
res, err := p.Run(t.Context())
var pe *PipelineError
require.ErrorAs(t, err, &pe)
assert.Equal(t, ErrUsage, pe.Code)
assert.Equal(t, PhasePreflight, pe.FailurePhase)
assert.False(t, pe.DiskMutated)
require.NotNil(t, res.Error)
assert.Equal(t, ErrUsage, res.Error.Code)
}

func TestPipelineCheckMutatesNothing(t *testing.T) {
dir := writeProject(t)
before, _ := os.ReadFile(filepath.Join(dir, "pyproject.toml"))
Expand Down
24 changes: 22 additions & 2 deletions libs/localenv/result.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package localenv

import "fmt"
import (
"encoding/json"
"fmt"
)

// Command path components, defined once so a rename touches a single place
// (spec §0 / invariant 8 / scenario 21). The verb is a subcommand of the
Expand Down Expand Up @@ -64,6 +67,7 @@ const (
type ErrorCode string

const (
ErrUsage ErrorCode = "E_USAGE"
ErrNoTarget ErrorCode = "E_NO_TARGET"
ErrManagerUnsupported ErrorCode = "E_MANAGER_UNSUPPORTED"
ErrUvMissing ErrorCode = "E_UV_MISSING"
Expand All @@ -81,7 +85,8 @@ const (
// PipelineError is a failure carrying a stable code, the phase at which it
// occurred, and whether disk was mutated before the failure. It marshals to the
// --json error object (spec §6.2). Code and FailurePhase are the stable
// contract; Err holds the wrapped cause for errors.Is/As and is not serialized.
// contract; Err holds the wrapped cause for errors.Is/As and is not serialized
// directly (its text is folded into the "message" field — see MarshalJSON).
type PipelineError struct {
Code ErrorCode `json:"code"`
FailurePhase PhaseName `json:"failurePhase"`
Expand All @@ -101,6 +106,21 @@ func (e *PipelineError) Unwrap() error {
return e.Err
}

// MarshalJSON serializes the full message (Error(), i.e. Msg plus any wrapped
// cause) into the "message" field. Without this the --json error object would
// carry only Msg and drop the cause (Err is json:"-"), so a JSON consumer would
// get strictly less detail than the text output — e.g. "resolving cluster name"
// without the "ambiguous"/"not found" reason. Text and JSON must agree.
func (e *PipelineError) MarshalJSON() ([]byte, error) {
type alias PipelineError // avoid recursing into this method
return json.Marshal((*alias)(&PipelineError{
Code: e.Code,
FailurePhase: e.FailurePhase,
Msg: e.Error(),
DiskMutated: e.DiskMutated,
}))
}

// NewError creates a PipelineError with a code and message. FailurePhase and
// DiskMutated are filled in by the pipeline when it records the failure. The
// message is formatted with fmt.Sprintf(format, args...); err may be nil.
Expand Down
Loading