From f9ffddd0e213f1577829a1bd8f4107afc00b7d5f Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Thu, 3 Sep 2026 17:22:31 +0200 Subject: [PATCH 1/6] Add orthogonal setup-local flags: --no-constraints and --no-dbconnect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *Why* `databricks environments setup-local` had a single `--constraints-only` mode that bundled "skip databricks-connect" with the rest of the setup. Callers need to control the setup axes independently — skip the remote pins, or skip the databricks-connect dependency — so the flags that control them should be orthogonal. *What* Adds two orthogonal, composable negative flags: - `--no-constraints` skips writing the remote Python-version and dependency pins (requires-python and the [tool.uv] constraint block); any existing values are left untouched, and provisioning still installs the resolved Python (the flag governs only what is written). - `--no-dbconnect` skips the databricks-connect dependency. Equivalent to the existing `--constraints-only`, which stays as-is for now. The `--no-constraints` "unmanaged" signal is a nil ConstraintDeps / empty requires-python; parseConstraints normalizes a missing [tool.uv].constraint-dependencies to a non-nil empty slice so that nil uniquely means the flag, not merely an artifact that omits the section. Default runs (no new flags) are byte-for-byte unchanged. *Verification* - Unit tests (libs/localenv): --no-constraints leaves existing pins untouched and omits them greenfield, and parseConstraints normalizes missing constraint-dependencies. - Acceptance goldens: no-constraints and no-dbconnect, plus the refreshed help output. - gofmt, go vet, and full go build ./... clean. Co-authored-by: Isaac --- .../cli/setup-local-orthogonal-flags.md | 1 + acceptance/localenv/help/output.txt | 2 + .../localenv/no-constraints/out.test.toml | 2 + acceptance/localenv/no-constraints/output.txt | 60 +++++++++++++++++++ acceptance/localenv/no-constraints/script | 15 +++++ acceptance/localenv/no-constraints/test.toml | 23 +++++++ .../localenv/no-dbconnect/out.test.toml | 2 + acceptance/localenv/no-dbconnect/output.txt | 59 ++++++++++++++++++ acceptance/localenv/no-dbconnect/script | 15 +++++ acceptance/localenv/no-dbconnect/test.toml | 23 +++++++ cmd/environments/output.go | 2 +- cmd/environments/sync.go | 12 +++- libs/localenv/constraints.go | 8 +++ libs/localenv/constraints_test.go | 17 ++++++ libs/localenv/merge.go | 34 +++++++++-- libs/localenv/merge_test.go | 50 ++++++++++++++++ libs/localenv/pipeline.go | 30 +++++++++- libs/localenv/pipeline_test.go | 58 ++++++++++++++++++ 18 files changed, 405 insertions(+), 8 deletions(-) create mode 100644 .nextchanges/cli/setup-local-orthogonal-flags.md create mode 100644 acceptance/localenv/no-constraints/out.test.toml create mode 100644 acceptance/localenv/no-constraints/output.txt create mode 100644 acceptance/localenv/no-constraints/script create mode 100644 acceptance/localenv/no-constraints/test.toml create mode 100644 acceptance/localenv/no-dbconnect/out.test.toml create mode 100644 acceptance/localenv/no-dbconnect/output.txt create mode 100644 acceptance/localenv/no-dbconnect/script create mode 100644 acceptance/localenv/no-dbconnect/test.toml diff --git a/.nextchanges/cli/setup-local-orthogonal-flags.md b/.nextchanges/cli/setup-local-orthogonal-flags.md new file mode 100644 index 00000000000..eaf5174ba3b --- /dev/null +++ b/.nextchanges/cli/setup-local-orthogonal-flags.md @@ -0,0 +1 @@ +Added orthogonal `--no-constraints` and `--no-dbconnect` flags to `databricks environments setup-local`. The flags compose: `--no-constraints` skips writing the remote Python-version and dependency pins, and `--no-dbconnect` skips the databricks-connect dependency. diff --git a/acceptance/localenv/help/output.txt b/acceptance/localenv/help/output.txt index 4ff2f8ac15b..bf8a2d2cf63 100644 --- a/acceptance/localenv/help/output.txt +++ b/acceptance/localenv/help/output.txt @@ -22,6 +22,8 @@ Flags: --dry-run compute the plan without writing files or provisioning -h, --help help for setup-local --job-task string job task to use as the compute target, as . (the task key is required) + --no-constraints skip writing the remote Python version and dependency constraints + --no-dbconnect skip adding the databricks-connect dependency --serverless-version string serverless version to use as the compute target (e.g. 5) Global Flags: diff --git a/acceptance/localenv/no-constraints/out.test.toml b/acceptance/localenv/no-constraints/out.test.toml new file mode 100644 index 00000000000..0938e678987 --- /dev/null +++ b/acceptance/localenv/no-constraints/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/localenv/no-constraints/output.txt b/acceptance/localenv/no-constraints/output.txt new file mode 100644 index 00000000000..6010c343f5a --- /dev/null +++ b/acceptance/localenv/no-constraints/output.txt @@ -0,0 +1,60 @@ + +>>> [CLI] environments setup-local --serverless-version 4 --no-constraints --dry-run --output json +{ + "schemaVersion": 1, + "command": "environments setup-local", + "ok": true, + "mode": "default", + "dryRun": true, + "compute": { + "source": "serverless", + "serverlessVersion": "v4", + "envKey": "serverless/serverless-v4" + }, + "resolved": { + "pythonVersion": "3.12", + "dbconnectVersion": "17.2.0", + "artifactSource": "network" + }, + "greenfield": false, + "plan": { + "wouldWrite": "[TEST_TMP_DIR]/pyproject.toml", + "wouldBackup": "[TEST_TMP_DIR]/pyproject.toml.bak", + "wouldInstallPython": "3.12", + "diff": "--- pyproject.toml\n+++ pyproject.toml.new\n@@ -3,4 +3,7 @@\n requires-python = \"\u003e=3.10\"\n \n [dependency-groups]\n-dev = [\"databricks-connect~=16.0\"]\n+dev = [\"databricks-connect~=17.2.0\"]\n+\n+[tool.databricks.environment]\n+environment_version = \"4\"\n" + }, + "phases": [ + { + "phase": "preflight", + "status": "ok" + }, + { + "phase": "resolve", + "status": "ok" + }, + { + "phase": "fetch", + "status": "ok" + }, + { + "phase": "merge", + "status": "ok" + }, + { + "phase": "provision", + "status": "ok" + }, + { + "phase": "validate", + "status": "ok" + } + ], + "warnings": [ + { + "code": "W_DBCONNECT_PIN_OVERRIDDEN", + "message": "databricks-connect \"databricks-connect~=16.0\" is replaced by the environment's \"databricks-connect~=17.2.0\"" + } + ], + "error": null, + "durationMs": [DURATION_MS] +} diff --git a/acceptance/localenv/no-constraints/script b/acceptance/localenv/no-constraints/script new file mode 100644 index 00000000000..f4711b38dd5 --- /dev/null +++ b/acceptance/localenv/no-constraints/script @@ -0,0 +1,15 @@ +# --no-constraints leaves the remote Python version and dependency pins +# unmanaged: the plan writes neither requires-python nor the [tool.uv] constraint +# block, and the user's existing requires-python is left untouched. The +# databricks-connect dependency (orthogonal to --no-constraints) is still managed. +# The JSON plan shows the diff. +cat > pyproject.toml <<'PY' +[project] +name = "demo" +requires-python = ">=3.10" + +[dependency-groups] +dev = ["databricks-connect~=16.0"] +PY + +trace $CLI environments setup-local --serverless-version 4 --no-constraints --dry-run --output json diff --git a/acceptance/localenv/no-constraints/test.toml b/acceptance/localenv/no-constraints/test.toml new file mode 100644 index 00000000000..6377a54fca8 --- /dev/null +++ b/acceptance/localenv/no-constraints/test.toml @@ -0,0 +1,23 @@ +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] + +# The script writes pyproject.toml as the merge input; --dry-run leaves it unchanged. +Ignore = ["pyproject.toml"] + +Env.DATABRICKS_LOCALENV_CONSTRAINT_SOURCE_URL_TEST_OVERRIDE = "$DATABRICKS_HOST" + +[[Server]] +Pattern = "GET /serverless/serverless-v4/pyproject.toml" +Response.Body = ''' +[project] +requires-python = ">=3.12" + +[dependency-groups] +dev = ["databricks-connect~=17.2.0"] + +[tool.uv] +constraint-dependencies = ["pyarrow<19"] +''' + +[[Repls]] +Old = 'uv uv \S+(?: \([^)]+\))?' +New = 'uv [UV_VERSION]' diff --git a/acceptance/localenv/no-dbconnect/out.test.toml b/acceptance/localenv/no-dbconnect/out.test.toml new file mode 100644 index 00000000000..0938e678987 --- /dev/null +++ b/acceptance/localenv/no-dbconnect/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/localenv/no-dbconnect/output.txt b/acceptance/localenv/no-dbconnect/output.txt new file mode 100644 index 00000000000..677a6fbf26b --- /dev/null +++ b/acceptance/localenv/no-dbconnect/output.txt @@ -0,0 +1,59 @@ + +>>> [CLI] environments setup-local --serverless-version 4 --no-dbconnect --dry-run --output json +{ + "schemaVersion": 1, + "command": "environments setup-local", + "ok": true, + "mode": "constraints-only", + "dryRun": true, + "compute": { + "source": "serverless", + "serverlessVersion": "v4", + "envKey": "serverless/serverless-v4" + }, + "resolved": { + "pythonVersion": "3.12", + "artifactSource": "network" + }, + "greenfield": false, + "plan": { + "wouldWrite": "[TEST_TMP_DIR]/pyproject.toml", + "wouldBackup": "[TEST_TMP_DIR]/pyproject.toml.bak", + "wouldInstallPython": "3.12", + "diff": "--- pyproject.toml\n+++ pyproject.toml.new\n@@ -1,7 +1,17 @@\n [project]\n name = \"demo\"\n-requires-python = \"\u003e=3.10\"\n+requires-python = \"\u003e=3.12\"\n dependencies = [\"databricks-connect==15.1.*\"]\n \n [dependency-groups]\n dev = [\"databricks-connect~=16.0\"]\n+\n+[tool.databricks.environment]\n+environment_version = \"4\"\n+\n+# managed by databricks environments setup-local — do not edit\n+[tool.uv]\n+constraint-dependencies = [\n+ \"pyarrow\u003c19\",\n+]\n+# end managed by databricks environments setup-local\n" + }, + "phases": [ + { + "phase": "preflight", + "status": "ok" + }, + { + "phase": "resolve", + "status": "ok" + }, + { + "phase": "fetch", + "status": "ok" + }, + { + "phase": "merge", + "status": "ok" + }, + { + "phase": "provision", + "status": "ok" + }, + { + "phase": "validate", + "status": "ok" + } + ], + "warnings": [ + { + "code": "W_REQUIRES_PYTHON_OVERRIDDEN", + "message": "requires-python \"\u003e=3.10\" is replaced by the environment's \"\u003e=3.12\"" + } + ], + "error": null, + "durationMs": [DURATION_MS] +} diff --git a/acceptance/localenv/no-dbconnect/script b/acceptance/localenv/no-dbconnect/script new file mode 100644 index 00000000000..63b38e08faa --- /dev/null +++ b/acceptance/localenv/no-dbconnect/script @@ -0,0 +1,15 @@ +# --no-dbconnect is the orthogonal spelling of --constraints-only: it omits the +# databricks-connect dependency (mode "constraints-only") while still managing +# requires-python and the [tool.uv] constraints. Existing databricks-connect +# requirements the user already had are left untouched. +cat > pyproject.toml <<'PY' +[project] +name = "demo" +requires-python = ">=3.10" +dependencies = ["databricks-connect==15.1.*"] + +[dependency-groups] +dev = ["databricks-connect~=16.0"] +PY + +trace $CLI environments setup-local --serverless-version 4 --no-dbconnect --dry-run --output json diff --git a/acceptance/localenv/no-dbconnect/test.toml b/acceptance/localenv/no-dbconnect/test.toml new file mode 100644 index 00000000000..6377a54fca8 --- /dev/null +++ b/acceptance/localenv/no-dbconnect/test.toml @@ -0,0 +1,23 @@ +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] + +# The script writes pyproject.toml as the merge input; --dry-run leaves it unchanged. +Ignore = ["pyproject.toml"] + +Env.DATABRICKS_LOCALENV_CONSTRAINT_SOURCE_URL_TEST_OVERRIDE = "$DATABRICKS_HOST" + +[[Server]] +Pattern = "GET /serverless/serverless-v4/pyproject.toml" +Response.Body = ''' +[project] +requires-python = ">=3.12" + +[dependency-groups] +dev = ["databricks-connect~=17.2.0"] + +[tool.uv] +constraint-dependencies = ["pyarrow<19"] +''' + +[[Repls]] +Old = 'uv uv \S+(?: \([^)]+\))?' +New = 'uv [UV_VERSION]' diff --git a/cmd/environments/output.go b/cmd/environments/output.go index 490fec152ee..d7307a18eb0 100644 --- a/cmd/environments/output.go +++ b/cmd/environments/output.go @@ -87,7 +87,7 @@ func renderResult(ctx context.Context, cmd *cobra.Command, res *libslocalenv.Res return nil } -// renderSuccess prints the friendly post-provision summary (DECO-27977). +// renderSuccess prints the friendly post-provision summary. // // It runs only on a non-dry-run success (renderResult returns earlier for JSON, // failures, and dry runs), so res.VenvPath is always set: the validate phase — the diff --git a/cmd/environments/sync.go b/cmd/environments/sync.go index 5505e6834cf..63365ac34ee 100644 --- a/cmd/environments/sync.go +++ b/cmd/environments/sync.go @@ -60,6 +60,11 @@ func addComputeFlags(cmd *cobra.Command) { cmd.Flags().String("serverless-version", "", "serverless version to use as the compute target (e.g. 5)") cmd.Flags().String("job-task", "", "job task to use as the compute target, as . (the task key is required)") cmd.Flags().Bool("constraints-only", false, "apply the Python version and constraints without adding the databricks-connect dependency") + // The negative flags (--no-constraints, --no-dbconnect) are orthogonal and + // compose. --no-dbconnect and the older --constraints-only are equivalent (both + // skip the databricks-connect dependency). + cmd.Flags().Bool("no-constraints", false, "skip writing the remote Python version and dependency constraints") + cmd.Flags().Bool("no-dbconnect", false, "skip adding the databricks-connect dependency") cmd.Flags().Bool("dry-run", false, "compute the plan without writing files or provisioning") // The mutual exclusivity of the target flags is enforced in the pipeline's // preflight (as E_USAGE) rather than via cmd.MarkFlagsMutuallyExclusive, so @@ -122,6 +127,8 @@ func runPipeline(cmd *cobra.Command) error { serverless, _ := cmd.Flags().GetString("serverless-version") jobTask, _ := cmd.Flags().GetString("job-task") constraintsOnly, _ := cmd.Flags().GetBool("constraints-only") + noConstraints, _ := cmd.Flags().GetBool("no-constraints") + noDBConnect, _ := cmd.Flags().GetBool("no-dbconnect") check, _ := cmd.Flags().GetBool("dry-run") computeFlags := libslocalenv.ComputeFlags{ @@ -134,8 +141,10 @@ func runPipeline(cmd *cobra.Command) error { // preflight, so a conflict is reported as E_USAGE through the phase/JSON // contract rather than as a bare error here. + // --no-dbconnect is the orthogonal spelling of --constraints-only; either skips + // the databricks-connect dependency, which the pipeline models as the mode. mode := libslocalenv.ModeDefault - if constraintsOnly { + if constraintsOnly || noDBConnect { mode = libslocalenv.ModeConstraintsOnly } @@ -177,6 +186,7 @@ func runPipeline(cmd *cobra.Command) error { p := &libslocalenv.Pipeline{ Mode: mode, Check: check, + SkipConstraints: noConstraints, ProjectDir: projectDir, ConstraintBaseURL: constraintBaseURL, CacheDir: cacheDir, diff --git a/libs/localenv/constraints.go b/libs/localenv/constraints.go index 36357facddb..5c3b9e64b0c 100644 --- a/libs/localenv/constraints.go +++ b/libs/localenv/constraints.go @@ -284,7 +284,15 @@ func parseConstraints(data []byte) (requiresPython, dbconnect string, deps []str } } + // Normalize a missing [tool.uv].constraint-dependencies to a non-nil empty + // slice. A nil ConstraintDeps is reserved as the --no-constraints "leave the + // constraint block unmanaged" signal (mergeToolUv and RenderFreshPyproject skip + // on nil); without this, an artifact that simply omits the key would be + // indistinguishable from the flag and would silently stop being managed. deps = p.Tool.UV.ConstraintDependencies + if deps == nil { + deps = []string{} + } return requiresPython, dbconnect, deps, nil } diff --git a/libs/localenv/constraints_test.go b/libs/localenv/constraints_test.go index 2590ce0c643..6eef3e639d6 100644 --- a/libs/localenv/constraints_test.go +++ b/libs/localenv/constraints_test.go @@ -259,3 +259,20 @@ func TestFetchConstraintsUnusableBodyDoesNotPoisonCache(t *testing.T) { assert.True(t, c.FromCache) assert.Equal(t, "==3.12.*", c.RequiresPython) } + +func TestParseConstraintsNormalizesMissingConstraintDepsToEmpty(t *testing.T) { + // An artifact without [tool.uv].constraint-dependencies yields a non-nil empty + // slice, not nil, so a nil ConstraintDeps is reserved as the --no-constraints + // "leave the constraint block unmanaged" signal (mergeToolUv / RenderFreshPyproject + // treat nil as skip). Without this, a normal artifact that simply omits the key + // would be indistinguishable from the flag. + _, _, deps, err := parseConstraints([]byte(`[project] +requires-python = ">=3.12" + +[dependency-groups] +dev = ["databricks-connect~=17.2.0"] +`)) + require.NoError(t, err) + require.NotNil(t, deps) + assert.Empty(t, deps) +} diff --git a/libs/localenv/merge.go b/libs/localenv/merge.go index e89c05ccef1..78b8080b288 100644 --- a/libs/localenv/merge.go +++ b/libs/localenv/merge.go @@ -217,6 +217,15 @@ func tableBounds(lines []string, name string) (header, end int, found bool) { // the line's leading whitespace. If the key is absent, it is inserted directly under the // [project] header. Returns whether the line slice changed. func mergeRequiresPython(lines []string, value string) ([]string, bool) { + // An empty value means requires-python is unmanaged (--no-constraints): leave + // the user's line untouched rather than overwrite it with a blank pin. This + // mirrors how an empty DatabricksConnect / EnvironmentVersion is a no-op; a + // real fetched artifact always carries a requires-python, so empty only ever + // reaches here when the caller deliberately cleared it. + if value == "" { + return lines, false + } + header, end, found := tableBounds(lines, "[project]") if !found { return lines, false @@ -861,6 +870,15 @@ func arrayLineSpan(lines []string, start, limit int) (last int, multiline bool) // marker-bracketed block already exists, its contents are replaced in place. Otherwise any // plain [tool.uv] table is removed and a fresh marker-bracketed block is appended at EOF. func mergeToolUv(lines, deps []string) ([]string, bool) { + // A nil deps slice means the [tool.uv] constraint region is unmanaged + // (--no-constraints): leave any existing block untouched and write none. + // Distinct from a non-nil empty slice, which still renders an empty managed + // block; a real fetched artifact always carries constraint-dependencies, so + // nil only reaches here when the caller deliberately cleared it. + if deps == nil { + return lines, false + } + start, stop, found := markerBounds(lines) if found { // Replace the existing managed region in place. Whether it owns a [tool.uv] @@ -1232,7 +1250,11 @@ func RenderFreshPyproject(projectName string, c Constraints) []byte { fmt.Fprintf(&b, "name = %q\n", projectName) // uv requires project.version when a [project] table is present. fmt.Fprintf(&b, "version = %q\n", freshProjectVersion) - fmt.Fprintf(&b, "requires-python = %q\n", c.RequiresPython) + // requires-python is omitted when unmanaged (--no-constraints); an empty pin + // would be invalid, and a fresh project without it lets uv pick the interpreter. + if c.RequiresPython != "" { + fmt.Fprintf(&b, "requires-python = %q\n", c.RequiresPython) + } b.WriteString("\n") b.WriteString("[dependency-groups]\n") if c.DatabricksConnect != "" { @@ -1250,9 +1272,13 @@ func RenderFreshPyproject(projectName string, c Constraints) []byte { fmt.Fprintf(&b, "environment_version = %q\n", c.EnvironmentVersion) b.WriteString("\n") } - for _, line := range renderToolUvBlock(c.ConstraintDeps, true) { - b.WriteString(line) - b.WriteString("\n") + // The [tool.uv] constraint block is omitted when unmanaged (--no-constraints, + // signalled by a nil slice); a non-nil empty slice still renders an empty block. + if c.ConstraintDeps != nil { + for _, line := range renderToolUvBlock(c.ConstraintDeps, true) { + b.WriteString(line) + b.WriteString("\n") + } } return []byte(b.String()) } diff --git a/libs/localenv/merge_test.go b/libs/localenv/merge_test.go index 048b401d617..67f67a69778 100644 --- a/libs/localenv/merge_test.go +++ b/libs/localenv/merge_test.go @@ -1155,3 +1155,53 @@ constraint-dependencies = ["old~=1.0"] func countOccurrences(s, substr string) int { return strings.Count(s, substr) } + +func TestMergeManagedSkipsRequiresPythonWhenEmpty(t *testing.T) { + // An empty RequiresPython is the --no-constraints signal: the merge must leave + // the user's requires-python untouched rather than overwrite it with "". + in := []byte(`[project] +name = "demo" +requires-python = ">=3.9" + +[dependency-groups] +dev = [] +`) + c := testConstraints() + c.RequiresPython = "" + out, regions, err := MergeManaged(in, c) + require.NoError(t, err) + assert.Contains(t, string(out), `requires-python = ">=3.9"`) + assert.NotContains(t, regions, regionRequiresPython) +} + +func TestMergeManagedSkipsToolUvWhenNil(t *testing.T) { + // A nil ConstraintDeps is the --no-constraints signal: no managed [tool.uv] + // constraint block is written and the region is not reported. + in := []byte(`[project] +name = "demo" +requires-python = "==3.12.*" + +[dependency-groups] +dev = [] +`) + c := testConstraints() + c.ConstraintDeps = nil + out, regions, err := MergeManaged(in, c) + require.NoError(t, err) + assert.NotContains(t, string(out), "constraint-dependencies") + assert.NotContains(t, regions, regionToolUv) +} + +func TestRenderFreshPyprojectOmitsConstraintsWhenEmpty(t *testing.T) { + // Greenfield --no-constraints: neither the Python pin nor the [tool.uv] + // constraint block is rendered, but databricks-connect (orthogonal) still is. + c := testConstraints() + c.RequiresPython = "" + c.ConstraintDeps = nil + out := RenderFreshPyproject("demo", c) + s := string(out) + assert.NotContains(t, s, "requires-python") + assert.NotContains(t, s, "constraint-dependencies") + assert.Contains(t, s, `"databricks-connect~=17.2.0",`) + requireValidTOML(t, out) +} diff --git a/libs/localenv/pipeline.go b/libs/localenv/pipeline.go index 192041767b9..5506c1c5232 100644 --- a/libs/localenv/pipeline.go +++ b/libs/localenv/pipeline.go @@ -59,6 +59,12 @@ type Pipeline struct { Compute ComputeClient PM PackageManager + // SkipConstraints (--no-constraints) leaves the remote Python version and + // dependency pins unmanaged: the merge writes neither requires-python nor the + // [tool.uv] constraint block, and any existing values are left untouched. It is + // orthogonal to Mode (the databricks-connect axis). + SkipConstraints bool + // Progress, when non-nil, receives a PhaseStarted call as each phase begins. // Left nil by callers that don't render progress (e.g. --output json). Progress Reporter @@ -384,13 +390,33 @@ func (p *Pipeline) mergePlan(_ context.Context, pyMinor string, c *Constraints, effective := *c effective.DatabricksConnect = dbcPin effective.EnvironmentVersion = envVersion + if p.SkipConstraints { + // --no-constraints: leave the remote Python version and dependency pins + // unmanaged. The empty/nil values signal both the merge (mergeRequiresPython, + // mergeToolUv) and the fresh render to skip those regions, leaving any + // existing values untouched. + // + // The flag governs only what is *written*. A provisioning run still installs + // and validates the resolved Python (pyMinor), so if the user's kept + // requires-python is disjoint from the target, uv surfaces it as a normal + // E_PROVISION rather than this command guessing an alternative. + effective.RequiresPython = "" + effective.ConstraintDeps = nil + } var changedRegions []string if greenfield { // No existing pyproject.toml — render a fresh one. The project name comes - // from the directory name as a reasonable default. + // from the directory name as a reasonable default. Only the regions actually + // rendered are reported (requires-python and tool.uv are omitted under + // --no-constraints). merged = RenderFreshPyproject(projectName(p.ProjectDir), effective) - changedRegions = []string{regionRequiresPython, regionToolUv} + if effective.RequiresPython != "" { + changedRegions = append(changedRegions, regionRequiresPython) + } + if effective.ConstraintDeps != nil { + changedRegions = append(changedRegions, regionToolUv) + } if dbcPin != "" { changedRegions = append(changedRegions, regionDatabricksConnect) } diff --git a/libs/localenv/pipeline_test.go b/libs/localenv/pipeline_test.go index 5cd97723b68..6e75c503a05 100644 --- a/libs/localenv/pipeline_test.go +++ b/libs/localenv/pipeline_test.go @@ -1265,3 +1265,61 @@ func TestPipelineReportsPhaseStarts(t *testing.T) { // A full successful run enters every phase exactly once in canonical order. assert.Equal(t, allPhases, rep.started) } + +func TestPipelineNoConstraintsLeavesExistingPinsUntouched(t *testing.T) { + dir := t.TempDir() + // An existing project with the user's own requires-python and no managed + // [tool.uv] constraint block. + require.NoError(t, os.WriteFile(filepath.Join(dir, "pyproject.toml"), []byte(`[project] +name = "demo" +requires-python = ">=3.9" + +[dependency-groups] +dev = ["databricks-connect~=16.0.0"] +`), 0o644)) + srv := newTestServer(t) + defer srv.Close() + + p := &Pipeline{ + Mode: ModeDefault, SkipConstraints: true, ProjectDir: dir, + ConstraintBaseURL: srv.URL, CacheDir: t.TempDir(), + Flags: ComputeFlags{Serverless: "v4"}, + Compute: stubCompute{}, PM: fakePM{py: "3.12", dbc: "17.2.0"}, + } + res, err := p.Run(t.Context()) + require.NoError(t, err) + assert.True(t, res.OK) + data, _ := os.ReadFile(filepath.Join(dir, "pyproject.toml")) + s := string(data) + // requires-python keeps the user's value; the artifact's ==3.12.* is not written. + assert.Contains(t, s, `requires-python = ">=3.9"`) + assert.NotContains(t, s, "==3.12.*") + // No managed [tool.uv] constraint-dependencies block is written. + assert.NotContains(t, s, "constraint-dependencies") + // databricks-connect is still managed: --no-constraints is orthogonal to it. + assert.Contains(t, s, "databricks-connect~=17.2.0") +} + +func TestPipelineNoConstraintsGreenfieldOmitsPins(t *testing.T) { + dir := t.TempDir() + srv := newTestServer(t) + defer srv.Close() + + p := &Pipeline{ + Mode: ModeDefault, SkipConstraints: true, ProjectDir: dir, + ConstraintBaseURL: srv.URL, CacheDir: t.TempDir(), + Flags: ComputeFlags{Serverless: "v4"}, + Compute: stubCompute{}, PM: fakePM{py: "3.12", dbc: "17.2.0"}, + } + res, err := p.Run(t.Context()) + require.NoError(t, err) + assert.True(t, res.OK) + assert.True(t, res.Greenfield) + data, _ := os.ReadFile(filepath.Join(dir, "pyproject.toml")) + s := string(data) + // The artifact's Python pin and constraint-dependencies are not written. + assert.NotContains(t, s, "==3.12.*") + assert.NotContains(t, s, "constraint-dependencies") + // databricks-connect (orthogonal) is still added. + assert.Contains(t, s, "databricks-connect~=17.2.0") +} From 158815d250ef78227ecf8e6d85833b796caeb334 Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Fri, 4 Sep 2026 09:32:26 +0200 Subject: [PATCH 2/6] Thread --no-constraints explicitly instead of a nil/empty signal value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *Why* Review feedback: the --no-constraints "leave the constraint block unmanaged" intent was encoded by clearing effective.RequiresPython to "" and effective.ConstraintDeps to nil, then having the merge, fresh render, and warning detector infer "skip" from those shapes. That overloaded the values (nil ConstraintDeps meant the flag, an empty slice meant "write an empty block"), and only a parseConstraints normalization step kept the two apart. Overloading a data value to carry a control signal is fragile — anyone constructing a Constraints by hand, or refactoring the parse path, silently re-collides the two and stops managing constraints with no test to catch it. *What* Thread an explicit skipConstraints bool from the pipeline into MergeManaged, RenderFreshPyproject, and detectMergeWarnings; gate the requires-python and [tool.uv] regions (and their two warnings) on it. The pipeline no longer clears effective's values, so those functions always see the real artifact values and the intent is unambiguous. Consequences of removing the sentinel: - parseConstraints no longer normalizes a missing constraint-dependencies to a non-nil empty slice; nil and empty now flow through identically. - mergeToolUv drops its nil-guard so nil is treated like empty (both render an empty managed block); mergeRequiresPython drops its empty-value guard. Behaviour is unchanged: default and --no-constraints output are byte-identical (every acceptance golden passes untouched). This is an internal cleanup that deletes the sentinel and its normalization. *Verification* - go build ./... and go vet ./libs/localenv/... ./cmd/environments/... clean. - go test ./libs/localenv/... ./cmd/environments/... pass. - go test ./acceptance -run TestAccept/localenv passes with goldens unchanged. - New unit tests: MergeManaged/RenderFreshPyproject skip the regions on skipConstraints=true even when the Constraints carry values; a nil ConstraintDeps writes an empty managed block when not skipping; detectMergeWarnings suppresses the constraint warnings under skip. Verified each fails without the gate before it was added. Co-authored-by: Isaac --- libs/localenv/constraints.go | 15 +-- libs/localenv/constraints_test.go | 12 +- libs/localenv/merge.go | 64 +++++----- libs/localenv/merge_test.go | 193 ++++++++++++++++-------------- libs/localenv/pipeline.go | 36 +++--- libs/localenv/warnings.go | 18 ++- libs/localenv/warnings_test.go | 31 ++++- 7 files changed, 202 insertions(+), 167 deletions(-) diff --git a/libs/localenv/constraints.go b/libs/localenv/constraints.go index 5c3b9e64b0c..7c26798412d 100644 --- a/libs/localenv/constraints.go +++ b/libs/localenv/constraints.go @@ -284,16 +284,11 @@ func parseConstraints(data []byte) (requiresPython, dbconnect string, deps []str } } - // Normalize a missing [tool.uv].constraint-dependencies to a non-nil empty - // slice. A nil ConstraintDeps is reserved as the --no-constraints "leave the - // constraint block unmanaged" signal (mergeToolUv and RenderFreshPyproject skip - // on nil); without this, an artifact that simply omits the key would be - // indistinguishable from the flag and would silently stop being managed. - deps = p.Tool.UV.ConstraintDependencies - if deps == nil { - deps = []string{} - } - return requiresPython, dbconnect, deps, nil + // A missing [tool.uv].constraint-dependencies yields a nil slice, which the + // merge treats identically to an empty one (an empty managed block). Whether + // the constraint region is managed at all is decided by the --no-constraints + // flag, threaded explicitly, not by the shape of this value. + return requiresPython, dbconnect, p.Tool.UV.ConstraintDependencies, nil } // depNameSepRe matches the first PEP 508 delimiter that ends a requirement's diff --git a/libs/localenv/constraints_test.go b/libs/localenv/constraints_test.go index 6eef3e639d6..65395317dd2 100644 --- a/libs/localenv/constraints_test.go +++ b/libs/localenv/constraints_test.go @@ -260,12 +260,11 @@ func TestFetchConstraintsUnusableBodyDoesNotPoisonCache(t *testing.T) { assert.Equal(t, "==3.12.*", c.RequiresPython) } -func TestParseConstraintsNormalizesMissingConstraintDepsToEmpty(t *testing.T) { - // An artifact without [tool.uv].constraint-dependencies yields a non-nil empty - // slice, not nil, so a nil ConstraintDeps is reserved as the --no-constraints - // "leave the constraint block unmanaged" signal (mergeToolUv / RenderFreshPyproject - // treat nil as skip). Without this, a normal artifact that simply omits the key - // would be indistinguishable from the flag. +func TestParseConstraintsMissingConstraintDepsIsValid(t *testing.T) { + // An artifact without [tool.uv].constraint-dependencies is valid; the parsed + // slice is empty. Its nil-vs-empty shape carries no meaning — whether the + // constraint region is managed is decided by the --no-constraints flag, threaded + // explicitly, so the merge treats nil and empty identically. _, _, deps, err := parseConstraints([]byte(`[project] requires-python = ">=3.12" @@ -273,6 +272,5 @@ requires-python = ">=3.12" dev = ["databricks-connect~=17.2.0"] `)) require.NoError(t, err) - require.NotNil(t, deps) assert.Empty(t, deps) } diff --git a/libs/localenv/merge.go b/libs/localenv/merge.go index 78b8080b288..6a225935c8c 100644 --- a/libs/localenv/merge.go +++ b/libs/localenv/merge.go @@ -103,7 +103,13 @@ func planDBConnect(target []byte, c Constraints) dbconnectPlan { // byte (comments, ordering, whitespace). It returns the merged bytes and the list of // regions that actually changed. The operation is idempotent: feeding its own output // back in produces identical bytes. -func MergeManaged(target []byte, c Constraints) (merged []byte, regions []string, err error) { +// skipConstraints (--no-constraints) leaves the requires-python and [tool.uv] +// constraint regions unmanaged: any existing values are preserved and none are +// written. It is threaded explicitly rather than inferred from empty/nil values in +// c, so the caller's intent is unambiguous and c always carries the real artifact +// values. The databricks-connect and environment regions are orthogonal and are +// always reconciled. +func MergeManaged(target []byte, c Constraints, skipConstraints bool) (merged []byte, regions []string, err error) { s := string(target) // Detect and normalize line endings. We process on "\n" and restore "\r\n" on @@ -131,9 +137,12 @@ func MergeManaged(target []byte, c Constraints) (merged []byte, regions []string return nil, nil, errNoProjectTable } - lines, rpChanged := mergeRequiresPython(lines, c.RequiresPython) - if rpChanged { - regions = append(regions, regionRequiresPython) + if !skipConstraints { + var rpChanged bool + lines, rpChanged = mergeRequiresPython(lines, c.RequiresPython) + if rpChanged { + regions = append(regions, regionRequiresPython) + } } lines, _, dbcChanged := mergeDatabricksConnect(lines, c.DatabricksConnect) @@ -154,9 +163,12 @@ func MergeManaged(target []byte, c Constraints) (merged []byte, regions []string regions = append(regions, regionDatabricksEnvironment) } - lines, uvChanged := mergeToolUv(lines, c.ConstraintDeps) - if uvChanged { - regions = append(regions, regionToolUv) + if !skipConstraints { + var uvChanged bool + lines, uvChanged = mergeToolUv(lines, c.ConstraintDeps) + if uvChanged { + regions = append(regions, regionToolUv) + } } out := restore(strings.Join(lines, "\n")) @@ -217,15 +229,8 @@ func tableBounds(lines []string, name string) (header, end int, found bool) { // the line's leading whitespace. If the key is absent, it is inserted directly under the // [project] header. Returns whether the line slice changed. func mergeRequiresPython(lines []string, value string) ([]string, bool) { - // An empty value means requires-python is unmanaged (--no-constraints): leave - // the user's line untouched rather than overwrite it with a blank pin. This - // mirrors how an empty DatabricksConnect / EnvironmentVersion is a no-op; a - // real fetched artifact always carries a requires-python, so empty only ever - // reaches here when the caller deliberately cleared it. - if value == "" { - return lines, false - } - + // Only reached when constraints are managed (MergeManaged gates on + // skipConstraints), where a fetched artifact always carries a requires-python. header, end, found := tableBounds(lines, "[project]") if !found { return lines, false @@ -870,15 +875,9 @@ func arrayLineSpan(lines []string, start, limit int) (last int, multiline bool) // marker-bracketed block already exists, its contents are replaced in place. Otherwise any // plain [tool.uv] table is removed and a fresh marker-bracketed block is appended at EOF. func mergeToolUv(lines, deps []string) ([]string, bool) { - // A nil deps slice means the [tool.uv] constraint region is unmanaged - // (--no-constraints): leave any existing block untouched and write none. - // Distinct from a non-nil empty slice, which still renders an empty managed - // block; a real fetched artifact always carries constraint-dependencies, so - // nil only reaches here when the caller deliberately cleared it. - if deps == nil { - return lines, false - } - + // Only reached when constraints are managed (MergeManaged gates on + // skipConstraints). A nil deps slice is treated identically to an empty one: + // both render an empty managed block. start, stop, found := markerBounds(lines) if found { // Replace the existing managed region in place. Whether it owns a [tool.uv] @@ -1244,15 +1243,15 @@ const freshProjectVersion = "0.0.0" // [tool.databricks.environment] section (serverless targets only), and the marker-bracketed // [tool.uv] constraint block. When c.DatabricksConnect is empty (constraints-only mode) the // dev group is emitted empty rather than with a blank entry. -func RenderFreshPyproject(projectName string, c Constraints) []byte { +func RenderFreshPyproject(projectName string, c Constraints, skipConstraints bool) []byte { var b strings.Builder b.WriteString("[project]\n") fmt.Fprintf(&b, "name = %q\n", projectName) // uv requires project.version when a [project] table is present. fmt.Fprintf(&b, "version = %q\n", freshProjectVersion) - // requires-python is omitted when unmanaged (--no-constraints); an empty pin - // would be invalid, and a fresh project without it lets uv pick the interpreter. - if c.RequiresPython != "" { + // requires-python is omitted when constraints are skipped (--no-constraints), + // which lets uv pick the interpreter for the fresh project. + if !skipConstraints { fmt.Fprintf(&b, "requires-python = %q\n", c.RequiresPython) } b.WriteString("\n") @@ -1272,9 +1271,10 @@ func RenderFreshPyproject(projectName string, c Constraints) []byte { fmt.Fprintf(&b, "environment_version = %q\n", c.EnvironmentVersion) b.WriteString("\n") } - // The [tool.uv] constraint block is omitted when unmanaged (--no-constraints, - // signalled by a nil slice); a non-nil empty slice still renders an empty block. - if c.ConstraintDeps != nil { + // The [tool.uv] constraint block is omitted when constraints are skipped + // (--no-constraints); otherwise it is always written (an empty block when the + // artifact carries no constraint-dependencies). + if !skipConstraints { for _, line := range renderToolUvBlock(c.ConstraintDeps, true) { b.WriteString(line) b.WriteString("\n") diff --git a/libs/localenv/merge_test.go b/libs/localenv/merge_test.go index 67f67a69778..f6581380dbc 100644 --- a/libs/localenv/merge_test.go +++ b/libs/localenv/merge_test.go @@ -39,7 +39,7 @@ dev = [ "pytest~=8.0", ] `) - out, regions, err := MergeManaged(in, testConstraints()) + out, regions, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) assert.Contains(t, string(out), `requires-python = "==3.12.*"`) assert.Contains(t, string(out), "# keep this comment") @@ -60,9 +60,9 @@ dev = [ "databricks-connect~=16.0.0", ] `) - once, _, err := MergeManaged(in, testConstraints()) + once, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) - twice, _, err := MergeManaged(once, testConstraints()) + twice, _, err := MergeManaged(once, testConstraints(), false) require.NoError(t, err) assert.Equal(t, string(once), string(twice)) } @@ -74,7 +74,7 @@ name = "demo" [dependency-groups] dev = ["databricks-connect~=16.0.0"] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) assert.Contains(t, string(out), `requires-python = "==3.12.*"`) } @@ -93,7 +93,7 @@ constraint-dependencies = [ ] ` + managedMarkerEnd + ` `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) assert.NotContains(t, string(out), "stale~=1.0.0") assert.Contains(t, string(out), "pydantic~=2.10.6") @@ -103,12 +103,12 @@ constraint-dependencies = [ func TestMergePreservesCRLF(t *testing.T) { in := []byte("[project]\r\nrequires-python = \">=3.10\"\r\n\r\n[dependency-groups]\r\ndev = [\"databricks-connect~=16.0.0\"]\r\n") - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) assert.Contains(t, string(out), "\r\n") assert.Contains(t, string(out), `requires-python = "==3.12.*"`) // Merging the CRLF output again must be byte-identical (idempotent under \r\n). - twice, _, err := MergeManaged(out, testConstraints()) + twice, _, err := MergeManaged(out, testConstraints(), false) require.NoError(t, err) assert.Equal(t, string(out), string(twice)) } @@ -124,7 +124,7 @@ dev = ["databricks-connect~=16.0.0"] package = true dev-dependencies = ["ruff"] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) assert.Contains(t, s, "[tool.uv]") @@ -142,7 +142,7 @@ dev-dependencies = ["ruff"] requireValidTOML(t, out) assert.Equal(t, 1, countOccurrences(s, "[tool.uv]")) // Merge-twice is byte-identical (header-less managed region stays header-less). - twice, _, err := MergeManaged(out, testConstraints()) + twice, _, err := MergeManaged(out, testConstraints(), false) require.NoError(t, err) assert.Equal(t, s, string(twice)) requireValidTOML(t, twice) @@ -159,7 +159,7 @@ dev = ["databricks-connect~=16.0.0"] package = true constraint-dependencies = ["old~=1.0"] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) assert.Contains(t, s, "package = true") @@ -170,7 +170,7 @@ constraint-dependencies = ["old~=1.0"] requireValidTOML(t, out) assert.Equal(t, 1, countOccurrences(s, "[tool.uv]")) // Merge-twice is byte-identical. - twice, _, err := MergeManaged(out, testConstraints()) + twice, _, err := MergeManaged(out, testConstraints(), false) require.NoError(t, err) assert.Equal(t, string(out), string(twice)) } @@ -185,7 +185,7 @@ dev = ["databricks-connect~=16.0.0"] [tool.uv] constraint-dependencies = ["old~=1.0"] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) assert.NotContains(t, s, "old~=1.0") @@ -208,7 +208,7 @@ constraint-dependencies = [ "old~=1.0", ] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) assert.NotContains(t, s, "old~=1.0") @@ -219,7 +219,7 @@ constraint-dependencies = [ assert.Equal(t, 1, countOccurrences(s, managedMarkerStart)) requireValidTOML(t, out) // Merge-twice is byte-identical. - twice, _, err := MergeManaged(out, testConstraints()) + twice, _, err := MergeManaged(out, testConstraints(), false) require.NoError(t, err) assert.Equal(t, string(out), string(twice)) } @@ -231,7 +231,7 @@ requires-python = ">=3.10" [dependency-groups] dev = ["databricks-connect~=16.0.0", "pytest~=8.0"] `) - out, regions, err := MergeManaged(in, testConstraints()) + out, regions, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) // Sibling element and single-line array layout are preserved. assert.Contains(t, string(out), `dev = ["databricks-connect~=17.2.0", "pytest~=8.0"]`) @@ -247,7 +247,7 @@ dev = [ "databricks-connect~=16.0.0", ] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) // The trailing comma on the managed element is preserved. assert.Contains(t, string(out), ` "databricks-connect~=17.2.0",`) @@ -264,14 +264,14 @@ dev = [ "pytest~=8.0", ] `) - out, regions, err := MergeManaged(in, testConstraints()) + out, regions, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) assert.Contains(t, s, `"databricks-connect~=17.2.0",`) assert.Contains(t, s, `"pytest~=8.0",`, "existing element preserved") assert.Contains(t, regions, "databricks-connect") // Idempotent: a second merge finds the element and rewrites in place. - out2, _, err := MergeManaged(out, testConstraints()) + out2, _, err := MergeManaged(out, testConstraints(), false) require.NoError(t, err) assert.Equal(t, s, string(out2)) } @@ -283,10 +283,10 @@ requires-python = ">=3.10" [dependency-groups] dev = ["pytest~=8.0"] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) assert.Contains(t, string(out), `dev = ["databricks-connect~=17.2.0", "pytest~=8.0"]`) - out2, _, err := MergeManaged(out, testConstraints()) + out2, _, err := MergeManaged(out, testConstraints(), false) require.NoError(t, err) assert.Equal(t, string(out), string(out2)) } @@ -298,7 +298,7 @@ requires-python = ">=3.10" [dependency-groups] dev = [] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) assert.Contains(t, string(out), `dev = ["databricks-connect~=17.2.0"]`) } @@ -310,12 +310,12 @@ requires-python = ">=3.10" [dependency-groups] test = ["pytest~=8.0"] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) assert.Contains(t, s, `"databricks-connect~=17.2.0",`) assert.Contains(t, s, `test = ["pytest~=8.0"]`, "sibling group untouched") - out2, _, err := MergeManaged(out, testConstraints()) + out2, _, err := MergeManaged(out, testConstraints(), false) require.NoError(t, err) assert.Equal(t, s, string(out2)) } @@ -324,12 +324,12 @@ func TestMergeInsertsDependencyGroupsWhenAbsent(t *testing.T) { in := []byte(`[project] requires-python = ">=3.10" `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) assert.Contains(t, s, "[dependency-groups]") assert.Contains(t, s, `"databricks-connect~=17.2.0",`) - out2, _, err := MergeManaged(out, testConstraints()) + out2, _, err := MergeManaged(out, testConstraints(), false) require.NoError(t, err) assert.Equal(t, s, string(out2)) } @@ -345,7 +345,7 @@ dev = ["databricks-connect~=16.0.0", "pytest~=8.0", ] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) assert.Equal(t, 1, strings.Count(s, "databricks-connect"), "must not duplicate the pin") @@ -365,7 +365,7 @@ dev = [ "pytest~=8.0", ] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) assert.Equal(t, 1, strings.Count(s, "databricks-connect"), "must not duplicate the pin") @@ -384,7 +384,7 @@ dev = [ "pytest~=8.0", ] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) // Code element is rewritten; the comment mention is left verbatim. @@ -397,7 +397,7 @@ func TestMergeRewritesNonCanonicalDatabricksConnectSpelling(t *testing.T) { // place, not left undetected so the insert path adds a conflicting second pin. for _, spelling := range []string{"databricks_connect", "Databricks-Connect", "databricks.connect"} { in := []byte("[project]\nrequires-python = \">=3.10\"\n\n[dependency-groups]\ndev = [\n \"" + spelling + "~=16.0.0\",\n]\n") - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err, spelling) s := string(out) assert.Equal(t, 1, strings.Count(s, `"databricks-connect~=17.2.0"`), "spelling %q must be rewritten in place, not duplicated:\n%s", spelling, s) @@ -416,13 +416,13 @@ dev = [ "pytest~=8.0" ] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) assert.Contains(t, s, `"pytest~=8.0",`, "previous last element gains a separating comma") assert.Contains(t, s, `"databricks-connect~=17.2.0",`) // Round-trips as valid TOML. - out2, _, err := MergeManaged(out, testConstraints()) + out2, _, err := MergeManaged(out, testConstraints(), false) require.NoError(t, err) assert.Equal(t, s, string(out2)) } @@ -437,14 +437,14 @@ dev = ["pytest~=8.0"] `) c := testConstraints() c.DatabricksConnect = "" - out, regions, err := MergeManaged(in, c) + out, regions, err := MergeManaged(in, c, false) require.NoError(t, err) assert.NotContains(t, string(out), "databricks-connect") assert.NotContains(t, regions, "databricks-connect") } func TestRenderFreshPyproject(t *testing.T) { - out := RenderFreshPyproject("demo", testConstraints()) + out := RenderFreshPyproject("demo", testConstraints(), false) s := string(out) assert.Contains(t, s, `name = "demo"`) assert.Contains(t, s, `requires-python = "==3.12.*"`) @@ -455,7 +455,7 @@ func TestRenderFreshPyproject(t *testing.T) { // A cluster target (no EnvironmentVersion) writes no [tool.databricks.environment]. assert.NotContains(t, s, "[tool.databricks.environment]") // A fresh render is itself a no-op under MergeManaged (already fully managed). - merged, _, err := MergeManaged(out, testConstraints()) + merged, _, err := MergeManaged(out, testConstraints(), false) require.NoError(t, err) assert.Equal(t, s, string(merged)) } @@ -463,13 +463,13 @@ func TestRenderFreshPyproject(t *testing.T) { func TestRenderFreshPyprojectServerlessWritesEnvironment(t *testing.T) { c := testConstraints() c.EnvironmentVersion = "5" - out := RenderFreshPyproject("demo", c) + out := RenderFreshPyproject("demo", c, false) s := string(out) assert.Contains(t, s, "[tool.databricks.environment]") assert.Contains(t, s, `environment_version = "5"`) requireValidTOML(t, out) // A fresh render is itself a no-op under MergeManaged (already fully managed). - merged, _, err := MergeManaged(out, c) + merged, _, err := MergeManaged(out, c, false) require.NoError(t, err) assert.Equal(t, s, string(merged)) } @@ -483,7 +483,7 @@ dev = ["databricks-connect~=16.0.0"] `) c := testConstraints() c.EnvironmentVersion = "5" - out, regions, err := MergeManaged(in, c) + out, regions, err := MergeManaged(in, c, false) require.NoError(t, err) s := string(out) assert.Contains(t, s, "[tool.databricks.environment]") @@ -491,7 +491,7 @@ dev = ["databricks-connect~=16.0.0"] assert.Contains(t, regions, regionDatabricksEnvironment) requireValidTOML(t, out) // Idempotent. - twice, _, err := MergeManaged(out, c) + twice, _, err := MergeManaged(out, c, false) require.NoError(t, err) assert.Equal(t, s, string(twice)) } @@ -508,7 +508,7 @@ environment_version = "4" # pinned `) c := testConstraints() c.EnvironmentVersion = "5" - out, regions, err := MergeManaged(in, c) + out, regions, err := MergeManaged(in, c, false) require.NoError(t, err) s := string(out) assert.Contains(t, s, `environment_version = "5" # pinned`) @@ -531,7 +531,7 @@ dev = ["databricks-connect~=16.0.0"] `) c := testConstraints() c.EnvironmentVersion = "5" - out, _, err := MergeManaged(in, c) + out, _, err := MergeManaged(in, c, false) require.NoError(t, err) s := string(out) assert.Contains(t, s, `environment_version = "5"`) @@ -553,7 +553,7 @@ dev = ["databricks-connect~=16.0.0"] environment_version = "4" `) c := testConstraints() // EnvironmentVersion == "": cluster target. - out, regions, err := MergeManaged(in, c) + out, regions, err := MergeManaged(in, c, false) require.NoError(t, err) assert.Contains(t, string(out), `environment_version = "4"`) assert.NotContains(t, regions, regionDatabricksEnvironment) @@ -584,7 +584,7 @@ constraint-dependencies = [ `) c := testConstraints() c.EnvironmentVersion = "5" - out, regions, err := MergeManaged(in, c) + out, regions, err := MergeManaged(in, c, false) require.NoError(t, err) s := string(out) assert.Contains(t, s, "[tool.databricks.environment]") @@ -595,7 +595,7 @@ constraint-dependencies = [ assert.Equal(t, 1, countOccurrences(s, "[tool.databricks.environment]")) requireValidTOML(t, out) // Idempotent on the upgraded file. - twice, _, err := MergeManaged(out, c) + twice, _, err := MergeManaged(out, c, false) require.NoError(t, err) assert.Equal(t, s, string(twice)) } @@ -617,7 +617,7 @@ constraint-dependencies = ["requests[security]~=2.0", "old-dep~=1.0", ] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) // The whole stale array is gone (both the bracket-bearing first element and @@ -629,7 +629,7 @@ constraint-dependencies = ["requests[security]~=2.0", requireValidTOML(t, out) assert.Equal(t, 1, countOccurrences(s, "[tool.uv]")) // Idempotent. - twice, _, err := MergeManaged(out, testConstraints()) + twice, _, err := MergeManaged(out, testConstraints(), false) require.NoError(t, err) assert.Equal(t, s, string(twice)) } @@ -663,7 +663,7 @@ dev = [ "databricks-connect~=16.0.0", ] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) // docs consolidated (its db-connect removed); dev updated to the managed pin. @@ -690,7 +690,7 @@ dev = [ "databricks-connect~=16.0", ] `) - out, regions, err := MergeManaged(in, testConstraints()) + out, regions, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) assert.NotContains(t, s, "databricks-connect==15.1.*", "the stray project.dependencies pin is removed") @@ -701,7 +701,7 @@ dev = [ requireValidTOML(t, out) // Idempotent: a second merge finds nothing to remove and produces identical bytes. - out2, _, err := MergeManaged(out, testConstraints()) + out2, _, err := MergeManaged(out, testConstraints(), false) require.NoError(t, err) assert.Equal(t, string(out), string(out2)) } @@ -718,7 +718,7 @@ spark = ["databricks-connect==15.0.0"] [dependency-groups] dev = ["databricks-connect~=16.0"] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) assert.Contains(t, s, "spark = []", "the optional-dependency extra is emptied") @@ -736,7 +736,7 @@ dependencies = ["numpy", "databricks-connect==15.1.*", "pytest"] [dependency-groups] dev = ["databricks-connect~=16.0"] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) assert.Contains(t, s, `dependencies = ["numpy", "pytest"]`) @@ -764,7 +764,7 @@ extra = ["databricks-connect"] dev = ["databricks-connect~=16.0"] test = ["databricks-connect>=15,<20"] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) assert.Contains(t, s, `"databricks-connect>=15",`, "an overlapping pin co-resolves and is kept") @@ -783,7 +783,7 @@ func TestMergeKeepsEnvEqualPinInProjectDeps(t *testing.T) { requires-python = ">=3.10" dependencies = ["databricks-connect~=17.2.0"] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) assert.Contains(t, string(out), `dependencies = ["databricks-connect~=17.2.0"]`, "the env-equal pin is kept") requireValidTOML(t, out) @@ -803,7 +803,7 @@ dependencies = [ [dependency-groups] dev = ["databricks-connect~=16.0"] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) assert.NotContains(t, s, "databricks-connect==15.1.*") @@ -824,7 +824,7 @@ dev = ["databricks-connect~=16.0"] docs = ["databricks-connect~=14.3"] `) c := Constraints{RequiresPython: "==3.12.*", ConstraintDeps: []string{"pydantic~=2.10.6"}} - out, regions, err := MergeManaged(in, c) + out, regions, err := MergeManaged(in, c, false) require.NoError(t, err) s := string(out) assert.Contains(t, s, "databricks-connect==15.1.*", "project.dependencies pin left untouched") @@ -868,7 +868,7 @@ dev = ["databricks-connect~=16.0"] "comment on the opening bracket line": "# runtime deps", } for name, in := range cases { - out, _, err := MergeManaged([]byte(in), testConstraints()) + out, _, err := MergeManaged([]byte(in), testConstraints(), false) require.NoError(t, err, name) s := string(out) assert.NotContains(t, s, "databricks-connect==15.1.*", name) @@ -892,7 +892,7 @@ dev = [ "databricks-connect==15.0.0", ] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) assert.Contains(t, s, `"databricks-connect~=17.2.0",`, "the managed pin is kept and updated") @@ -911,7 +911,7 @@ requires-python = ">=3.10" dev = [{include-group = "spark"}] spark = ['databricks-connect==15.0.0'] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) assert.Contains(t, s, `{include-group = "spark"}`, "the include-group reference is preserved") @@ -928,7 +928,7 @@ requires-python = ">=3.10" [dependency-groups] dev = ["pytest"] # keep "databricks-connect~=14.3" for docs `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) // The comment's databricks-connect~=14.3 is preserved verbatim. @@ -943,7 +943,7 @@ requires-python = ">=3.10" # maintained by platform team [dependency-groups] dev = ["databricks-connect~=16.0.0"] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) assert.Contains(t, s, `requires-python = "==3.12.*" # maintained by platform team`) @@ -963,7 +963,7 @@ dev = ["databricks-connect~=16.0.0"] [tool.custom] # user table dev = ["databricks-connect==1.0.0"] # must not be managed `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) // [project].requires-python was found and updated despite the header comment. @@ -1002,7 +1002,7 @@ dev = ["databricks-connect~=16.0.0"] name = "internal" url = "https://packages.example/simple" `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) s := string(out) requireValidTOML(t, out) @@ -1011,7 +1011,7 @@ url = "https://packages.example/simple" assert.Contains(t, s, `name = "internal"`) assert.Contains(t, s, "pydantic~=2.10.6") // Merge-twice is byte-identical. - twice, _, err := MergeManaged(out, testConstraints()) + twice, _, err := MergeManaged(out, testConstraints(), false) require.NoError(t, err) assert.Equal(t, s, string(twice)) } @@ -1030,13 +1030,13 @@ constraint-dependencies = [] [dependency-groups] dev = ["databricks-connect~=16.0.0"] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) requireValidTOML(t, out) assert.Contains(t, string(out), "description = \"\"\"\nA multi-line project description.\n\"\"\"") assert.Contains(t, string(out), `requires-python = "==3.12.*"`) - twice, _, err := MergeManaged(out, testConstraints()) + twice, _, err := MergeManaged(out, testConstraints(), false) require.NoError(t, err) assert.Equal(t, out, twice) } @@ -1060,7 +1060,7 @@ dev = ["databricks-connect~=16.0.0"] [tool.uv] constraint-dependencies = [] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) requireValidTOML(t, out) s := string(out) @@ -1080,7 +1080,7 @@ description = """same-line value""" [dependency-groups] dev = ["databricks-connect~=16.0.0"] `) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) requireValidTOML(t, out) s := string(out) @@ -1092,7 +1092,7 @@ dev = ["databricks-connect~=16.0.0"] func TestMergePreservesCRLFInsideMultilineString(t *testing.T) { in := []byte("[project]\r\nrequires-python = \">=3.10\"\r\ndescription = \"\"\"\r\ncontinued\r\n\"\"\"\r\n\r\n[dependency-groups]\r\ndev = [\"databricks-connect~=16.0.0\"]\r\n") - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) requireValidTOML(t, out) assert.NotContains(t, strings.ReplaceAll(string(out), "\r\n", ""), "\n") @@ -1105,7 +1105,7 @@ requires-python = ">=3.10" description = """ unterminated `) - _, _, err := MergeManaged(in, testConstraints()) + _, _, err := MergeManaged(in, testConstraints(), false) require.ErrorIs(t, err, errMultilineString) } @@ -1126,12 +1126,12 @@ func TestMergePreservesMultilineStringDelimiterEdgeCases(t *testing.T) { in := []byte("[project]\nrequires-python = \">=3.10\"\ndescription = " + tt.value + "\n\n[dependency-groups]\ndev = [\"databricks-connect~=16.0.0\"]\n") requireValidTOML(t, in) - out, _, err := MergeManaged(in, testConstraints()) + out, _, err := MergeManaged(in, testConstraints(), false) require.NoError(t, err) requireValidTOML(t, out) assert.Contains(t, string(out), "description = "+tt.value) - twice, _, err := MergeManaged(out, testConstraints()) + twice, _, err := MergeManaged(out, testConstraints(), false) require.NoError(t, err) assert.Equal(t, out, twice) }) @@ -1147,7 +1147,7 @@ dev = ["databricks-connect~=16.0.0"] [tool.uv] constraint-dependencies = ["old~=1.0"] `) - _, _, err := MergeManaged(in, testConstraints()) + _, _, err := MergeManaged(in, testConstraints(), false) require.Error(t, err) assert.ErrorIs(t, err, errNoProjectTable) } @@ -1156,9 +1156,9 @@ func countOccurrences(s, substr string) int { return strings.Count(s, substr) } -func TestMergeManagedSkipsRequiresPythonWhenEmpty(t *testing.T) { - // An empty RequiresPython is the --no-constraints signal: the merge must leave - // the user's requires-python untouched rather than overwrite it with "". +func TestMergeManagedSkipConstraintsLeavesRequiresPythonUntouched(t *testing.T) { + // skipConstraints=true leaves the user's requires-python untouched even though + // the Constraints carry a real pin: the flag, not an empty value, drives the skip. in := []byte(`[project] name = "demo" requires-python = ">=3.9" @@ -1166,17 +1166,16 @@ requires-python = ">=3.9" [dependency-groups] dev = [] `) - c := testConstraints() - c.RequiresPython = "" - out, regions, err := MergeManaged(in, c) + c := testConstraints() // carries requires-python = "==3.12.*" + out, regions, err := MergeManaged(in, c, true) require.NoError(t, err) assert.Contains(t, string(out), `requires-python = ">=3.9"`) assert.NotContains(t, regions, regionRequiresPython) } -func TestMergeManagedSkipsToolUvWhenNil(t *testing.T) { - // A nil ConstraintDeps is the --no-constraints signal: no managed [tool.uv] - // constraint block is written and the region is not reported. +func TestMergeManagedSkipConstraintsLeavesToolUvUntouched(t *testing.T) { + // skipConstraints=true writes no managed [tool.uv] constraint block even though + // the Constraints carry constraint-dependencies. in := []byte(`[project] name = "demo" requires-python = "==3.12.*" @@ -1184,21 +1183,39 @@ requires-python = "==3.12.*" [dependency-groups] dev = [] `) - c := testConstraints() - c.ConstraintDeps = nil - out, regions, err := MergeManaged(in, c) + c := testConstraints() // carries constraint-dependencies + out, regions, err := MergeManaged(in, c, true) require.NoError(t, err) assert.NotContains(t, string(out), "constraint-dependencies") assert.NotContains(t, regions, regionToolUv) } -func TestRenderFreshPyprojectOmitsConstraintsWhenEmpty(t *testing.T) { - // Greenfield --no-constraints: neither the Python pin nor the [tool.uv] - // constraint block is rendered, but databricks-connect (orthogonal) still is. +func TestMergeManagedNilConstraintDepsWritesEmptyManagedBlock(t *testing.T) { + // With constraints managed, a nil ConstraintDeps (an artifact that simply omits + // constraint-dependencies) is treated identically to an empty slice: an empty + // managed [tool.uv] block is written. nil no longer signals "skip". + in := []byte(`[project] +name = "demo" +requires-python = "==3.12.*" + +[dependency-groups] +dev = [] +`) c := testConstraints() - c.RequiresPython = "" c.ConstraintDeps = nil - out := RenderFreshPyproject("demo", c) + out, regions, err := MergeManaged(in, c, false) + require.NoError(t, err) + assert.Contains(t, regions, regionToolUv) + assert.Contains(t, string(out), "constraint-dependencies = [") + requireValidTOML(t, out) +} + +func TestRenderFreshPyprojectSkipConstraintsOmitsConstraints(t *testing.T) { + // Greenfield with skipConstraints=true: neither the Python pin nor the [tool.uv] + // constraint block is rendered even though the Constraints carry them, but + // databricks-connect (orthogonal) still is. + c := testConstraints() // carries requires-python and constraint-dependencies + out := RenderFreshPyproject("demo", c, true) s := string(out) assert.NotContains(t, s, "requires-python") assert.NotContains(t, s, "constraint-dependencies") diff --git a/libs/localenv/pipeline.go b/libs/localenv/pipeline.go index 5506c1c5232..07fa1b0a88a 100644 --- a/libs/localenv/pipeline.go +++ b/libs/localenv/pipeline.go @@ -390,19 +390,16 @@ func (p *Pipeline) mergePlan(_ context.Context, pyMinor string, c *Constraints, effective := *c effective.DatabricksConnect = dbcPin effective.EnvironmentVersion = envVersion - if p.SkipConstraints { - // --no-constraints: leave the remote Python version and dependency pins - // unmanaged. The empty/nil values signal both the merge (mergeRequiresPython, - // mergeToolUv) and the fresh render to skip those regions, leaving any - // existing values untouched. - // - // The flag governs only what is *written*. A provisioning run still installs - // and validates the resolved Python (pyMinor), so if the user's kept - // requires-python is disjoint from the target, uv surfaces it as a normal - // E_PROVISION rather than this command guessing an alternative. - effective.RequiresPython = "" - effective.ConstraintDeps = nil - } + // --no-constraints (p.SkipConstraints) leaves the remote Python version and + // dependency pins unmanaged. It is threaded explicitly into the merge, fresh + // render, and warning detector below rather than encoded by clearing + // effective's values, so those functions see the real artifact values and the + // intent is unambiguous. + // + // The flag governs only what is *written*. A provisioning run still installs + // and validates the resolved Python (pyMinor), so if the user's kept + // requires-python is disjoint from the target, uv surfaces it as a normal + // E_PROVISION rather than this command guessing an alternative. var changedRegions []string if greenfield { @@ -410,12 +407,9 @@ func (p *Pipeline) mergePlan(_ context.Context, pyMinor string, c *Constraints, // from the directory name as a reasonable default. Only the regions actually // rendered are reported (requires-python and tool.uv are omitted under // --no-constraints). - merged = RenderFreshPyproject(projectName(p.ProjectDir), effective) - if effective.RequiresPython != "" { - changedRegions = append(changedRegions, regionRequiresPython) - } - if effective.ConstraintDeps != nil { - changedRegions = append(changedRegions, regionToolUv) + merged = RenderFreshPyproject(projectName(p.ProjectDir), effective, p.SkipConstraints) + if !p.SkipConstraints { + changedRegions = append(changedRegions, regionRequiresPython, regionToolUv) } if dbcPin != "" { changedRegions = append(changedRegions, regionDatabricksConnect) @@ -424,7 +418,7 @@ func (p *Pipeline) mergePlan(_ context.Context, pyMinor string, c *Constraints, changedRegions = append(changedRegions, regionDatabricksEnvironment) } } else { - merged, changedRegions, err = MergeManaged(baseBytes, effective) + merged, changedRegions, err = MergeManaged(baseBytes, effective, p.SkipConstraints) if err != nil { return nil, greenfield, p.fail(PhaseMerge, false, NewError(ErrMerge, err, "merge managed regions failed")) } @@ -435,7 +429,7 @@ func (p *Pipeline) mergePlan(_ context.Context, pyMinor string, c *Constraints, // edits come from the merge itself (planDBConnect), so a warning can never claim a // rewrite or removal that did not happen. p.res.Warnings = append(p.res.Warnings, - detectMergeWarnings(baseBytes, effective, planDBConnect(baseBytes, effective))...) + detectMergeWarnings(baseBytes, effective, planDBConnect(baseBytes, effective), p.SkipConstraints)...) } // Under --dry-run, build the plan (with a diff) for reporting. A real run does diff --git a/libs/localenv/warnings.go b/libs/localenv/warnings.go index 1767efb3242..e0bb2a1b469 100644 --- a/libs/localenv/warnings.go +++ b/libs/localenv/warnings.go @@ -173,7 +173,12 @@ func resolutionRequirements(p userPyprojectTOML) []string { // deterministic and ordered (requires-python, then databricks-connect, then the // standalone-pyspark collision, then constraint conflicts in the order uv would // encounter them) so goldens are stable. -func detectMergeWarnings(userPyproject []byte, c Constraints, plan dbconnectPlan) []Warning { +// skipConstraints (--no-constraints) suppresses the requires-python and +// constraint-dependency warnings: when those regions are left unmanaged the merge +// neither overrides the user's requires-python nor imposes constraints, so there is +// nothing to warn about. It is threaded explicitly for the same reason MergeManaged +// takes it — the intent is not inferred from empty/nil values in c. +func detectMergeWarnings(userPyproject []byte, c Constraints, plan dbconnectPlan, skipConstraints bool) []Warning { if len(userPyproject) == 0 { return nil } @@ -188,8 +193,9 @@ func detectMergeWarnings(userPyproject []byte, c Constraints, plan dbconnectPlan var warnings []Warning // The user pinned a requires-python that differs from the env's pin; the merge - // replaces it with the managed value. - if up := strings.TrimSpace(p.Project.RequiresPython); up != "" && c.RequiresPython != "" && up != strings.TrimSpace(c.RequiresPython) { + // replaces it with the managed value. Skipped under --no-constraints, where the + // merge leaves the user's requires-python untouched. + if up := strings.TrimSpace(p.Project.RequiresPython); !skipConstraints && up != "" && c.RequiresPython != "" && up != strings.TrimSpace(c.RequiresPython) { warnings = append(warnings, Warning{ Code: WarnRequiresPythonOverridden, Message: fmt.Sprintf("requires-python %q is replaced by the environment's %q", up, c.RequiresPython), @@ -219,7 +225,11 @@ func detectMergeWarnings(userPyproject []byte, c Constraints, plan dbconnectPlan warnings = append(warnings, standalonePysparkWarnings(survivors)...) } - warnings = append(warnings, constraintConflicts(survivors, c.ConstraintDeps)...) + // Skipped under --no-constraints: no constraint block is written, so the user's + // requirements cannot conflict with a managed one. + if !skipConstraints { + warnings = append(warnings, constraintConflicts(survivors, c.ConstraintDeps)...) + } // A cluster target leaves c.EnvironmentVersion empty and does not manage the // serverless environment section, so an environment_version left over from an diff --git a/libs/localenv/warnings_test.go b/libs/localenv/warnings_test.go index 8609e5a811d..ff5a2c25351 100644 --- a/libs/localenv/warnings_test.go +++ b/libs/localenv/warnings_test.go @@ -12,7 +12,7 @@ import ( // against a different notion of what the merge rewrites or removes than production uses — // that divergence is the bug this wiring exists to prevent. func detectWarnings(userPyproject []byte, c Constraints) []Warning { - return detectMergeWarnings(userPyproject, c, planDBConnect(userPyproject, c)) + return detectMergeWarnings(userPyproject, c, planDBConnect(userPyproject, c), false) } // codes extracts the warning codes in order for concise assertions. @@ -46,6 +46,27 @@ dev = ["databricks-connect~=16.1.0"] assert.Equal(t, []string{WarnRequiresPythonOverridden, WarnDBConnectPinOverridden}, codes(got)) } +func TestDetectMergeWarningsSkipConstraintsSuppressesConstraintWarnings(t *testing.T) { + // Under --no-constraints the merge leaves requires-python and the constraint + // block unmanaged, so neither the requires-python-overridden warning nor a + // constraint conflict is reported — but the orthogonal databricks-connect + // override warning still is. + user := []byte(`[project] +name = "demo" +requires-python = ">=3.10" + +[dependency-groups] +dev = ["databricks-connect~=16.1.0", "pydantic~=1.0"] +`) + c := Constraints{ + RequiresPython: "==3.12.*", + DatabricksConnect: "databricks-connect~=18.0.0", + ConstraintDeps: []string{"pydantic~=2.10.6"}, + } + got := detectMergeWarnings(user, c, planDBConnect(user, c), true) + assert.Equal(t, []string{WarnDBConnectPinOverridden}, codes(got)) +} + func TestDetectMergeWarningsWithMultilineString(t *testing.T) { user := []byte(`[project] requires-python = ">=3.10" @@ -300,7 +321,7 @@ spark = ["databricks-connect==15.0.0"] // Consolidation actually fixes the project: re-running on the merged file finds a // single managed pin and nothing to warn about (the spark stray is gone). - merged, _, err := MergeManaged(both, c) + merged, _, err := MergeManaged(both, c, false) require.NoError(t, err) assert.Empty(t, detectWarnings(merged, c)) @@ -508,7 +529,7 @@ Dev = ["databricks-connect==16.1.0"] assert.Equal(t, []string{WarnDBConnectConsolidated}, codes(got)) assert.NotContains(t, got[0].Message, "is replaced") - merged, _, err := MergeManaged(user, c) + merged, _, err := MergeManaged(user, c, false) require.NoError(t, err) assert.NotContains(t, string(merged), `databricks-connect==16.1.0`, "the stray pin is removed, not retained") } @@ -544,7 +565,7 @@ requires-python = "==3.12.*" assert.NotContains(t, got[0].Message, "is replaced", name) // The merged file really does carry both pins, which is what the code reports. - merged, _, err := MergeManaged([]byte(body), c) + merged, _, err := MergeManaged([]byte(body), c, false) require.NoError(t, err, name) assert.Contains(t, string(merged), "databricks-connect==16.1.0", name) assert.Contains(t, string(merged), "databricks-connect==17.0.0", name) @@ -560,7 +581,7 @@ Dev = ["databricks-connect==16.1.0"] `) got := detectWarnings(capitalized, c) assert.Equal(t, []string{WarnDBConnectConsolidated}, codes(got)) - merged, _, err := MergeManaged(capitalized, c) + merged, _, err := MergeManaged(capitalized, c, false) require.NoError(t, err) assert.NotContains(t, string(merged), "databricks-connect==16.1.0") assert.Contains(t, string(merged), "databricks-connect==17.0.0") From b8f914b0ba971e5afef1db72245579e7043e93c9 Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Fri, 4 Sep 2026 09:44:45 +0200 Subject: [PATCH 3/6] Fix changelog fragment to the required single-bullet format *Why* The rebase onto latest main picked up a stricter `validate_nextchanges.py`, which now requires each fragment to be a single `* `-bullet line ending in a period with a trailing PR link. The existing prose fragment failed that check, breaking the `lint` and `preview` CI jobs. *What* Rewrite `.nextchanges/cli/setup-local-orthogonal-flags.md` as one bullet line with the `([#6464](...))` trailing PR link. No content change to the described behaviour. *Verification* - PR_NUMBER=6464 python3 tools/validate_nextchanges.py exits 0. Co-authored-by: Isaac --- .nextchanges/cli/setup-local-orthogonal-flags.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.nextchanges/cli/setup-local-orthogonal-flags.md b/.nextchanges/cli/setup-local-orthogonal-flags.md index eaf5174ba3b..d445847216c 100644 --- a/.nextchanges/cli/setup-local-orthogonal-flags.md +++ b/.nextchanges/cli/setup-local-orthogonal-flags.md @@ -1 +1 @@ -Added orthogonal `--no-constraints` and `--no-dbconnect` flags to `databricks environments setup-local`. The flags compose: `--no-constraints` skips writing the remote Python-version and dependency pins, and `--no-dbconnect` skips the databricks-connect dependency. +* Add orthogonal `--no-constraints` and `--no-dbconnect` flags to `databricks environments setup-local`: `--no-constraints` skips writing the remote Python-version and dependency pins, and `--no-dbconnect` skips the databricks-connect dependency. ([#6464](https://github.com/databricks/cli/pull/6464)) From 03d88cfb152871c57a9da746026a42ad82036a3d Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Fri, 4 Sep 2026 09:59:51 +0200 Subject: [PATCH 4/6] Record --no-constraints in the setup-local telemetry event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *Why* Review feedback: `--no-constraints` was threaded into the pipeline but never surfaced in `SetupLocalEvent`. Because the flag leaves `Mode` at `default` (Mode only records the databricks-connect axis), a `--no-constraints` run was indistinguishable from a plain run in telemetry, so its adoption could not be measured. *What* Add a `skip_constraints` bool to `SetupLocalEvent` (not omitempty, so a genuine false is distinguishable from an older CLI that did not report it, matching the existing `success`/`greenfield` fields). Carry it on `Result` as an internal-only `json:"-"` field — like `PythonInterpreter` — so the telemetry event stays purely Result-derived without expanding the `--output json` contract. The pipeline sets it from `p.SkipConstraints`. Note: `SetupLocalEvent` mirrors the server-side lumberjack proto; the matching `skip_constraints` field must be added there for the value to be captured. Until then the server ignores the unknown field (no harm), same as any CLI-first telemetry addition. *Verification* - go build ./..., go vet, and gofmt clean. - go test ./cmd/environments/... ./libs/localenv/... pass; new unit test asserts a --no-constraints run records skip_constraints=true and a plain run leaves it false (verified it fails without the mapping). - go test ./acceptance -run TestAccept/localenv passes with goldens unchanged (the field is json:"-", so the JSON contract is untouched). Co-authored-by: Isaac --- cmd/environments/telemetry.go | 9 +++++---- cmd/environments/telemetry_test.go | 21 +++++++++++++++++++++ libs/localenv/pipeline.go | 1 + libs/localenv/result.go | 15 ++++++++++----- libs/telemetry/protos/setup_local.go | 6 ++++++ 5 files changed, 43 insertions(+), 9 deletions(-) diff --git a/cmd/environments/telemetry.go b/cmd/environments/telemetry.go index c8c653a8060..05c8aeb0a6c 100644 --- a/cmd/environments/telemetry.go +++ b/cmd/environments/telemetry.go @@ -29,10 +29,11 @@ func buildSetupLocalEvent(res *libslocalenv.Result) *protos.SetupLocalEvent { return nil } event := &protos.SetupLocalEvent{ - Success: res.OK, - Mode: modeType(res.Mode), - Greenfield: res.Greenfield, - ComputeSource: protos.SetupLocalComputeSourceUnspecified, + Success: res.OK, + Mode: modeType(res.Mode), + SkipConstraints: res.SkipConstraints, + Greenfield: res.Greenfield, + ComputeSource: protos.SetupLocalComputeSourceUnspecified, } if res.Compute != nil { event.ComputeSource = computeSourceType(res.Compute.Source) diff --git a/cmd/environments/telemetry_test.go b/cmd/environments/telemetry_test.go index a458821c0a5..867930f46da 100644 --- a/cmd/environments/telemetry_test.go +++ b/cmd/environments/telemetry_test.go @@ -64,6 +64,27 @@ func TestBuildSetupLocalEvent(t *testing.T) { }, got) }) + t.Run("no-constraints run records skipConstraints", func(t *testing.T) { + res := &libslocalenv.Result{ + OK: true, + Mode: libslocalenv.ModeDefault.String(), + SkipConstraints: true, + Compute: &libslocalenv.ComputeInfo{Source: "cluster", EnvKey: "dbr/16.4"}, + } + got := buildSetupLocalEvent(res) + assert.True(t, got.SkipConstraints) + }) + + t.Run("run without --no-constraints leaves skipConstraints false", func(t *testing.T) { + res := &libslocalenv.Result{ + OK: true, + Mode: libslocalenv.ModeDefault.String(), + Compute: &libslocalenv.ComputeInfo{Source: "cluster", EnvKey: "dbr/16.4"}, + } + got := buildSetupLocalEvent(res) + assert.False(t, got.SkipConstraints) + }) + t.Run("nil compute leaves source and envKey empty", func(t *testing.T) { res := &libslocalenv.Result{ OK: false, diff --git a/libs/localenv/pipeline.go b/libs/localenv/pipeline.go index 07fa1b0a88a..89aa84dc65c 100644 --- a/libs/localenv/pipeline.go +++ b/libs/localenv/pipeline.go @@ -106,6 +106,7 @@ func (p *Pipeline) Run(ctx context.Context) (*Result, error) { p.res.Command = CommandName p.res.Mode = p.Mode.String() p.res.DryRun = p.Check + p.res.SkipConstraints = p.SkipConstraints // Phases start as pending and flip to ok/error as the run progresses. p.res.Phases = initialPhases() diff --git a/libs/localenv/result.go b/libs/localenv/result.go index 2f44f167817..f7189f02743 100644 --- a/libs/localenv/result.go +++ b/libs/localenv/result.go @@ -317,11 +317,16 @@ type Result struct { // fallback, named in the text summary so the user can tell which interpreter // backs the venv. Not serialized: the structured result stays categorical via // PythonResolution (a path would leak machine layout to JSON consumers). - PythonInterpreter string `json:"-"` - Phases []PhaseStatus `json:"phases"` - Warnings []Warning `json:"warnings"` - Error *PipelineError `json:"error"` - BackupPath string `json:"backupPath,omitempty"` + PythonInterpreter string `json:"-"` + // SkipConstraints records whether --no-constraints was set. Not serialized: it + // is not part of the --output json contract, but the telemetry event reads it to + // distinguish a --no-constraints run from a plain one (Mode only records the + // databricks-connect axis). + SkipConstraints bool `json:"-"` + Phases []PhaseStatus `json:"phases"` + Warnings []Warning `json:"warnings"` + Error *PipelineError `json:"error"` + BackupPath string `json:"backupPath,omitempty"` // DurationMs is the pipeline's wall time in milliseconds (spec §6). It covers the // CLI pipeline only; the extension measures its own end-to-end latency (process // spawn, interpreter adoption) separately. diff --git a/libs/telemetry/protos/setup_local.go b/libs/telemetry/protos/setup_local.go index f359abf8664..a608b6182fb 100644 --- a/libs/telemetry/protos/setup_local.go +++ b/libs/telemetry/protos/setup_local.go @@ -72,6 +72,12 @@ type SetupLocalEvent struct { // Provisioning mode: default or constraints-only. Mode SetupLocalMode `json:"mode,omitempty"` + // Whether --no-constraints was set, leaving requires-python and the dependency + // constraints unmanaged. Orthogonal to Mode (which records the databricks-connect + // axis), so without this a --no-constraints run is indistinguishable from a plain + // run. Not omitempty for the same reason as Success. + SkipConstraints bool `json:"skip_constraints"` + // Whether pyproject.toml was created fresh (true) vs an existing project // updated (false). Not omitempty for the same reason as Success. Greenfield bool `json:"greenfield"` From b2d0dabfabe0e5a66ea7a176dcbcdb225a230998 Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Fri, 4 Sep 2026 13:21:41 +0200 Subject: [PATCH 5/6] Replace merge skip-bools with a MergeOptions struct; make --no-dbconnect explicit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *Why* The previous change threaded --no-constraints as a positional bool through MergeManaged/RenderFreshPyproject/detectMergeWarnings. A positional bool per axis does not scale: every new axis re-churns all ~75 call sites and yields boolean-blind calls like MergeManaged(x, c, true, false). The remaining axis — databricks-connect — was still driven by the empty-dbcPin signal value (exactly the "other signal values elsewhere" the reviewer flagged), so it had the same overloading problem --no-constraints just shed. *What* Introduce MergeOptions{SkipConstraints, SkipDBConnect bool}, passed to MergeManaged, RenderFreshPyproject, and detectMergeWarnings. The zero value manages every axis, so a future axis is a new field with no call-site churn, and each call reads self-documentingly. Thread SkipDBConnect explicitly everywhere the empty-dbcPin signal used to reach: the merge gates the databricks-connect region on it, the fresh render emits an empty dev group under it, the warning detector suppresses the databricks-connect / standalone-pyspark warnings, planDBConnect returns the zero plan, and validate skips the databricks-connect assertion and version reporting. The pipeline no longer clears the pin — c carries the real artifact value throughout, and intent is a flag, not an empty string. An empty databricks-connect value now means only "the artifact has no pin" (a data no-op), never "skip". Behaviour is unchanged: default, --no-constraints, and --no-dbconnect / --constraints-only output are byte-identical (every acceptance golden passes untouched). *Verification* - go build ./..., go vet, and gofmt clean. - go test ./libs/localenv/... ./cmd/environments/... pass; new unit tests cover SkipDBConnect in the merge, fresh render, and warning detector (verified each fails without its gate before restoring). - go test ./acceptance -run TestAccept/localenv passes with goldens unchanged. Co-authored-by: Isaac --- libs/localenv/merge.go | 75 ++++++++------ libs/localenv/merge_test.go | 183 +++++++++++++++++++-------------- libs/localenv/pipeline.go | 100 +++++++++--------- libs/localenv/warnings.go | 37 +++---- libs/localenv/warnings_test.go | 31 ++++-- 5 files changed, 247 insertions(+), 179 deletions(-) diff --git a/libs/localenv/merge.go b/libs/localenv/merge.go index 6a225935c8c..e3cdbca3121 100644 --- a/libs/localenv/merge.go +++ b/libs/localenv/merge.go @@ -75,13 +75,14 @@ type dbconnectPlan struct { } // planDBConnect returns the databricks-connect edits merging target would make, or -// the zero plan in constraints-only mode (empty pin) where databricks-connect is left +// the zero plan when databricks-connect is skipped (--no-dbconnect / --constraints-only) +// or the artifact carries no pin — the cases where MergeManaged leaves databricks-connect // untouched. It mirrors MergeManaged's preprocessing (CRLF normalization and // multi-line string protection) and runs both databricks-connect passes on a clone, // in the same order as MergeManaged, so replacedDevPin and removed match what the // real merge does. -func planDBConnect(target []byte, c Constraints) dbconnectPlan { - if c.DatabricksConnect == "" { +func planDBConnect(target []byte, c Constraints, skipDBConnect bool) dbconnectPlan { + if skipDBConnect || c.DatabricksConnect == "" { return dbconnectPlan{} } // Mirror MergeManaged's own preprocessing so the same lines are inspected. @@ -99,17 +100,27 @@ func planDBConnect(target []byte, c Constraints) dbconnectPlan { return dbconnectPlan{replacedDevPin: replaced, removed: removed} } +// MergeOptions selects which orthogonal managed axes are left unmanaged. Each flag +// is threaded explicitly rather than inferred from empty/nil values in the +// Constraints, so a caller's intent is unambiguous and the Constraints always carry +// the real artifact values. The zero value manages every axis. A new axis is a new +// field here — callers that manage everything keep passing MergeOptions{} unchanged. +type MergeOptions struct { + // SkipConstraints (--no-constraints) leaves the requires-python and [tool.uv] + // constraint regions unmanaged: existing values are preserved and none written. + SkipConstraints bool + // SkipDBConnect (--no-dbconnect / --constraints-only) leaves the + // databricks-connect dependency unmanaged: an existing pin is preserved and none + // is injected or asserted. + SkipDBConnect bool +} + // MergeManaged applies the managed transforms to target, preserving every other // byte (comments, ordering, whitespace). It returns the merged bytes and the list of // regions that actually changed. The operation is idempotent: feeding its own output -// back in produces identical bytes. -// skipConstraints (--no-constraints) leaves the requires-python and [tool.uv] -// constraint regions unmanaged: any existing values are preserved and none are -// written. It is threaded explicitly rather than inferred from empty/nil values in -// c, so the caller's intent is unambiguous and c always carries the real artifact -// values. The databricks-connect and environment regions are orthogonal and are -// always reconciled. -func MergeManaged(target []byte, c Constraints, skipConstraints bool) (merged []byte, regions []string, err error) { +// back in produces identical bytes. opts selects which axes are managed; the +// environment region is always reconciled. +func MergeManaged(target []byte, c Constraints, opts MergeOptions) (merged []byte, regions []string, err error) { s := string(target) // Detect and normalize line endings. We process on "\n" and restore "\r\n" on @@ -137,7 +148,7 @@ func MergeManaged(target []byte, c Constraints, skipConstraints bool) (merged [] return nil, nil, errNoProjectTable } - if !skipConstraints { + if !opts.SkipConstraints { var rpChanged bool lines, rpChanged = mergeRequiresPython(lines, c.RequiresPython) if rpChanged { @@ -145,17 +156,21 @@ func MergeManaged(target []byte, c Constraints, skipConstraints bool) (merged [] } } - lines, _, dbcChanged := mergeDatabricksConnect(lines, c.DatabricksConnect) - // In the install flow, after the managed pin lands in the dev group, remove any - // databricks-connect pin elsewhere that is disjoint from it — the pins that would - // otherwise make uv unsatisfiable. Compatible pins are left alone. Skipped in - // constraints-only mode (empty pin), where databricks-connect is left untouched. - strayChanged := false - if c.DatabricksConnect != "" { - lines, _, strayChanged = removeStrayDatabricksConnect(lines, c.DatabricksConnect) - } - if dbcChanged || strayChanged { - regions = append(regions, regionDatabricksConnect) + if !opts.SkipDBConnect { + var dbcChanged bool + lines, _, dbcChanged = mergeDatabricksConnect(lines, c.DatabricksConnect) + // In the install flow, after the managed pin lands in the dev group, remove any + // databricks-connect pin elsewhere that is disjoint from it — the pins that would + // otherwise make uv unsatisfiable. Compatible pins are left alone. An empty pin + // (an artifact without databricks-connect) is a data no-op, distinct from the + // SkipDBConnect intent handled by the gate above. + strayChanged := false + if c.DatabricksConnect != "" { + lines, _, strayChanged = removeStrayDatabricksConnect(lines, c.DatabricksConnect) + } + if dbcChanged || strayChanged { + regions = append(regions, regionDatabricksConnect) + } } lines, envChanged := mergeDatabricksEnvironment(lines, c.EnvironmentVersion) @@ -163,7 +178,7 @@ func MergeManaged(target []byte, c Constraints, skipConstraints bool) (merged [] regions = append(regions, regionDatabricksEnvironment) } - if !skipConstraints { + if !opts.SkipConstraints { var uvChanged bool lines, uvChanged = mergeToolUv(lines, c.ConstraintDeps) if uvChanged { @@ -1241,9 +1256,9 @@ const freshProjectVersion = "0.0.0" // RenderFreshPyproject produces a complete managed pyproject.toml for a project that has // none, with [project], [dependency-groups].dev (carrying the databricks-connect pin), the // [tool.databricks.environment] section (serverless targets only), and the marker-bracketed -// [tool.uv] constraint block. When c.DatabricksConnect is empty (constraints-only mode) the -// dev group is emitted empty rather than with a blank entry. -func RenderFreshPyproject(projectName string, c Constraints, skipConstraints bool) []byte { +// [tool.uv] constraint block. When databricks-connect is skipped (--no-dbconnect / +// --constraints-only) or the artifact carries no pin, the dev group is emitted empty. +func RenderFreshPyproject(projectName string, c Constraints, opts MergeOptions) []byte { var b strings.Builder b.WriteString("[project]\n") fmt.Fprintf(&b, "name = %q\n", projectName) @@ -1251,12 +1266,12 @@ func RenderFreshPyproject(projectName string, c Constraints, skipConstraints boo fmt.Fprintf(&b, "version = %q\n", freshProjectVersion) // requires-python is omitted when constraints are skipped (--no-constraints), // which lets uv pick the interpreter for the fresh project. - if !skipConstraints { + if !opts.SkipConstraints { fmt.Fprintf(&b, "requires-python = %q\n", c.RequiresPython) } b.WriteString("\n") b.WriteString("[dependency-groups]\n") - if c.DatabricksConnect != "" { + if !opts.SkipDBConnect && c.DatabricksConnect != "" { b.WriteString("dev = [\n") fmt.Fprintf(&b, " %q,\n", c.DatabricksConnect) b.WriteString("]\n") @@ -1274,7 +1289,7 @@ func RenderFreshPyproject(projectName string, c Constraints, skipConstraints boo // The [tool.uv] constraint block is omitted when constraints are skipped // (--no-constraints); otherwise it is always written (an empty block when the // artifact carries no constraint-dependencies). - if !skipConstraints { + if !opts.SkipConstraints { for _, line := range renderToolUvBlock(c.ConstraintDeps, true) { b.WriteString(line) b.WriteString("\n") diff --git a/libs/localenv/merge_test.go b/libs/localenv/merge_test.go index f6581380dbc..96ec013a523 100644 --- a/libs/localenv/merge_test.go +++ b/libs/localenv/merge_test.go @@ -39,7 +39,7 @@ dev = [ "pytest~=8.0", ] `) - out, regions, err := MergeManaged(in, testConstraints(), false) + out, regions, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.Contains(t, string(out), `requires-python = "==3.12.*"`) assert.Contains(t, string(out), "# keep this comment") @@ -60,9 +60,9 @@ dev = [ "databricks-connect~=16.0.0", ] `) - once, _, err := MergeManaged(in, testConstraints(), false) + once, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) - twice, _, err := MergeManaged(once, testConstraints(), false) + twice, _, err := MergeManaged(once, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.Equal(t, string(once), string(twice)) } @@ -74,7 +74,7 @@ name = "demo" [dependency-groups] dev = ["databricks-connect~=16.0.0"] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.Contains(t, string(out), `requires-python = "==3.12.*"`) } @@ -93,7 +93,7 @@ constraint-dependencies = [ ] ` + managedMarkerEnd + ` `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.NotContains(t, string(out), "stale~=1.0.0") assert.Contains(t, string(out), "pydantic~=2.10.6") @@ -103,12 +103,12 @@ constraint-dependencies = [ func TestMergePreservesCRLF(t *testing.T) { in := []byte("[project]\r\nrequires-python = \">=3.10\"\r\n\r\n[dependency-groups]\r\ndev = [\"databricks-connect~=16.0.0\"]\r\n") - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.Contains(t, string(out), "\r\n") assert.Contains(t, string(out), `requires-python = "==3.12.*"`) // Merging the CRLF output again must be byte-identical (idempotent under \r\n). - twice, _, err := MergeManaged(out, testConstraints(), false) + twice, _, err := MergeManaged(out, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.Equal(t, string(out), string(twice)) } @@ -124,7 +124,7 @@ dev = ["databricks-connect~=16.0.0"] package = true dev-dependencies = ["ruff"] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) assert.Contains(t, s, "[tool.uv]") @@ -142,7 +142,7 @@ dev-dependencies = ["ruff"] requireValidTOML(t, out) assert.Equal(t, 1, countOccurrences(s, "[tool.uv]")) // Merge-twice is byte-identical (header-less managed region stays header-less). - twice, _, err := MergeManaged(out, testConstraints(), false) + twice, _, err := MergeManaged(out, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.Equal(t, s, string(twice)) requireValidTOML(t, twice) @@ -159,7 +159,7 @@ dev = ["databricks-connect~=16.0.0"] package = true constraint-dependencies = ["old~=1.0"] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) assert.Contains(t, s, "package = true") @@ -170,7 +170,7 @@ constraint-dependencies = ["old~=1.0"] requireValidTOML(t, out) assert.Equal(t, 1, countOccurrences(s, "[tool.uv]")) // Merge-twice is byte-identical. - twice, _, err := MergeManaged(out, testConstraints(), false) + twice, _, err := MergeManaged(out, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.Equal(t, string(out), string(twice)) } @@ -185,7 +185,7 @@ dev = ["databricks-connect~=16.0.0"] [tool.uv] constraint-dependencies = ["old~=1.0"] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) assert.NotContains(t, s, "old~=1.0") @@ -208,7 +208,7 @@ constraint-dependencies = [ "old~=1.0", ] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) assert.NotContains(t, s, "old~=1.0") @@ -219,7 +219,7 @@ constraint-dependencies = [ assert.Equal(t, 1, countOccurrences(s, managedMarkerStart)) requireValidTOML(t, out) // Merge-twice is byte-identical. - twice, _, err := MergeManaged(out, testConstraints(), false) + twice, _, err := MergeManaged(out, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.Equal(t, string(out), string(twice)) } @@ -231,7 +231,7 @@ requires-python = ">=3.10" [dependency-groups] dev = ["databricks-connect~=16.0.0", "pytest~=8.0"] `) - out, regions, err := MergeManaged(in, testConstraints(), false) + out, regions, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) // Sibling element and single-line array layout are preserved. assert.Contains(t, string(out), `dev = ["databricks-connect~=17.2.0", "pytest~=8.0"]`) @@ -247,7 +247,7 @@ dev = [ "databricks-connect~=16.0.0", ] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) // The trailing comma on the managed element is preserved. assert.Contains(t, string(out), ` "databricks-connect~=17.2.0",`) @@ -264,14 +264,14 @@ dev = [ "pytest~=8.0", ] `) - out, regions, err := MergeManaged(in, testConstraints(), false) + out, regions, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) assert.Contains(t, s, `"databricks-connect~=17.2.0",`) assert.Contains(t, s, `"pytest~=8.0",`, "existing element preserved") assert.Contains(t, regions, "databricks-connect") // Idempotent: a second merge finds the element and rewrites in place. - out2, _, err := MergeManaged(out, testConstraints(), false) + out2, _, err := MergeManaged(out, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.Equal(t, s, string(out2)) } @@ -283,10 +283,10 @@ requires-python = ">=3.10" [dependency-groups] dev = ["pytest~=8.0"] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.Contains(t, string(out), `dev = ["databricks-connect~=17.2.0", "pytest~=8.0"]`) - out2, _, err := MergeManaged(out, testConstraints(), false) + out2, _, err := MergeManaged(out, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.Equal(t, string(out), string(out2)) } @@ -298,7 +298,7 @@ requires-python = ">=3.10" [dependency-groups] dev = [] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.Contains(t, string(out), `dev = ["databricks-connect~=17.2.0"]`) } @@ -310,12 +310,12 @@ requires-python = ">=3.10" [dependency-groups] test = ["pytest~=8.0"] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) assert.Contains(t, s, `"databricks-connect~=17.2.0",`) assert.Contains(t, s, `test = ["pytest~=8.0"]`, "sibling group untouched") - out2, _, err := MergeManaged(out, testConstraints(), false) + out2, _, err := MergeManaged(out, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.Equal(t, s, string(out2)) } @@ -324,12 +324,12 @@ func TestMergeInsertsDependencyGroupsWhenAbsent(t *testing.T) { in := []byte(`[project] requires-python = ">=3.10" `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) assert.Contains(t, s, "[dependency-groups]") assert.Contains(t, s, `"databricks-connect~=17.2.0",`) - out2, _, err := MergeManaged(out, testConstraints(), false) + out2, _, err := MergeManaged(out, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.Equal(t, s, string(out2)) } @@ -345,7 +345,7 @@ dev = ["databricks-connect~=16.0.0", "pytest~=8.0", ] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) assert.Equal(t, 1, strings.Count(s, "databricks-connect"), "must not duplicate the pin") @@ -365,7 +365,7 @@ dev = [ "pytest~=8.0", ] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) assert.Equal(t, 1, strings.Count(s, "databricks-connect"), "must not duplicate the pin") @@ -384,7 +384,7 @@ dev = [ "pytest~=8.0", ] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) // Code element is rewritten; the comment mention is left verbatim. @@ -397,7 +397,7 @@ func TestMergeRewritesNonCanonicalDatabricksConnectSpelling(t *testing.T) { // place, not left undetected so the insert path adds a conflicting second pin. for _, spelling := range []string{"databricks_connect", "Databricks-Connect", "databricks.connect"} { in := []byte("[project]\nrequires-python = \">=3.10\"\n\n[dependency-groups]\ndev = [\n \"" + spelling + "~=16.0.0\",\n]\n") - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err, spelling) s := string(out) assert.Equal(t, 1, strings.Count(s, `"databricks-connect~=17.2.0"`), "spelling %q must be rewritten in place, not duplicated:\n%s", spelling, s) @@ -416,13 +416,13 @@ dev = [ "pytest~=8.0" ] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) assert.Contains(t, s, `"pytest~=8.0",`, "previous last element gains a separating comma") assert.Contains(t, s, `"databricks-connect~=17.2.0",`) // Round-trips as valid TOML. - out2, _, err := MergeManaged(out, testConstraints(), false) + out2, _, err := MergeManaged(out, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.Equal(t, s, string(out2)) } @@ -437,14 +437,14 @@ dev = ["pytest~=8.0"] `) c := testConstraints() c.DatabricksConnect = "" - out, regions, err := MergeManaged(in, c, false) + out, regions, err := MergeManaged(in, c, MergeOptions{}) require.NoError(t, err) assert.NotContains(t, string(out), "databricks-connect") assert.NotContains(t, regions, "databricks-connect") } func TestRenderFreshPyproject(t *testing.T) { - out := RenderFreshPyproject("demo", testConstraints(), false) + out := RenderFreshPyproject("demo", testConstraints(), MergeOptions{}) s := string(out) assert.Contains(t, s, `name = "demo"`) assert.Contains(t, s, `requires-python = "==3.12.*"`) @@ -455,7 +455,7 @@ func TestRenderFreshPyproject(t *testing.T) { // A cluster target (no EnvironmentVersion) writes no [tool.databricks.environment]. assert.NotContains(t, s, "[tool.databricks.environment]") // A fresh render is itself a no-op under MergeManaged (already fully managed). - merged, _, err := MergeManaged(out, testConstraints(), false) + merged, _, err := MergeManaged(out, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.Equal(t, s, string(merged)) } @@ -463,13 +463,13 @@ func TestRenderFreshPyproject(t *testing.T) { func TestRenderFreshPyprojectServerlessWritesEnvironment(t *testing.T) { c := testConstraints() c.EnvironmentVersion = "5" - out := RenderFreshPyproject("demo", c, false) + out := RenderFreshPyproject("demo", c, MergeOptions{}) s := string(out) assert.Contains(t, s, "[tool.databricks.environment]") assert.Contains(t, s, `environment_version = "5"`) requireValidTOML(t, out) // A fresh render is itself a no-op under MergeManaged (already fully managed). - merged, _, err := MergeManaged(out, c, false) + merged, _, err := MergeManaged(out, c, MergeOptions{}) require.NoError(t, err) assert.Equal(t, s, string(merged)) } @@ -483,7 +483,7 @@ dev = ["databricks-connect~=16.0.0"] `) c := testConstraints() c.EnvironmentVersion = "5" - out, regions, err := MergeManaged(in, c, false) + out, regions, err := MergeManaged(in, c, MergeOptions{}) require.NoError(t, err) s := string(out) assert.Contains(t, s, "[tool.databricks.environment]") @@ -491,7 +491,7 @@ dev = ["databricks-connect~=16.0.0"] assert.Contains(t, regions, regionDatabricksEnvironment) requireValidTOML(t, out) // Idempotent. - twice, _, err := MergeManaged(out, c, false) + twice, _, err := MergeManaged(out, c, MergeOptions{}) require.NoError(t, err) assert.Equal(t, s, string(twice)) } @@ -508,7 +508,7 @@ environment_version = "4" # pinned `) c := testConstraints() c.EnvironmentVersion = "5" - out, regions, err := MergeManaged(in, c, false) + out, regions, err := MergeManaged(in, c, MergeOptions{}) require.NoError(t, err) s := string(out) assert.Contains(t, s, `environment_version = "5" # pinned`) @@ -531,7 +531,7 @@ dev = ["databricks-connect~=16.0.0"] `) c := testConstraints() c.EnvironmentVersion = "5" - out, _, err := MergeManaged(in, c, false) + out, _, err := MergeManaged(in, c, MergeOptions{}) require.NoError(t, err) s := string(out) assert.Contains(t, s, `environment_version = "5"`) @@ -553,7 +553,7 @@ dev = ["databricks-connect~=16.0.0"] environment_version = "4" `) c := testConstraints() // EnvironmentVersion == "": cluster target. - out, regions, err := MergeManaged(in, c, false) + out, regions, err := MergeManaged(in, c, MergeOptions{}) require.NoError(t, err) assert.Contains(t, string(out), `environment_version = "4"`) assert.NotContains(t, regions, regionDatabricksEnvironment) @@ -584,7 +584,7 @@ constraint-dependencies = [ `) c := testConstraints() c.EnvironmentVersion = "5" - out, regions, err := MergeManaged(in, c, false) + out, regions, err := MergeManaged(in, c, MergeOptions{}) require.NoError(t, err) s := string(out) assert.Contains(t, s, "[tool.databricks.environment]") @@ -595,7 +595,7 @@ constraint-dependencies = [ assert.Equal(t, 1, countOccurrences(s, "[tool.databricks.environment]")) requireValidTOML(t, out) // Idempotent on the upgraded file. - twice, _, err := MergeManaged(out, c, false) + twice, _, err := MergeManaged(out, c, MergeOptions{}) require.NoError(t, err) assert.Equal(t, s, string(twice)) } @@ -617,7 +617,7 @@ constraint-dependencies = ["requests[security]~=2.0", "old-dep~=1.0", ] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) // The whole stale array is gone (both the bracket-bearing first element and @@ -629,7 +629,7 @@ constraint-dependencies = ["requests[security]~=2.0", requireValidTOML(t, out) assert.Equal(t, 1, countOccurrences(s, "[tool.uv]")) // Idempotent. - twice, _, err := MergeManaged(out, testConstraints(), false) + twice, _, err := MergeManaged(out, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.Equal(t, s, string(twice)) } @@ -663,7 +663,7 @@ dev = [ "databricks-connect~=16.0.0", ] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) // docs consolidated (its db-connect removed); dev updated to the managed pin. @@ -690,7 +690,7 @@ dev = [ "databricks-connect~=16.0", ] `) - out, regions, err := MergeManaged(in, testConstraints(), false) + out, regions, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) assert.NotContains(t, s, "databricks-connect==15.1.*", "the stray project.dependencies pin is removed") @@ -701,7 +701,7 @@ dev = [ requireValidTOML(t, out) // Idempotent: a second merge finds nothing to remove and produces identical bytes. - out2, _, err := MergeManaged(out, testConstraints(), false) + out2, _, err := MergeManaged(out, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.Equal(t, string(out), string(out2)) } @@ -718,7 +718,7 @@ spark = ["databricks-connect==15.0.0"] [dependency-groups] dev = ["databricks-connect~=16.0"] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) assert.Contains(t, s, "spark = []", "the optional-dependency extra is emptied") @@ -736,7 +736,7 @@ dependencies = ["numpy", "databricks-connect==15.1.*", "pytest"] [dependency-groups] dev = ["databricks-connect~=16.0"] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) assert.Contains(t, s, `dependencies = ["numpy", "pytest"]`) @@ -764,7 +764,7 @@ extra = ["databricks-connect"] dev = ["databricks-connect~=16.0"] test = ["databricks-connect>=15,<20"] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) assert.Contains(t, s, `"databricks-connect>=15",`, "an overlapping pin co-resolves and is kept") @@ -783,7 +783,7 @@ func TestMergeKeepsEnvEqualPinInProjectDeps(t *testing.T) { requires-python = ">=3.10" dependencies = ["databricks-connect~=17.2.0"] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.Contains(t, string(out), `dependencies = ["databricks-connect~=17.2.0"]`, "the env-equal pin is kept") requireValidTOML(t, out) @@ -803,7 +803,7 @@ dependencies = [ [dependency-groups] dev = ["databricks-connect~=16.0"] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) assert.NotContains(t, s, "databricks-connect==15.1.*") @@ -824,7 +824,7 @@ dev = ["databricks-connect~=16.0"] docs = ["databricks-connect~=14.3"] `) c := Constraints{RequiresPython: "==3.12.*", ConstraintDeps: []string{"pydantic~=2.10.6"}} - out, regions, err := MergeManaged(in, c, false) + out, regions, err := MergeManaged(in, c, MergeOptions{}) require.NoError(t, err) s := string(out) assert.Contains(t, s, "databricks-connect==15.1.*", "project.dependencies pin left untouched") @@ -868,7 +868,7 @@ dev = ["databricks-connect~=16.0"] "comment on the opening bracket line": "# runtime deps", } for name, in := range cases { - out, _, err := MergeManaged([]byte(in), testConstraints(), false) + out, _, err := MergeManaged([]byte(in), testConstraints(), MergeOptions{}) require.NoError(t, err, name) s := string(out) assert.NotContains(t, s, "databricks-connect==15.1.*", name) @@ -892,7 +892,7 @@ dev = [ "databricks-connect==15.0.0", ] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) assert.Contains(t, s, `"databricks-connect~=17.2.0",`, "the managed pin is kept and updated") @@ -911,7 +911,7 @@ requires-python = ">=3.10" dev = [{include-group = "spark"}] spark = ['databricks-connect==15.0.0'] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) assert.Contains(t, s, `{include-group = "spark"}`, "the include-group reference is preserved") @@ -928,7 +928,7 @@ requires-python = ">=3.10" [dependency-groups] dev = ["pytest"] # keep "databricks-connect~=14.3" for docs `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) // The comment's databricks-connect~=14.3 is preserved verbatim. @@ -943,7 +943,7 @@ requires-python = ">=3.10" # maintained by platform team [dependency-groups] dev = ["databricks-connect~=16.0.0"] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) assert.Contains(t, s, `requires-python = "==3.12.*" # maintained by platform team`) @@ -963,7 +963,7 @@ dev = ["databricks-connect~=16.0.0"] [tool.custom] # user table dev = ["databricks-connect==1.0.0"] # must not be managed `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) // [project].requires-python was found and updated despite the header comment. @@ -1002,7 +1002,7 @@ dev = ["databricks-connect~=16.0.0"] name = "internal" url = "https://packages.example/simple" `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) s := string(out) requireValidTOML(t, out) @@ -1011,7 +1011,7 @@ url = "https://packages.example/simple" assert.Contains(t, s, `name = "internal"`) assert.Contains(t, s, "pydantic~=2.10.6") // Merge-twice is byte-identical. - twice, _, err := MergeManaged(out, testConstraints(), false) + twice, _, err := MergeManaged(out, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.Equal(t, s, string(twice)) } @@ -1030,13 +1030,13 @@ constraint-dependencies = [] [dependency-groups] dev = ["databricks-connect~=16.0.0"] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) requireValidTOML(t, out) assert.Contains(t, string(out), "description = \"\"\"\nA multi-line project description.\n\"\"\"") assert.Contains(t, string(out), `requires-python = "==3.12.*"`) - twice, _, err := MergeManaged(out, testConstraints(), false) + twice, _, err := MergeManaged(out, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.Equal(t, out, twice) } @@ -1060,7 +1060,7 @@ dev = ["databricks-connect~=16.0.0"] [tool.uv] constraint-dependencies = [] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) requireValidTOML(t, out) s := string(out) @@ -1080,7 +1080,7 @@ description = """same-line value""" [dependency-groups] dev = ["databricks-connect~=16.0.0"] `) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) requireValidTOML(t, out) s := string(out) @@ -1092,7 +1092,7 @@ dev = ["databricks-connect~=16.0.0"] func TestMergePreservesCRLFInsideMultilineString(t *testing.T) { in := []byte("[project]\r\nrequires-python = \">=3.10\"\r\ndescription = \"\"\"\r\ncontinued\r\n\"\"\"\r\n\r\n[dependency-groups]\r\ndev = [\"databricks-connect~=16.0.0\"]\r\n") - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) requireValidTOML(t, out) assert.NotContains(t, strings.ReplaceAll(string(out), "\r\n", ""), "\n") @@ -1105,7 +1105,7 @@ requires-python = ">=3.10" description = """ unterminated `) - _, _, err := MergeManaged(in, testConstraints(), false) + _, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.ErrorIs(t, err, errMultilineString) } @@ -1126,12 +1126,12 @@ func TestMergePreservesMultilineStringDelimiterEdgeCases(t *testing.T) { in := []byte("[project]\nrequires-python = \">=3.10\"\ndescription = " + tt.value + "\n\n[dependency-groups]\ndev = [\"databricks-connect~=16.0.0\"]\n") requireValidTOML(t, in) - out, _, err := MergeManaged(in, testConstraints(), false) + out, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.NoError(t, err) requireValidTOML(t, out) assert.Contains(t, string(out), "description = "+tt.value) - twice, _, err := MergeManaged(out, testConstraints(), false) + twice, _, err := MergeManaged(out, testConstraints(), MergeOptions{}) require.NoError(t, err) assert.Equal(t, out, twice) }) @@ -1147,7 +1147,7 @@ dev = ["databricks-connect~=16.0.0"] [tool.uv] constraint-dependencies = ["old~=1.0"] `) - _, _, err := MergeManaged(in, testConstraints(), false) + _, _, err := MergeManaged(in, testConstraints(), MergeOptions{}) require.Error(t, err) assert.ErrorIs(t, err, errNoProjectTable) } @@ -1167,7 +1167,7 @@ requires-python = ">=3.9" dev = [] `) c := testConstraints() // carries requires-python = "==3.12.*" - out, regions, err := MergeManaged(in, c, true) + out, regions, err := MergeManaged(in, c, MergeOptions{SkipConstraints: true}) require.NoError(t, err) assert.Contains(t, string(out), `requires-python = ">=3.9"`) assert.NotContains(t, regions, regionRequiresPython) @@ -1184,7 +1184,7 @@ requires-python = "==3.12.*" dev = [] `) c := testConstraints() // carries constraint-dependencies - out, regions, err := MergeManaged(in, c, true) + out, regions, err := MergeManaged(in, c, MergeOptions{SkipConstraints: true}) require.NoError(t, err) assert.NotContains(t, string(out), "constraint-dependencies") assert.NotContains(t, regions, regionToolUv) @@ -1203,7 +1203,7 @@ dev = [] `) c := testConstraints() c.ConstraintDeps = nil - out, regions, err := MergeManaged(in, c, false) + out, regions, err := MergeManaged(in, c, MergeOptions{}) require.NoError(t, err) assert.Contains(t, regions, regionToolUv) assert.Contains(t, string(out), "constraint-dependencies = [") @@ -1215,10 +1215,43 @@ func TestRenderFreshPyprojectSkipConstraintsOmitsConstraints(t *testing.T) { // constraint block is rendered even though the Constraints carry them, but // databricks-connect (orthogonal) still is. c := testConstraints() // carries requires-python and constraint-dependencies - out := RenderFreshPyproject("demo", c, true) + out := RenderFreshPyproject("demo", c, MergeOptions{SkipConstraints: true}) s := string(out) assert.NotContains(t, s, "requires-python") assert.NotContains(t, s, "constraint-dependencies") assert.Contains(t, s, `"databricks-connect~=17.2.0",`) requireValidTOML(t, out) } + +func TestMergeManagedSkipDBConnectLeavesExistingPinUntouched(t *testing.T) { + // skipDBConnect=true leaves the user's databricks-connect pin untouched even + // though the Constraints carry a different managed pin — the flag, not an empty + // pin, drives the skip. The orthogonal requires-python is still managed. + in := []byte(`[project] +name = "demo" +requires-python = ">=3.9" + +[dependency-groups] +dev = ["databricks-connect~=16.0.0"] +`) + c := testConstraints() // carries databricks-connect~=17.2.0 + out, regions, err := MergeManaged(in, c, MergeOptions{SkipDBConnect: true}) + require.NoError(t, err) + assert.Contains(t, string(out), `"databricks-connect~=16.0.0"`) + assert.NotContains(t, string(out), "17.2.0") + assert.NotContains(t, regions, regionDatabricksConnect) +} + +func TestRenderFreshPyprojectSkipDBConnectOmitsDBConnect(t *testing.T) { + // Greenfield with skipDBConnect=true: the dev group is empty even though the + // Constraints carry a databricks-connect pin, but the orthogonal requires-python + // and constraint block still render. + c := testConstraints() // carries databricks-connect~=17.2.0 + out := RenderFreshPyproject("demo", c, MergeOptions{SkipDBConnect: true}) + s := string(out) + assert.NotContains(t, s, "databricks-connect") + assert.Contains(t, s, "dev = []") + assert.Contains(t, s, `requires-python = "==3.12.*"`) + assert.Contains(t, s, "constraint-dependencies") + requireValidTOML(t, out) +} diff --git a/libs/localenv/pipeline.go b/libs/localenv/pipeline.go index 89aa84dc65c..af599df8cf2 100644 --- a/libs/localenv/pipeline.go +++ b/libs/localenv/pipeline.go @@ -213,18 +213,22 @@ func (p *Pipeline) run(ctx context.Context) error { return p.fail(PhaseFetch, false, NewError(ErrFetch, err, "cannot parse python version from constraints %q", c.RequiresPython)) } - dbcPin := c.DatabricksConnect - if p.Mode == ModeConstraintsOnly { - // constraints-only stops *managing* the databricks-connect pin rather than - // removing it. Clearing dbcPin means the merge neither injects nor asserts a - // pin: greenfield renders dev = [] (no databricks-connect), while an existing - // project that already pins databricks-connect keeps its pin untouched (see - // mergeDatabricksConnect — an empty value is a no-op, not a deletion). - dbcPin = "" + // --no-dbconnect / --constraints-only stops *managing* the databricks-connect + // pin rather than removing it: the merge neither injects nor asserts a pin + // (greenfield renders dev = [], an existing pin is left untouched), the version + // is not reported, and validate skips the databricks-connect assertion. It is + // threaded explicitly (skipDBConnect / MergeOptions) rather than by clearing the + // pin, so c keeps the real artifact value. + skipDBConnect := p.Mode == ModeConstraintsOnly + opts := MergeOptions{SkipConstraints: p.SkipConstraints, SkipDBConnect: skipDBConnect} + + dbcVersion := dbcVersionFromPin(c.DatabricksConnect) + if skipDBConnect { + dbcVersion = "" } p.res.Resolved = &ResolvedInfo{ PythonVersion: pyMinor, - DBConnectVersion: dbcVersionFromPin(dbcPin), + DBConnectVersion: dbcVersion, ArtifactSource: artifactSource(c.FromCache), } @@ -232,7 +236,7 @@ func (p *Pipeline) run(ctx context.Context) error { // The serverless environment version (empty for cluster targets) is written // into [tool.databricks.environment] so the project also runs in serverless Jobs. p.report(ctx, PhaseMerge) - mergedBytes, greenfield, err := p.mergePlan(ctx, pyMinor, c, dbcPin, compute.ServerlessEnvironmentVersion()) + mergedBytes, greenfield, err := p.mergePlan(ctx, pyMinor, c, compute.ServerlessEnvironmentVersion(), opts) if err != nil { return err } @@ -260,7 +264,7 @@ func (p *Pipeline) run(ctx context.Context) error { // Phase: validate — assert the venv matches the target. p.report(ctx, PhaseValidate) - return p.validate(ctx, pyMinor, dbcPin) + return p.validate(ctx, pyMinor, c.DatabricksConnect, skipDBConnect) } // resolve runs ResolveCompute and records the resolve phase. @@ -351,10 +355,10 @@ func (p *Pipeline) plannedBackupName() (string, error) { // mergePlan computes the merged pyproject.toml bytes (without writing to disk), // decides greenfield vs. existing, and builds the Plan (populated only under -// --dry-run). dbcPin is the databricks-connect pin to inject, or "" in -// constraints-only mode. envVersion is the serverless environment version to -// write into [tool.databricks.environment], or "" for a cluster target. -func (p *Pipeline) mergePlan(_ context.Context, pyMinor string, c *Constraints, dbcPin, envVersion string) (merged []byte, greenfield bool, err error) { +// --dry-run). opts selects which managed axes are written. envVersion is the +// serverless environment version to write into [tool.databricks.environment], or +// "" for a cluster target. +func (p *Pipeline) mergePlan(_ context.Context, pyMinor string, c *Constraints, envVersion string, opts MergeOptions) (merged []byte, greenfield bool, err error) { pyproject := p.pyprojectPath() // The merge base is the live pyproject.toml, not the backup. MergeManaged @@ -379,47 +383,41 @@ func (p *Pipeline) mergePlan(_ context.Context, pyMinor string, c *Constraints, } greenfield = baseBytes == nil - // The artifact drives the merge; in constraints-only mode we clear the - // databricks-connect pin so it is neither written nor asserted. envVersion is - // the resolved serverless version (empty for cluster targets). + // The artifact drives the merge. envVersion is the resolved serverless version + // (empty for cluster targets); it is deliberately NOT gated by SkipDBConnect — + // unlike the databricks-connect pin (a managed *dependency* the mode opts out + // of), the environment version records the resolved compute *target*, which is + // still resolved. Recording it keeps the target discoverable for VS Code and + // serverless Jobs even when the databricks-connect dependency is turned off. // - // envVersion is deliberately NOT cleared in constraints-only mode: unlike the - // databricks-connect pin (a managed *dependency* the mode opts out of), the - // environment version records the resolved compute *target*, which the mode - // still resolves. Recording it keeps the target discoverable for VS Code and - // serverless Jobs even when dependency management is turned off. + // The skip axes in opts are threaded explicitly into the merge, fresh render, + // and warning detector rather than encoded by clearing effective's values, so + // those functions see the real artifact values and the intent is unambiguous. + // Each governs only what is *written*: a provisioning run still installs and + // validates the resolved Python (pyMinor), so if a kept requires-python is + // disjoint from the target, uv surfaces a normal E_PROVISION rather than the + // command guessing an alternative. effective := *c - effective.DatabricksConnect = dbcPin effective.EnvironmentVersion = envVersion - // --no-constraints (p.SkipConstraints) leaves the remote Python version and - // dependency pins unmanaged. It is threaded explicitly into the merge, fresh - // render, and warning detector below rather than encoded by clearing - // effective's values, so those functions see the real artifact values and the - // intent is unambiguous. - // - // The flag governs only what is *written*. A provisioning run still installs - // and validates the resolved Python (pyMinor), so if the user's kept - // requires-python is disjoint from the target, uv surfaces it as a normal - // E_PROVISION rather than this command guessing an alternative. var changedRegions []string if greenfield { // No existing pyproject.toml — render a fresh one. The project name comes // from the directory name as a reasonable default. Only the regions actually - // rendered are reported (requires-python and tool.uv are omitted under - // --no-constraints). - merged = RenderFreshPyproject(projectName(p.ProjectDir), effective, p.SkipConstraints) - if !p.SkipConstraints { + // rendered are reported (requires-python and tool.uv omitted under + // --no-constraints; databricks-connect omitted under --no-dbconnect). + merged = RenderFreshPyproject(projectName(p.ProjectDir), effective, opts) + if !opts.SkipConstraints { changedRegions = append(changedRegions, regionRequiresPython, regionToolUv) } - if dbcPin != "" { + if !opts.SkipDBConnect && c.DatabricksConnect != "" { changedRegions = append(changedRegions, regionDatabricksConnect) } if envVersion != "" { changedRegions = append(changedRegions, regionDatabricksEnvironment) } } else { - merged, changedRegions, err = MergeManaged(baseBytes, effective, p.SkipConstraints) + merged, changedRegions, err = MergeManaged(baseBytes, effective, opts) if err != nil { return nil, greenfield, p.fail(PhaseMerge, false, NewError(ErrMerge, err, "merge managed regions failed")) } @@ -430,7 +428,7 @@ func (p *Pipeline) mergePlan(_ context.Context, pyMinor string, c *Constraints, // edits come from the merge itself (planDBConnect), so a warning can never claim a // rewrite or removal that did not happen. p.res.Warnings = append(p.res.Warnings, - detectMergeWarnings(baseBytes, effective, planDBConnect(baseBytes, effective), p.SkipConstraints)...) + detectMergeWarnings(baseBytes, effective, planDBConnect(baseBytes, effective, opts.SkipDBConnect), opts)...) } // Under --dry-run, build the plan (with a diff) for reporting. A real run does @@ -538,9 +536,10 @@ func (p *Pipeline) provision(ctx context.Context, pyMinor string) error { } // validate reads the Python and databricks-connect versions from the venv and -// populates the venv path. dbcPin is "" in constraints-only mode, where the DB -// Connect assertion is skipped. -func (p *Pipeline) validate(ctx context.Context, expectedPyMinor, dbcPin string) error { +// populates the venv path. dbcPin is the resolved databricks-connect pin (the real +// artifact value); skipDBConnect is true under --no-dbconnect / --constraints-only, +// where the databricks-connect assertion and version reporting are skipped. +func (p *Pipeline) validate(ctx context.Context, expectedPyMinor, dbcPin string, skipDBConnect bool) error { info, err := p.PM.Validate(ctx, p.ProjectDir) if err != nil { return p.fail(PhaseValidate, true, asPipelineError(err, ErrValidate, "validation failed")) @@ -568,9 +567,10 @@ func (p *Pipeline) validate(ctx context.Context, expectedPyMinor, dbcPin string) "python version mismatch: want %s, got %s", expectedPyMinor, pyVer)) } - // In default mode, assert the installed databricks-connect major matches the + // When databricks-connect is managed, assert the installed major matches the // pin's major. dbcPin is e.g. "databricks-connect~=17.2.0"; dbcVer is "17.2.0". - if dbcPin != "" { + // Skipped under --no-dbconnect / --constraints-only. + if !skipDBConnect && dbcPin != "" { pinMajor := dbcMajorFromPin(dbcPin) if pinMajor == "" { return p.fail(PhaseValidate, true, NewError(ErrValidate, nil, @@ -587,10 +587,10 @@ func (p *Pipeline) validate(ctx context.Context, expectedPyMinor, dbcPin string) } } - // Report the installed databricks-connect version only in default mode. In - // constraints-only mode databricks-connect is not a managed dependency, so the - // spec omits dbconnectVersion even if the package is present transitively. - defaultMode := dbcPin != "" + // Report the installed databricks-connect version only when it is a managed + // dependency. Under --no-dbconnect / --constraints-only it is not, so the spec + // omits dbconnectVersion even if the package is present transitively. + defaultMode := !skipDBConnect && dbcPin != "" detail := "python=" + pyVer if defaultMode && dbcVer != "" { diff --git a/libs/localenv/warnings.go b/libs/localenv/warnings.go index e0bb2a1b469..afb452f728e 100644 --- a/libs/localenv/warnings.go +++ b/libs/localenv/warnings.go @@ -173,12 +173,13 @@ func resolutionRequirements(p userPyprojectTOML) []string { // deterministic and ordered (requires-python, then databricks-connect, then the // standalone-pyspark collision, then constraint conflicts in the order uv would // encounter them) so goldens are stable. -// skipConstraints (--no-constraints) suppresses the requires-python and -// constraint-dependency warnings: when those regions are left unmanaged the merge -// neither overrides the user's requires-python nor imposes constraints, so there is -// nothing to warn about. It is threaded explicitly for the same reason MergeManaged -// takes it — the intent is not inferred from empty/nil values in c. -func detectMergeWarnings(userPyproject []byte, c Constraints, plan dbconnectPlan, skipConstraints bool) []Warning { +// opts suppresses the warnings for whichever axes are left unmanaged: under +// SkipConstraints the merge neither overrides the user's requires-python nor imposes +// constraints, and under SkipDBConnect it neither injects nor rewrites the +// databricks-connect pin — so there is nothing to warn about on those axes. The +// flags are threaded explicitly for the same reason MergeManaged takes them: intent +// is not inferred from empty/nil values in c. +func detectMergeWarnings(userPyproject []byte, c Constraints, plan dbconnectPlan, opts MergeOptions) []Warning { if len(userPyproject) == 0 { return nil } @@ -195,7 +196,7 @@ func detectMergeWarnings(userPyproject []byte, c Constraints, plan dbconnectPlan // The user pinned a requires-python that differs from the env's pin; the merge // replaces it with the managed value. Skipped under --no-constraints, where the // merge leaves the user's requires-python untouched. - if up := strings.TrimSpace(p.Project.RequiresPython); !skipConstraints && up != "" && c.RequiresPython != "" && up != strings.TrimSpace(c.RequiresPython) { + if up := strings.TrimSpace(p.Project.RequiresPython); !opts.SkipConstraints && up != "" && c.RequiresPython != "" && up != strings.TrimSpace(c.RequiresPython) { warnings = append(warnings, Warning{ Code: WarnRequiresPythonOverridden, Message: fmt.Sprintf("requires-python %q is replaced by the environment's %q", up, c.RequiresPython), @@ -208,26 +209,26 @@ func detectMergeWarnings(userPyproject []byte, c Constraints, plan dbconnectPlan // survive it, so excluding both keeps the conflict scan honest. survivors := removeDBConnectPins(retainedRequirements(p, plan.replacedDevPin), plan.removed) - // The user's databricks-connect pins differ from the env's. Only meaningful in - // default mode (c.DatabricksConnect is empty in constraints-only, where - // databricks-connect is left untouched). - if c.DatabricksConnect != "" { + // The user's databricks-connect pins differ from the env's. Only meaningful when + // databricks-connect is managed: skipped under --no-dbconnect / --constraints-only, + // and a no-op when the artifact carries no pin. + managesDBConnect := !opts.SkipDBConnect && c.DatabricksConnect != "" + if managesDBConnect { warnings = append(warnings, dbconnectWarnings(plan, survivors, c.DatabricksConnect)...) } // A standalone pyspark collides with databricks-connect's vendored pyspark whenever - // databricks-connect ends up in the environment — whether the env manages it (default - // mode, c.DatabricksConnect set) or the user's own pyproject pins it (kept as-is in - // constraints-only mode, since mergeDatabricksConnect is a no-op on an empty managed - // value). Gate on its presence by either route, not on the mode, so this agrees with - // the validate hard-fail, which keys on the installed venv rather than the mode. - if c.DatabricksConnect != "" || len(dbconnectPins(survivors)) > 0 { + // databricks-connect ends up in the environment — whether the env manages it (a pin + // is managed) or the user's own pyproject pins it (kept as-is when databricks-connect + // is skipped). Gate on its presence by either route, not on the mode, so this agrees + // with the validate hard-fail, which keys on the installed venv rather than the mode. + if managesDBConnect || len(dbconnectPins(survivors)) > 0 { warnings = append(warnings, standalonePysparkWarnings(survivors)...) } // Skipped under --no-constraints: no constraint block is written, so the user's // requirements cannot conflict with a managed one. - if !skipConstraints { + if !opts.SkipConstraints { warnings = append(warnings, constraintConflicts(survivors, c.ConstraintDeps)...) } diff --git a/libs/localenv/warnings_test.go b/libs/localenv/warnings_test.go index ff5a2c25351..00078191de0 100644 --- a/libs/localenv/warnings_test.go +++ b/libs/localenv/warnings_test.go @@ -12,7 +12,7 @@ import ( // against a different notion of what the merge rewrites or removes than production uses — // that divergence is the bug this wiring exists to prevent. func detectWarnings(userPyproject []byte, c Constraints) []Warning { - return detectMergeWarnings(userPyproject, c, planDBConnect(userPyproject, c), false) + return detectMergeWarnings(userPyproject, c, planDBConnect(userPyproject, c, false), MergeOptions{}) } // codes extracts the warning codes in order for concise assertions. @@ -63,10 +63,29 @@ dev = ["databricks-connect~=16.1.0", "pydantic~=1.0"] DatabricksConnect: "databricks-connect~=18.0.0", ConstraintDeps: []string{"pydantic~=2.10.6"}, } - got := detectMergeWarnings(user, c, planDBConnect(user, c), true) + got := detectMergeWarnings(user, c, planDBConnect(user, c, false), MergeOptions{SkipConstraints: true}) assert.Equal(t, []string{WarnDBConnectPinOverridden}, codes(got)) } +func TestDetectMergeWarningsSkipDBConnectSuppressesDBConnectWarning(t *testing.T) { + // Under --no-dbconnect the merge leaves the databricks-connect pin unmanaged, so + // no databricks-connect override warning is reported — but the orthogonal + // requires-python override still is. + user := []byte(`[project] +name = "demo" +requires-python = ">=3.10" + +[dependency-groups] +dev = ["databricks-connect~=16.1.0"] +`) + c := Constraints{ + RequiresPython: "==3.12.*", + DatabricksConnect: "databricks-connect~=18.0.0", + } + got := detectMergeWarnings(user, c, planDBConnect(user, c, true), MergeOptions{SkipDBConnect: true}) + assert.Equal(t, []string{WarnRequiresPythonOverridden}, codes(got)) +} + func TestDetectMergeWarningsWithMultilineString(t *testing.T) { user := []byte(`[project] requires-python = ">=3.10" @@ -321,7 +340,7 @@ spark = ["databricks-connect==15.0.0"] // Consolidation actually fixes the project: re-running on the merged file finds a // single managed pin and nothing to warn about (the spark stray is gone). - merged, _, err := MergeManaged(both, c, false) + merged, _, err := MergeManaged(both, c, MergeOptions{}) require.NoError(t, err) assert.Empty(t, detectWarnings(merged, c)) @@ -529,7 +548,7 @@ Dev = ["databricks-connect==16.1.0"] assert.Equal(t, []string{WarnDBConnectConsolidated}, codes(got)) assert.NotContains(t, got[0].Message, "is replaced") - merged, _, err := MergeManaged(user, c, false) + merged, _, err := MergeManaged(user, c, MergeOptions{}) require.NoError(t, err) assert.NotContains(t, string(merged), `databricks-connect==16.1.0`, "the stray pin is removed, not retained") } @@ -565,7 +584,7 @@ requires-python = "==3.12.*" assert.NotContains(t, got[0].Message, "is replaced", name) // The merged file really does carry both pins, which is what the code reports. - merged, _, err := MergeManaged([]byte(body), c, false) + merged, _, err := MergeManaged([]byte(body), c, MergeOptions{}) require.NoError(t, err, name) assert.Contains(t, string(merged), "databricks-connect==16.1.0", name) assert.Contains(t, string(merged), "databricks-connect==17.0.0", name) @@ -581,7 +600,7 @@ Dev = ["databricks-connect==16.1.0"] `) got := detectWarnings(capitalized, c) assert.Equal(t, []string{WarnDBConnectConsolidated}, codes(got)) - merged, _, err := MergeManaged(capitalized, c, false) + merged, _, err := MergeManaged(capitalized, c, MergeOptions{}) require.NoError(t, err) assert.NotContains(t, string(merged), "databricks-connect==16.1.0") assert.Contains(t, string(merged), "databricks-connect==17.0.0") From acb905c311e1ddc02def034320f4a4ef6fa32c91 Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Fri, 4 Sep 2026 13:36:44 +0200 Subject: [PATCH 6/6] Gate the [project] guard on SkipConstraints; pass MergeOptions through consistently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *Why* Review follow-ups on the MergeOptions change: - MergeManaged rejected a pyproject.toml without a [project] table even under --no-constraints, though [project] is required only to hold requires-python — which that mode does not write. A valid uv file with only dependency groups / tool config failed with E_MERGE for a reason that no longer applied. Confirmed uv itself syncs a [project]-less file, so relaxing the guard is safe end to end. - planDBConnect and validate still took a bare skip bool, reintroducing the boolean-blind call sites MergeOptions was meant to remove. - Several comments restated flag semantics already documented on MergeOptions. *What* - Gate the [project] guard on !opts.SkipConstraints, so a [project]-less file is merged for its other axes under --no-constraints instead of rejected. - Pass MergeOptions through planDBConnect and validate instead of a bare bool. - Trim comments that duplicated the MergeOptions field docs. Behaviour is unchanged for every existing case (all have [project]); the only new behaviour is that --no-constraints now succeeds on a [project]-less file. *Verification* - go build ./..., go vet, gofmt clean. - go test ./libs/localenv/... ./cmd/environments/... pass; new unit test covers a [project]-less file merging under SkipConstraints (verified it fails with the guard ungated before restoring). - go test ./acceptance -run TestAccept/localenv passes with goldens unchanged. Co-authored-by: Isaac --- libs/localenv/merge.go | 27 ++++++++------- libs/localenv/merge_test.go | 16 +++++++++ libs/localenv/pipeline.go | 63 +++++++++++++++------------------- libs/localenv/warnings.go | 8 ++--- libs/localenv/warnings_test.go | 6 ++-- 5 files changed, 63 insertions(+), 57 deletions(-) diff --git a/libs/localenv/merge.go b/libs/localenv/merge.go index e3cdbca3121..388feed0bbd 100644 --- a/libs/localenv/merge.go +++ b/libs/localenv/merge.go @@ -81,8 +81,8 @@ type dbconnectPlan struct { // multi-line string protection) and runs both databricks-connect passes on a clone, // in the same order as MergeManaged, so replacedDevPin and removed match what the // real merge does. -func planDBConnect(target []byte, c Constraints, skipDBConnect bool) dbconnectPlan { - if skipDBConnect || c.DatabricksConnect == "" { +func planDBConnect(target []byte, c Constraints, opts MergeOptions) dbconnectPlan { + if opts.SkipDBConnect || c.DatabricksConnect == "" { return dbconnectPlan{} } // Mirror MergeManaged's own preprocessing so the same lines are inspected. @@ -140,12 +140,15 @@ func MergeManaged(target []byte, c Constraints, opts MergeOptions) (merged []byt } lines := strings.Split(protected, "\n") - // requires-python is a managed value; if there is no [project] table to hold - // it, this is not a file we can faithfully merge (greenfield goes through - // RenderFreshPyproject, which always writes [project]). Fail loudly rather - // than silently skip the version pin. - if _, _, ok := tableBounds(lines, "[project]"); !ok { - return nil, nil, errNoProjectTable + // requires-python needs a [project] table to hold it; without one the merge + // would silently drop the pin, so fail loudly instead. Only enforced when + // constraints are managed — under SkipConstraints there is no requires-python to + // write, so a [project]-less file (e.g. one with only dependency groups) is + // merged for its other axes rather than rejected. + if !opts.SkipConstraints { + if _, _, ok := tableBounds(lines, "[project]"); !ok { + return nil, nil, errNoProjectTable + } } if !opts.SkipConstraints { @@ -1264,8 +1267,7 @@ func RenderFreshPyproject(projectName string, c Constraints, opts MergeOptions) fmt.Fprintf(&b, "name = %q\n", projectName) // uv requires project.version when a [project] table is present. fmt.Fprintf(&b, "version = %q\n", freshProjectVersion) - // requires-python is omitted when constraints are skipped (--no-constraints), - // which lets uv pick the interpreter for the fresh project. + // Omitted under SkipConstraints, letting uv pick the interpreter for the project. if !opts.SkipConstraints { fmt.Fprintf(&b, "requires-python = %q\n", c.RequiresPython) } @@ -1286,9 +1288,8 @@ func RenderFreshPyproject(projectName string, c Constraints, opts MergeOptions) fmt.Fprintf(&b, "environment_version = %q\n", c.EnvironmentVersion) b.WriteString("\n") } - // The [tool.uv] constraint block is omitted when constraints are skipped - // (--no-constraints); otherwise it is always written (an empty block when the - // artifact carries no constraint-dependencies). + // Written whenever constraints are managed — an empty block when the artifact + // carries no constraint-dependencies (nil and empty are treated alike). if !opts.SkipConstraints { for _, line := range renderToolUvBlock(c.ConstraintDeps, true) { b.WriteString(line) diff --git a/libs/localenv/merge_test.go b/libs/localenv/merge_test.go index 96ec013a523..3e175ae0e2a 100644 --- a/libs/localenv/merge_test.go +++ b/libs/localenv/merge_test.go @@ -1152,6 +1152,22 @@ constraint-dependencies = ["old~=1.0"] assert.ErrorIs(t, err, errNoProjectTable) } +func TestMergeManagedSkipConstraintsAllowsMissingProjectTable(t *testing.T) { + // The [project] table is required only to hold requires-python. Under + // SkipConstraints there is none to write, so a [project]-less file (e.g. one with + // only dependency groups) is merged for its other axes rather than rejected. + in := []byte(`[dependency-groups] +dev = ["databricks-connect~=16.0.0"] +`) + out, regions, err := MergeManaged(in, testConstraints(), MergeOptions{SkipConstraints: true}) + require.NoError(t, err) + // databricks-connect is still reconciled; requires-python is not written. + assert.Contains(t, regions, regionDatabricksConnect) + assert.Contains(t, string(out), `"databricks-connect~=17.2.0"`) + assert.NotContains(t, string(out), "requires-python") + requireValidTOML(t, out) +} + func countOccurrences(s, substr string) int { return strings.Count(s, substr) } diff --git a/libs/localenv/pipeline.go b/libs/localenv/pipeline.go index af599df8cf2..23eb71b2981 100644 --- a/libs/localenv/pipeline.go +++ b/libs/localenv/pipeline.go @@ -213,17 +213,17 @@ func (p *Pipeline) run(ctx context.Context) error { return p.fail(PhaseFetch, false, NewError(ErrFetch, err, "cannot parse python version from constraints %q", c.RequiresPython)) } - // --no-dbconnect / --constraints-only stops *managing* the databricks-connect - // pin rather than removing it: the merge neither injects nor asserts a pin - // (greenfield renders dev = [], an existing pin is left untouched), the version - // is not reported, and validate skips the databricks-connect assertion. It is - // threaded explicitly (skipDBConnect / MergeOptions) rather than by clearing the - // pin, so c keeps the real artifact value. - skipDBConnect := p.Mode == ModeConstraintsOnly - opts := MergeOptions{SkipConstraints: p.SkipConstraints, SkipDBConnect: skipDBConnect} + // The skip axes are threaded explicitly rather than encoded by clearing c's + // values, so c keeps the real artifact values throughout. --constraints-only maps + // to --no-dbconnect: it stops managing the pin (not reported here, not asserted in + // validate) but keeps the resolved value on c. + opts := MergeOptions{ + SkipConstraints: p.SkipConstraints, + SkipDBConnect: p.Mode == ModeConstraintsOnly, + } dbcVersion := dbcVersionFromPin(c.DatabricksConnect) - if skipDBConnect { + if opts.SkipDBConnect { dbcVersion = "" } p.res.Resolved = &ResolvedInfo{ @@ -264,7 +264,7 @@ func (p *Pipeline) run(ctx context.Context) error { // Phase: validate — assert the venv matches the target. p.report(ctx, PhaseValidate) - return p.validate(ctx, pyMinor, c.DatabricksConnect, skipDBConnect) + return p.validate(ctx, pyMinor, c.DatabricksConnect, opts) } // resolve runs ResolveCompute and records the resolve phase. @@ -383,20 +383,14 @@ func (p *Pipeline) mergePlan(_ context.Context, pyMinor string, c *Constraints, } greenfield = baseBytes == nil - // The artifact drives the merge. envVersion is the resolved serverless version - // (empty for cluster targets); it is deliberately NOT gated by SkipDBConnect — - // unlike the databricks-connect pin (a managed *dependency* the mode opts out - // of), the environment version records the resolved compute *target*, which is - // still resolved. Recording it keeps the target discoverable for VS Code and - // serverless Jobs even when the databricks-connect dependency is turned off. + // envVersion (empty for cluster targets) is deliberately NOT gated by + // SkipDBConnect: unlike the databricks-connect pin (a managed *dependency* the + // mode opts out of), it records the resolved compute *target*, kept so VS Code + // and serverless Jobs can still discover it. // - // The skip axes in opts are threaded explicitly into the merge, fresh render, - // and warning detector rather than encoded by clearing effective's values, so - // those functions see the real artifact values and the intent is unambiguous. - // Each governs only what is *written*: a provisioning run still installs and - // validates the resolved Python (pyMinor), so if a kept requires-python is - // disjoint from the target, uv surfaces a normal E_PROVISION rather than the - // command guessing an alternative. + // The skip axes govern only what is *written*: provisioning still installs and + // validates the resolved Python (pyMinor), so a kept requires-python disjoint + // from the target surfaces as a normal E_PROVISION, not a guessed alternative. effective := *c effective.EnvironmentVersion = envVersion @@ -428,7 +422,7 @@ func (p *Pipeline) mergePlan(_ context.Context, pyMinor string, c *Constraints, // edits come from the merge itself (planDBConnect), so a warning can never claim a // rewrite or removal that did not happen. p.res.Warnings = append(p.res.Warnings, - detectMergeWarnings(baseBytes, effective, planDBConnect(baseBytes, effective, opts.SkipDBConnect), opts)...) + detectMergeWarnings(baseBytes, effective, planDBConnect(baseBytes, effective, opts), opts)...) } // Under --dry-run, build the plan (with a diff) for reporting. A real run does @@ -536,10 +530,10 @@ func (p *Pipeline) provision(ctx context.Context, pyMinor string) error { } // validate reads the Python and databricks-connect versions from the venv and -// populates the venv path. dbcPin is the resolved databricks-connect pin (the real -// artifact value); skipDBConnect is true under --no-dbconnect / --constraints-only, -// where the databricks-connect assertion and version reporting are skipped. -func (p *Pipeline) validate(ctx context.Context, expectedPyMinor, dbcPin string, skipDBConnect bool) error { +// populates the venv path. dbcPin is the resolved databricks-connect pin; when it +// is not managed (opts.SkipDBConnect) the databricks-connect assertion and version +// reporting are skipped. +func (p *Pipeline) validate(ctx context.Context, expectedPyMinor, dbcPin string, opts MergeOptions) error { info, err := p.PM.Validate(ctx, p.ProjectDir) if err != nil { return p.fail(PhaseValidate, true, asPipelineError(err, ErrValidate, "validation failed")) @@ -569,8 +563,7 @@ func (p *Pipeline) validate(ctx context.Context, expectedPyMinor, dbcPin string, // When databricks-connect is managed, assert the installed major matches the // pin's major. dbcPin is e.g. "databricks-connect~=17.2.0"; dbcVer is "17.2.0". - // Skipped under --no-dbconnect / --constraints-only. - if !skipDBConnect && dbcPin != "" { + if !opts.SkipDBConnect && dbcPin != "" { pinMajor := dbcMajorFromPin(dbcPin) if pinMajor == "" { return p.fail(PhaseValidate, true, NewError(ErrValidate, nil, @@ -588,12 +581,12 @@ func (p *Pipeline) validate(ctx context.Context, expectedPyMinor, dbcPin string, } // Report the installed databricks-connect version only when it is a managed - // dependency. Under --no-dbconnect / --constraints-only it is not, so the spec - // omits dbconnectVersion even if the package is present transitively. - defaultMode := !skipDBConnect && dbcPin != "" + // dependency, so the spec omits dbconnectVersion even if the package is present + // transitively. + managed := !opts.SkipDBConnect && dbcPin != "" detail := "python=" + pyVer - if defaultMode && dbcVer != "" { + if managed && dbcVer != "" { detail += " databricks-connect=" + dbcVer } p.markOK(PhaseValidate, detail) @@ -604,7 +597,7 @@ func (p *Pipeline) validate(ctx context.Context, expectedPyMinor, dbcPin string, // sets the working directory when it shells out). venvDir is already ".venv". p.res.VenvPath = venvDir if p.res.Resolved != nil { - if defaultMode { + if managed { p.res.Resolved.DBConnectVersion = dbcVer } else { p.res.Resolved.DBConnectVersion = "" diff --git a/libs/localenv/warnings.go b/libs/localenv/warnings.go index afb452f728e..b94ee3f2be1 100644 --- a/libs/localenv/warnings.go +++ b/libs/localenv/warnings.go @@ -173,12 +173,8 @@ func resolutionRequirements(p userPyprojectTOML) []string { // deterministic and ordered (requires-python, then databricks-connect, then the // standalone-pyspark collision, then constraint conflicts in the order uv would // encounter them) so goldens are stable. -// opts suppresses the warnings for whichever axes are left unmanaged: under -// SkipConstraints the merge neither overrides the user's requires-python nor imposes -// constraints, and under SkipDBConnect it neither injects nor rewrites the -// databricks-connect pin — so there is nothing to warn about on those axes. The -// flags are threaded explicitly for the same reason MergeManaged takes them: intent -// is not inferred from empty/nil values in c. +// opts suppresses the warnings for whichever axes are left unmanaged: a skipped +// axis is not written, so there is nothing on it to warn about. func detectMergeWarnings(userPyproject []byte, c Constraints, plan dbconnectPlan, opts MergeOptions) []Warning { if len(userPyproject) == 0 { return nil diff --git a/libs/localenv/warnings_test.go b/libs/localenv/warnings_test.go index 00078191de0..01bd35ac00b 100644 --- a/libs/localenv/warnings_test.go +++ b/libs/localenv/warnings_test.go @@ -12,7 +12,7 @@ import ( // against a different notion of what the merge rewrites or removes than production uses — // that divergence is the bug this wiring exists to prevent. func detectWarnings(userPyproject []byte, c Constraints) []Warning { - return detectMergeWarnings(userPyproject, c, planDBConnect(userPyproject, c, false), MergeOptions{}) + return detectMergeWarnings(userPyproject, c, planDBConnect(userPyproject, c, MergeOptions{}), MergeOptions{}) } // codes extracts the warning codes in order for concise assertions. @@ -63,7 +63,7 @@ dev = ["databricks-connect~=16.1.0", "pydantic~=1.0"] DatabricksConnect: "databricks-connect~=18.0.0", ConstraintDeps: []string{"pydantic~=2.10.6"}, } - got := detectMergeWarnings(user, c, planDBConnect(user, c, false), MergeOptions{SkipConstraints: true}) + got := detectMergeWarnings(user, c, planDBConnect(user, c, MergeOptions{}), MergeOptions{SkipConstraints: true}) assert.Equal(t, []string{WarnDBConnectPinOverridden}, codes(got)) } @@ -82,7 +82,7 @@ dev = ["databricks-connect~=16.1.0"] RequiresPython: "==3.12.*", DatabricksConnect: "databricks-connect~=18.0.0", } - got := detectMergeWarnings(user, c, planDBConnect(user, c, true), MergeOptions{SkipDBConnect: true}) + got := detectMergeWarnings(user, c, planDBConnect(user, c, MergeOptions{SkipDBConnect: true}), MergeOptions{SkipDBConnect: true}) assert.Equal(t, []string{WarnRequiresPythonOverridden}, codes(got)) }