Uh oh!
There was an error while loading. Please reload this page.
Allow configuring cascade deletion in DABs - #5846
Merged
andrewnester merged 16 commits intoAug 11, 2026
Merged
Conversation
Add a `cascade` field to the pipeline bundle resource that controls whether destroying a pipeline also deletes its datasets (materialized views, streaming tables, and views). When unset, the server default applies (cascade); set `cascade: false` to retain the datasets on destroy. The field is delete-time only: it is not part of the pipeline spec, so it is never sent on create/update. The direct engine persists it in state (via a PipelineState wrapper around CreatePipeline, mirroring the sql_warehouse lifecycle pattern) and reads it at delete time, force-sending cascade so an explicit false survives query-string omitempty (honored for query params as of databricks-sdk-go v0.152.0). A cascade-only change is a state-only update with no pipeline API call. cascade is classified input_only so it does not show remote drift. The terraform engine drops the field for now: the pinned provider has no such attribute yet (pending terraform-provider-databricks#5860), so terraform-engine support is a follow-up. Co-authored-by: Isaac
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
lennartkats-db
requested changes
Jul 9, 2026
| pipelines: | ||
| my_pipeline: | ||
| name: test-pipeline-cascade | ||
| cascade: false |
Contributor
There was a problem hiding this comment.
Could you make sure the default template also sets purge_on_destroy: false? With a very short comment to the side explaining it? Could be a separate PR.
ContributorAuthor
There was a problem hiding this comment.
ack will put this change in a follow-up PR.
Uh oh!
There was an error while loading. Please reload this page.
6 tasks
… message fix - Rename the pipeline config field cascade -> cascade_on_destroy, matching the Lakebase purge_on_delete precedent (per review discussion). - Add ValidateCascadeOnDestroy mutator: hard error when the field is used with the terraform engine, which does not support it yet. - Fix the destroy approval message so a pipeline with cascade_on_destroy: false is not described as deleting its STs/MVs, and mention the property otherwise. - Comment why ForceSendFields is needed in DoDelete. - Regenerate schema, refschema, and affected acceptance outputs. Co-authored-by: Isaac
ronaldz-db
commented
Jul 10, 2026
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| pipelines: | ||
| my_pipeline: | ||
| name: test-pipeline-cascade | ||
| cascade: false |
ContributorAuthor
There was a problem hiding this comment.
ack will put this change in a follow-up PR.
…cade # Conflicts: # NEXT_CHANGELOG.md
ronaldz-db added a commit
to ronaldz-db/terraform-provider-databricks
that referenced
this pull request
Jul 15, 2026
Align the pipeline destroy-time attribute name with the DABs config field (databricks/cli#5846), which settled on cascade_on_destroy. Co-authored-by: Isaac
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| This action will result in the deletion of the following Lakeflow Spark Declarative Pipelines along with the | ||
| Streaming Tables (STs) and Materialized Views (MVs) managed by them: | ||
| Streaming Tables (STs) and Materialized Views (MVs) managed by them. Set 'cascade_on_destroy: false' on a pipeline to retain datasets on pipeline deletion: |
Contributor
There was a problem hiding this comment.
Suggested change
| Streaming Tables (STs) and Materialized Views (MVs) managed by them. Set 'cascade_on_destroy: false' on a pipeline to retain datasets on pipeline deletion: | |
| Streaming Tables (STs) and Materialized Views (MVs) managed by them. Set 'cascade_on_destroy: false' on a pipeline to retain the Streaming Tables and Materialized Views on pipeline deletion: |
datasets seems like a vague term to use here? In the context of dashboards, for example, this just means a SQL query.
I could be wrong about this, so if this is standard for pipelines, feel free to keep it as is.
ContributorAuthor
There was a problem hiding this comment.
We generally use datasets in pipelines docs now, since pipeline-owned tables have been changing recently. Now, we also allow users to create Views in pipelines. In the near future, we plan to switch to creating regular delta tables as well.
Uh oh!
There was an error while loading. Please reload this page.
| type PipelineRemote struct { | ||
| pipelines.CreatePipeline | ||
| CascadeOnDestroy *bool `json:"cascade_on_destroy,omitempty"` |
Contributor
There was a problem hiding this comment.
Can be omitted from this struct because cascade on destroy will never be set in the remote. It'll never be returned as part of the get response
ContributorAuthor
There was a problem hiding this comment.
ah I was confused and thought that there was a strict requirement that PipelineRemote be a superset. But looking closer, I don't think there a benefit to defining for the remote version. Dropped it
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| deletePipelineWithCascadeMessage = `This action will result in the deletion of the following Lakeflow Spark Declarative Pipelines along with the | ||
| Streaming Tables (STs) and Materialized Views (MVs) managed by them. Set 'cascade_on_destroy: false' on a pipeline to retain datasets on pipeline deletion:` | ||
| deletePipelineNoCascadeMessage = `This action will result in the deletion of the following Lakeflow Spark Declarative Pipelines. |
Uh oh!
There was an error while loading. Please reload this page.
Drop the files.watcherExclude block that was unrelated to the cascade_on_destroy feature, per review feedback. Co-authored-by: Isaac
- Add docs URL to the terraform-engine validation error so users can learn about direct deployment mode. - Omit CascadeOnDestroy from PipelineRemote: it is an input-only, delete-time setting the GET response never returns, so drift is suppressed automatically (missing_in_remote). Matches the postgres input-only field pattern. - Propagate errors from pipelineDeletionCascades instead of silently defaulting, and add unit coverage. Co-authored-by: Isaac
| v, err := dyn.GetByPath(b.Config.Value(), path) | ||
| if err != nil { | ||
| return true, nil |
Contributor
There was a problem hiding this comment.
This embeds a client side default here. Treating unset as true. Won't this get out of date once the server side default changes?
Contributor
There was a problem hiding this comment.
I guess there's no good way to handle this scenario? It does mean we'll eventually end up showing wrong messages to people.
Contributor
There was a problem hiding this comment.
Can you add a to-do message here to change the value once the server side default flips?
ContributorAuthor
There was a problem hiding this comment.
Yep, added a note here. Unfortunately, there may be a brief period where the DABs default and server-side default are different. But the DABs documentation describes the actual encoded behavior, so we won't be showing the wrong messages
shreyas-goenka
approved these changes
Jul 16, 2026
| v, err := dyn.GetByPath(b.Config.Value(), path) | ||
| if err != nil { | ||
| return true, nil |
Contributor
There was a problem hiding this comment.
Can you add a to-do message here to change the value once the server side default flips?
lennartkats-db
approved these changes
Jul 16, 2026
shreyas-goenka
previously requested changes
Jul 16, 2026
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| pipelines: | ||
| my_pipeline: | ||
| name: test-pipeline-cascade | ||
| cascade_on_destroy: false |
Contributor
There was a problem hiding this comment.
Can you please add a test where we start we set cascade on destroy to false, we do a deployment, and then we unset this value, deploy again, and then destroy. I suspect that if we unset this That does not take hold and we continue to use false as the value for the bundle deployment. This was flagged by GPT during a code review:
> Unsetting cascade_on_destroy does not persist the new state. For false → unset, the planner detects the local diff (Old=false, New=nil), but because the remote omits this field, Remote=nil. Change
> classification therefore marks it skip: remote_already_set, and skipped resources do not save their new state. A later destroy still reads false from persisted state and sends cascade=false instead of
> using the server default.
>
> Could we force local changes to this client-only field to produce a state-only Update—perhaps through OverrideChangeDesc—and add regression coverage for: deploy with false → remove field → deploy →
> destroy?
Contributor
There was a problem hiding this comment.
The recommendation of adding override change desk here sounds like a reasonable solution to me:
The closest prior art is Lakebase’s purge_on_delete in /tmp/isaac-review-cli/bundle/direct/dresources/postgres_project.go:148:
- It is absent remotely.
- DoUpdate removes it from the API update mask.
- The framework still saves state when the resource action is Update.
However, purge_on_delete is a plain bool; unset and false have identical behavior. Its acceptance test even shows that removing the explicit false leaves false persisted. That is harmless there, but not
for cascade_on_destroy, where unset means the server default (true) and therefore differs from false.
The intended generic behavior is documented by ReasonMissingInRemote: suppress remote absence only “when there is no local change.” The current classifier violates that for false → nil because earlier
remote_already_set/allEmpty checks win.
Two fixes:
- Minimal: add ResourcePipeline.OverrideChangeDesc that forces cascade_on_destroy to Update whenever Old != New. DoUpdate already makes this state-only.
- General: change classification so locally changed fields absent from RemoteType bypass both remote_already_set and allEmpty. This is cleaner but affects all input-only fields and needs broader tests.
For this PR, I’d recommend the resource-specific override plus the regression test.
WDYT @denik
Contributor
There was a problem hiding this comment.
I think you'll need a snippet like:
func (*ResourcePipeline) OverrideChangeDesc(
_ context.Context,
path *structpath.PathNode,
change *ChangeDesc,
_ *PipelineRemote,
) error {
if path.String() == "cascade_on_destroy" &&
!structdiff.IsEqual(change.Old, change.New) {
change.Action = deployplan.Update
change.Reason = deployplan.ReasonCustom
}
return nil
}
ContributorAuthor
There was a problem hiding this comment.
oh interesting, thanks for the guidance. Added the override and additional test cases
Collaborator
Integration test reportCommit: 015b52e
8 interesting tests: 4 RECOVERED, 4 SKIP
Top 43 slowest tests (at least 2 minutes):
|
denik
reviewed
Jul 23, 2026
Uh oh!
There was an error while loading. Please reload this page.
cascade_on_destroy is absent from PipelineRemote (it is a delete-time-only, input-only field the GET never returns), so refschema should tag it INPUT+STATE, not ALL. Regenerate the golden to reflect the removal from the remote type. Co-authored-by: Isaac
denik
reviewed
Jul 29, 2026
| // Remote value is always nil. We add this override to force a state-only update. | ||
| func (*ResourcePipeline) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, change *ChangeDesc, _ *PipelineRemote) error { | ||
| if path.String() == "cascade_on_destroy" && !structdiff.IsEqual(change.Old, change.New) { | ||
| change.Action = deployplan.Update |
Contributor
There was a problem hiding this comment.
This is already the default behaviour, why do we need to encode it here?
ContributorAuthor
There was a problem hiding this comment.
This block is the problem:
if structdiff.IsEqual(ch.Remote, ch.New) {
ch.Action = deployplan.Skip
ch.Reason = deployplan.ReasonRemoteAlreadySet
} Since this field is not in the remote, unsetting the field previously set to false will result in old=false, new=nil, Remote=nil, which falls into the structdiff.IsEqual(ch.Remote, ch.New) case and results in deployplan.Skip. The state version will not be updated due ot this.
Adjusting resources.yml categories doesn't help since the structdiff.IsEqual(ch.Remote, ch.New) check runs first and short-circuits before we consider anything else.
Contributor
There was a problem hiding this comment.
I see. this is a bug in the planner, we should not trigger this block for your field because it's not even in remote. I'll think about this fix but for now we can proceed with your workaround.
Could you add your comment to OverrideChangeDesc impl?
ContributorAuthor
There was a problem hiding this comment.
👍 updated the comment with more details
Contributor
There was a problem hiding this comment.
I'm making a change that fixes remote_already_set guard: #6112
With it, you should not need OverrideChangeDesc, could you try rebasing on top of that and removing it?
ContributorAuthor
There was a problem hiding this comment.
I think there is still a problem here, where allEmpty incorrectly treats boolean fields with value "false" as empty.
I will keep in the OverrideChangeDesc for now to close out this PR
denik
approved these changes
Jul 29, 2026
Explain why the override is needed: unsetting the field yields old=false/new=nil/remote=nil, which the classifier's IsEqual(remote,new) check skips as remote_already_set even though the field is absent from the remote, so state is never re-saved. Co-authored-by: Isaac
… in dabs map The classifier's allEmpty branch treats a false boolean as empty, so a false<->nil cascade_on_destroy edit is skipped and never persists to state. Reword the OverrideChangeDesc comment to describe that mechanism instead of the pre-databricks#6112 "remote already set" framing. Regenerate the terraform<->dabs field map to register cascade_on_destroy as a dabs-only pipeline field. Co-authored-by: Isaac
Contributor
An authorized user can trigger integration tests manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 11, 2026
radakam added a commit
that referenced
this pull request
Aug 11, 2026
## Summary Regen leftovers from #5846 that keep `main` red: - `validate-generated`: add `cascade_on_destroy` to the PyDABs pipeline model - `task test`: refresh `config-remote-sync/split/positional` destroy-prompt golden
eng-dev-ecosystem-bot
commented
Aug 11, 2026
Collaborator
Integration test reportCommit: 90fe615
552 interesting tests: 475 MISS, 71 FAIL, 4 RECOVERED, 2 SKIP
Top 50 slowest tests (at least 2 minutes):
|
yolocs pushed a commit
to yolocs/dbcli
that referenced
this pull request
Aug 11, 2026
…cks#6232) ## Summary Drop stale `Local = true` left by databricks#5846 after databricks#6196 removed the field from `TestConfig`. Unblocks `task test-pipelines` on main.
deco-sdk-taggingBot
added a commit
that referenced
this pull request
Aug 12, 2026
## Release v1.12.0 ### CLI * `databricks aitools install` now supports Gemini CLI, installing Databricks agent skills into its skills directory. * `databricks aitools install` now supports Pi, installing Databricks agent skills into its skills directory. * A locally built CLI (`go build`, without release flags) now reports the next release version with a `-dev` prerelease, e.g. `1.12.0-dev+abcdef123456`, instead of `0.0.0-dev+abcdef123456`. The old string sorted below every published release even though a local build is newer than the latest release; the new one sorts above the latest release and below the release it will become, matching what goreleaser already produces for snapshot builds. * Added the `databricks environments setup-local` command, which provisions (or updates) a local Python environment matched to a Databricks compute target. It resolves the target to an environment key, fetches the pinned Python version, databricks-connect version, and dependency constraints published for that key, then provisions a matched `.venv` with uv. ### Bundles * Added a `cascade_on_destroy` field to the pipeline resource to control whether destroying a pipeline also deletes its datasets (MVs, STs, Views). When unset, the server default applies; set `cascade_on_destroy: false` to retain the datasets on destroy. Supported with the direct deployment engine ([#5846](#5846)). * Fix `bundle.deployment.lock.force` being ignored. The `--force-lock` flag's default value overwrote the value configured in `databricks.yml`, so setting the field had no effect and a stale deployment lock could only be overridden with the flag. ([#6188](#6188)) * direct: experimental `job_runs` now sends a CLI-managed idempotency token on every run-now, so an SDK retry after a lost response returns the same run. Configured `idempotency_token` values are rejected. * direct: the experimental `job_runs` resource now waits for the triggered run to finish, so other resources can reference its outcome (e.g. `${resources.job_runs.nightly.state.result_state}`). A run that does not succeed fails the deploy, naming the failed task, and is run again on the next deploy. If a deploy is interrupted while waiting, the next one resumes waiting on the same run. * direct: Fixed model serving `telemetry_config` drift and applied planned telemetry updates. Unsupported endpoint types now fail when telemetry is applied; create may still succeed because it drops the field ([#6106](#6106)). * The `cli_version` field in the direct engine's deployment state (`resources.json`) now records the CLI version that last wrote the state. Previously it kept the version of the CLI that first created the state. * Add support for UC secrets resource ([#5861](#5861)) ### Dependency Updates * Bump `github.com/databricks/databricks-sdk-go` from v0.166.0 to v0.169.0. * Bump Terraform provider from v1.124.0 to v1.126.0 ([#6250](#6250)).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Add a
cascade_on_destoryfield to the pipeline bundle resource that controls whether destroying a pipeline also cascades to its datasets. When unset, we maintain the existing default of true.This change applies to the direct engine. We have a separate change on the Terraform side: databricks/terraform-provider-databricks#5860
Why
Previously, pipeline deletion would automatically cascade to its tables. After user feedback, we decided to make this configurable in the DeletePipeline API. This extends support to DABs as well, which was another common user ask.
Tests
Unit tests