Skip to content

libs/dyn/merge: clear an inherited value when an override is null#5949

Open
apoorva-01 wants to merge 1 commit into
databricks:mainfrom
apoorva-01:merge-null-clears-inherited
Open

libs/dyn/merge: clear an inherited value when an override is null#5949
apoorva-01 wants to merge 1 commit into
databricks:mainfrom
apoorva-01:merge-null-clears-inherited

Conversation

@apoorva-01

Copy link
Copy Markdown

Changes

An explicit null in an override now clears the inherited value instead of being ignored, so autoscale: null in a target actually drops the base autoscale block.

I kept it scoped to map entries in merge.Merge (key present + incoming nil = clear). Absent keys aren't affected and top-level Merge(x, nil) == x still holds. The tidier alternative is making nil always win in merge() itself, but that flips a documented invariant three tests assert, and touches every caller. Happy to go that way if you'd rather.

Why

Closes #5701. There's currently no way to drop an inherited field in a target, so you can't switch a job cluster to single-node for just one target.

Tests

Unit test for a null override clearing an existing entry (checks value and locations), plus a no_autoscale target in the override/clusters acceptance test. Both fail without the fix.

A null in a target (e.g. autoscale: null) was treated as "no value" and
ignored, so there was no way to drop a field inherited from the base.
@github-actions

Copy link
Copy Markdown
Contributor

An authorized user can trigger integration tests manually by following the instructions below:

Trigger:
go/deco-tests-run/cli

Inputs:

  • PR number: 5949
  • Commit SHA: 6202847265647cc657ce68f4a17b4d733df56693

Checks will be approved automatically on success.

@github-actions

Copy link
Copy Markdown
Contributor

Approval status: pending

/acceptance/bundle/ - needs approval

Files: acceptance/bundle/override/clusters/databricks.yml, acceptance/bundle/override/clusters/output.txt, acceptance/bundle/override/clusters/script
Suggested: @janniklasrose
Also eligible: @pietern, @denik, @shreyas-goenka, @andrewnester, @anton-107, @lennartkats-db

General files (require maintainer)

Files: .nextchanges/bundles/merge-null-target-override.md, libs/dyn/merge/merge.go, libs/dyn/merge/merge_test.go
Based on git history:

  • @janniklasrose -- recent work in .nextchanges/bundles/

Any maintainer (@andrewnester, @anton-107, @denik, @pietern, @shreyas-goenka, @simonfaltum, @renaudhartert-db, @janniklasrose, @lennartkats-db) can approve all areas.
See OWNERS for ownership rules.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

deep merge can't nullify autoscale settings in target.

1 participant