Skip to content

bundle: keep resources.json in step with DMS, marked by a state feature - #6475

Open
shreyas-goenka wants to merge 5 commits into
isaac/pr6052-fixesfrom
isaac/dms-dual-state
Open

bundle: keep resources.json in step with DMS, marked by a state feature#6475
shreyas-goenka wants to merge 5 commits into
isaac/pr6052-fixesfrom
isaac/dms-dual-state

Conversation

@shreyas-goenka

@shreyas-goenka shreyas-goenka commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Changes

Stacked on #6094. Two things:

  1. resources.json records the recording feature: state_version: 3 plus features: {record_deployment_history: {}}, using the feature-flag scaffolding already in dstate. The migration guard keys off that marker instead of "has any resources", and turning recording off for a marked deployment is refused. While recording, the file's resource set is never read back — the service is asked on every open.
  2. The acceptance id helpers (read_id.py, replace_ids.py, read_state.py) read ids and state from the deployment metadata service when recording is on, via new acceptance/bin/dms_resources.py. It resolves the deployment from the workspace node the service registers (the node's object id is the deployment id, per libs/dms/resolve.go) and lists its resources, which carry both resource_id and the recorded state.

Why

A CLI that does not know the feature must refuse the state rather than deploy over a deployment it would leave the service out of step with. The feature list is the existing gate for exactly that. (2) is groundwork for making the file a hollow marker: once the helpers no longer read resources out of it, the deploy no longer needs to write them.

Not included: hollowing the file. It still needs bundle generate, bind and configsync to get a DMS client — they open the state with none and read __id__ from it, so with a hollow file generate calls GET /api/2.0/lakeview/dashboards/ with an empty id.

Tests

bundle/dms/record shows the marker in both the local and the uploaded state; bundle/dms/existing-state covers both sides of the guard and the version-2-vs-3 contrast; new bundle/dms/requires-recording covers the refusal when recording is turned off. Two tests that seed a state file now run with recording on; the ones that cannot are opted out with their reasons recorded. Full suite green apart from bundle/templates/lakeflow-integrations, which fails identically on this PR's base (no network for a pypi download).

This PR was written by Claude Code.

@shreyas-goenka
shreyas-goenka force-pushed the isaac/dms-dual-state branch 2 times, most recently from 7047d22 to 1fd16ac Compare September 2, 2026 02:03
@shreyas-goenka
shreyas-goenka force-pushed the isaac/pr6052-fixes branch 2 times, most recently from a5b1a71 to 58257c8 Compare September 2, 2026 02:30
@shreyas-goenka
shreyas-goenka force-pushed the isaac/dms-dual-state branch 5 times, most recently from cee981f to f7d92ee Compare September 2, 2026 13:03
@shreyas-goenka
shreyas-goenka force-pushed the isaac/dms-dual-state branch 3 times, most recently from 855650e to 0af021f Compare September 2, 2026 14:16
@shreyas-goenka
shreyas-goenka force-pushed the isaac/pr6052-fixes branch 2 times, most recently from 26ad86b to c98c8e1 Compare September 2, 2026 23:39
@shreyas-goenka
shreyas-goenka marked this pull request as ready for review September 2, 2026 23:50
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Approval status: pending

/acceptance/bundle/ - needs approval

30 files changed
Suggested: @denik
Also eligible: @janniklasrose, @pietern, @andrewnester, @lennartkats-db, @anton-107

/bundle/ - needs approval

4 files changed
Suggested: @denik
Also eligible: @janniklasrose, @pietern, @andrewnester, @lennartkats-db, @anton-107

/cmd/bundle/ - needs approval

Files: cmd/bundle/generate/dashboard.go, cmd/bundle/generate/genie_space.go, cmd/bundle/utils/process.go
Suggested: @denik
Also eligible: @janniklasrose, @pietern, @andrewnester, @lennartkats-db, @anton-107

General files (require maintainer)

5 files changed
Based on git history:

  • @denik -- recent work in cmd/bundle/utils/, bundle/direct/dstate/, acceptance/bin/

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

@shreyas-goenka
shreyas-goenka force-pushed the isaac/pr6052-fixes branch 2 times, most recently from bb7adc4 to 84f6960 Compare September 3, 2026 00:14
@shreyas-goenka shreyas-goenka reopened this Sep 3, 2026
shreyas-goenka and others added 4 commits September 3, 2026 00:30
Recording made the service the source of truth for resource state, but the
state file was still written on every deploy. Mark it as depending on the
recording feature so an unaware CLI refuses it instead of deploying over a
deployment it would leave the service behind on, and use the marker rather
than "has any resources" to decide whether an existing state can be recorded.

While recording, the file's resource set is never read back: the service is
asked on every open, and holding no deployment means the resources are created
again. Turning recording off for a marked deployment is refused. The WAL needs
no special handling - whatever a replay rebuilds is replaced by what the
service holds.

Two tests that seed a state file now run with recording on; the ones that
cannot are opted out with their real reasons recorded.

Co-authored-by: Isaac <no-reply@databricks.com>
WIP. The id helpers read the direct-engine state file, which is the wrong source
while a bundle records deployment history: the service owns the resource set. Add
dms_resources.get_resources, which resolves the deployment from the workspace node
the service registers and lists its resources, and have read_id, replace_ids and
read_state use it when recording is on.

Not ready: resolving the remote state path runs "bundle validate", which fails for
tests that deploy with --var or other flags the helper cannot know (see
bundle/resources/secrets/*). And a hollow state file - the reason for reading from
the service at all - breaks "bundle generate", bind and configsync, which open the
state with no DMS client and need the resource ids from it.

Co-authored-by: Isaac <no-reply@databricks.com>
The service owns the resource set while recording, so writing the resources
into the state file as well left a second copy that nothing read. Write only
the header - above all the feature, which is what an unaware CLI refuses.

Every consumer of resource state therefore has to read it through the service.
bundle generate now opens the state with a client (new utils.DmsClientForState,
which also removes the construction process.go had inline), and bind/unbind
refuse a recorded state outright rather than silently binding against one that
looks empty. The acceptance helpers that read the state file - read_id,
read_state, replace_ids and print_state - go to the service too, so both DMS
variants of a test still assert one golden.

Co-authored-by: Isaac <no-reply@databricks.com>
The DMS tests now enable recording through the env var per command rather than
by editing databricks.yml, so the tests that turn it off had to do the same.
failed-delete drains the reads print_state.py makes against the service, which
would otherwise be left for the harness to diff.

Co-authored-by: Isaac <no-reply@databricks.com>
Wipes every local trace of a deployment and redeploys, so the only thing the
CLI can know comes from the service. Covers a dependency edge round-tripping
(depends_on is not something the service has a field for) and a resource type
that carries no deployment stamp, where an unchanged redeploy is only possible
if the recorded state really is the state.

Co-authored-by: Isaac <no-reply@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant