Support optional DoDelete in the direct engine - #6647
Draft
denik wants to merge 1 commit into
Draft
Conversation
Collaborator
Integration test reportCommit: ba38843
Top 10 slowest tests (at least 2 minutes):
|
denik
force-pushed
the
denik/optional-delete
branch
from
September 11, 2026 16:14
ba38843 to
e1a2487
Compare
Resources whose deletion has no backend effect (grants, permissions, secret_scope_acls) can now omit DoDelete instead of implementing a no-op. The engine treats a planned Delete for such a resource as a state-only cleanup (new PlanEntry.StateOnly flag): - adapter.go: DoDelete is optional; HasDoDelete() reports it. - Direct planner skips the remote read for the Delete (only used to detect already-gone resources, pointless when there is no delete call) and marks the entry StateOnly; apply removes the state entry without a delete call. - Shared rendering/counts (bundle plan text, deploy summary, destroy list, approval, CountActions) treat StateOnly as non-destructive/unchanged; the Delete stays in the JSON plan. - Terraform plan reader marks the same groups StateOnly so both engines' human output stays consistent (no change to what terraform applies). Co-authored-by: Isaac <no-reply@databricks.com>
denik
force-pushed
the
denik/optional-delete
branch
from
September 11, 2026 16:22
e1a2487 to
8d344c9
Compare
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Resources whose deletion has no backend effect —
grants,permissions,secret_scope_acls— previously implemented a no-opDoDelete(return nil). This makesDoDeleteoptional: omitting it declares the same intent, and lets the engine treat a plannedDeletefor such a resource as a state-only cleanup.Optimizations unlocked
Knowing a resource's delete is a no-op lets the engine do less and say less:
Deleteon such a resource the planner no longer callsDoRead— its only purpose was detecting an already-gone resource so apply could skip the delete call, which is pointless when there is no delete call. Fewer API round-trips, and no spurious read errors: e.g.destroy_without_mgmtpermsno longer emits a "does not have Manage permissions" warning.Gone, instead of running the full delete path (loading the persisted state and dispatching the no-opDoDelete/WaitAfterDelete).Resources:/Plan:tally only reflects real backend actions.prevent_destroybecomes inert for these resources — there is nothing to destroy.action: delete,state_only: true), so machine consumers still see the state transition.Technical changes
dresources/adapter.go:DoDeleteis now an optional bound caller;HasDoDelete()reports it; the wrapper is a nil-guarded no-op and its state-type validation is conditional. The three no-opDoDeletebodies are removed (rationale kept as comments).deployplan(plan.go,action.go): newPlanEntry.StateOnlyflag (serializedstate_only), a sibling toGone. UnlikeGone(a property of remote state) it is a property of the resource type.CountActionsleavesStateOnlydeletes out of the tally.bundle_plan.go: on aDeletefor a resource with noDoDelete, skip the remote read and setStateOnly.bundle_apply.go: aStateOnlydelete removes the state entry without running the delete path (shares the existingGonebranch).phases/deploy.go,phases/destroy.go,cmd/bundle/plan.go: excludeStateOnlydeletes from approval, summaries, and the textual plan.deploy/terraform/showplanfile.go: the terraform plan reader marks the samepermissions/grants/secret_aclsdeletesStateOnly, so both engines' human-facing output stays identical. This changes only what is shown, not what terraform applies.Tests
bundle/direct/...,bundle/deployplan/...,bundle/deploy/terraform/...,bundle/phases/...,cmd/bundle/....permissions/,secret_scopes/).This pull request and its description were written by Isaac.