Skip to content

Commit bb3c2e3

Browse files
committed
direct: Engine dedup+logging; dashboard saves draft state as Published=false
Engine.SaveState: - Accepts resourceKey for logging: "SaveState: resources.X id=Y N bytes: {...}" - Skips WAL write if state is unchanged (structdiff.IsEqual), logging a skip message. - Records the last saved value in e.lastSaved for subsequent comparisons. Dashboard DoCreate: - Saves intermediate state with Published=false (the actual draft state) instead of the user's Published=true. This ensures the planner sees a real diff (false→true) on the next deploy if publish is interrupted, rather than treating the resource as up-to-date and silently skipping the publish. publish-failure-retry acceptance test: - Adds `bundle plan -o json` and extracts the published change to verify old=false, new=true in the plan after a failed publish. README.md: update stale SetID+SaveState reference to current SaveState(ctx, id, state) API. Co-authored-by: Denis Bilenko
1 parent 572dc07 commit bb3c2e3

7 files changed

Lines changed: 52 additions & 17 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"action": "update",
3+
"old": false,
4+
"new": true,
5+
"remote": false
6+
}

acceptance/bundle/resources/dashboards/publish-failure-retry/output.txt

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,15 @@ Resources:
2525
Name: my dashboard
2626
URL: [DATABRICKS_URL]/dashboardsv3/[DASHBOARD_ID]/published?[WSPARAM]=[NUMID]
2727

28-
>>> [CLI] bundle plan
29-
update dashboards.dashboard1
28+
>>> [CLI] bundle plan -o json
3029

31-
Plan: 0 to add, 1 to change, 0 to delete, 0 unchanged
30+
>>> cat out.plan_published_change.json
31+
{
32+
"action": "update",
33+
"old": false,
34+
"new": true,
35+
"remote": false
36+
}
3237

3338
>>> [CLI] bundle deploy
3439
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/publish-failure-retry/default/files...

acceptance/bundle/resources/dashboards/publish-failure-retry/script

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@ errcode trace $CLI bundle deploy
2020
# Dashboard should be in state (tracked) despite the publish failure.
2121
trace $CLI bundle summary
2222

23-
# Plan should show that publishing is still needed.
24-
trace $CLI bundle plan
23+
# Plan should show published: false (saved state) -> true (desired).
24+
# Capture the Changes entry for 'published' to verify the diff direction.
25+
trace $CLI bundle plan -o json | jq '.plan["resources.dashboards.dashboard1"].changes.published' > out.plan_published_change.json
26+
trace cat out.plan_published_change.json
2527

2628
# Discard first-deploy requests so the output only contains second-deploy
2729
# calls, making it easy to confirm no CREATE was issued.

bundle/direct/apply.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func (d *DeploymentUnit) Deploy(ctx context.Context, db *dstate.DeploymentState,
5151
}
5252

5353
func (d *DeploymentUnit) Create(ctx context.Context, db *dstate.DeploymentState, newState any) error {
54-
engine := dresources.NewEngine(d.Adapter.StateType(), func(id string, x any) error {
54+
engine := dresources.NewEngine(d.ResourceKey, d.Adapter.StateType(), func(id string, x any) error {
5555
return db.SaveState(d.ResourceKey, id, x, d.DependsOn)
5656
})
5757

@@ -126,7 +126,7 @@ func (d *DeploymentUnit) Update(ctx context.Context, db *dstate.DeploymentState,
126126
return fmt.Errorf("internal error: DoUpdate not implemented for resource %s", d.ResourceKey)
127127
}
128128

129-
engine := dresources.NewEngine(d.Adapter.StateType(), func(_ string, x any) error {
129+
engine := dresources.NewEngine(d.ResourceKey, d.Adapter.StateType(), func(_ string, x any) error {
130130
return db.SaveState(d.ResourceKey, id, x, d.DependsOn)
131131
})
132132
remoteState, err := retryOnTransient(ctx, func() (any, error) {

bundle/direct/dresources/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ If a resource has fields that must not be sent in updates (deploy-only, lifecycl
3636

3737
## Async APIs
3838

39-
For resources whose create or update is asynchronous, poll inline inside `DoCreate`/`DoUpdate` after the initial API call. To prevent orphaning if deployment is interrupted during a long wait, call `engine.SetID(id)` then `engine.SaveState(config)` immediately after the resource is created and before any waiting. The framework provides a `*Engine` as the second argument to both methods.
39+
For resources whose create or update is asynchronous, poll inline inside `DoCreate`/`DoUpdate` after the initial API call. To prevent orphaning if deployment is interrupted during a long wait, call `engine.SaveState(ctx, id, config)` immediately after the resource is created and before any waiting. The framework provides a `*Engine` as the second argument to both methods.
4040

4141
## Slice ordering: KeyedSlices
4242

bundle/direct/dresources/dashboard.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -298,10 +298,14 @@ func (r *ResourceDashboard) DoCreate(ctx context.Context, engine *Engine, config
298298

299299
// Persist the etag in state.
300300
config.Etag = createResp.Etag
301-
// Save state before publishing so an interrupted publish leaves a tracked
302-
// draft rather than an orphan. The next deploy finds the draft in state and
303-
// re-publishes via DoUpdate without recreating the dashboard.
301+
// Save state with Published=false: the dashboard exists as a draft; publish
302+
// has not succeeded yet. Using Published=false ensures the planner sees a
303+
// real diff (false→true) if publish is interrupted, triggering a DoUpdate
304+
// on the next deploy instead of silently treating the resource as up-to-date.
305+
savedPublished := config.Published
306+
config.Published = false
304307
engine.SaveState(ctx, createResp.DashboardId, config)
308+
config.Published = savedPublished
305309

306310
var publishResp *dashboards.PublishedDashboard
307311
// Note, today config.Published is always true (we do not have this field in input config).

bundle/direct/dresources/engine.go

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,35 +2,41 @@ package dresources
22

33
import (
44
"context"
5+
"encoding/json"
56
"fmt"
67
"reflect"
78

9+
"github.com/databricks/cli/libs/log"
810
"github.com/databricks/cli/libs/logdiag"
11+
"github.com/databricks/cli/libs/structs/structdiff"
912
)
1013

1114
// Engine provides state persistence to resource implementations.
1215
// Pass it to DoCreate or DoUpdate to save intermediate state before long-running
1316
// wait operations, so the resource is not orphaned if deployment is interrupted.
1417
type Engine struct {
15-
id string
16-
stateType reflect.Type
17-
saveFunc func(id string, x any) error
18+
resourceKey string
19+
id string
20+
stateType reflect.Type
21+
saveFunc func(id string, x any) error
22+
lastSaved any
1823
}
1924

2025
// NewEngine creates an Engine with the given state type and save function.
2126
// The framework calls this before invoking DoCreate or DoUpdate.
22-
func NewEngine(stateType reflect.Type, saveFunc func(id string, x any) error) *Engine {
23-
return &Engine{id: "", stateType: stateType, saveFunc: saveFunc}
27+
func NewEngine(resourceKey string, stateType reflect.Type, saveFunc func(id string, x any) error) *Engine {
28+
return &Engine{resourceKey: resourceKey, id: "", stateType: stateType, saveFunc: saveFunc, lastSaved: nil}
2429
}
2530

2631
// NewNopEngine creates an Engine that discards all saves. Use in tests.
2732
func NewNopEngine(stateType reflect.Type) *Engine {
28-
return NewEngine(stateType, func(_ string, _ any) error { return nil })
33+
return NewEngine("", stateType, func(_ string, _ any) error { return nil })
2934
}
3035

3136
// SaveState saves the resource state. id must be the resource's identifier; on
3237
// the first call it is recorded, and subsequent calls panic if a different id is
3338
// passed. x must be a pointer to the same struct type as the resource's state.
39+
// If the state is identical to what was last saved, the write is skipped.
3440
// Failures to persist state are logged but do not abort the deployment — the
3541
// resource already exists and aborting would not undo its creation.
3642
func (e *Engine) SaveState(ctx context.Context, id string, x any) {
@@ -43,7 +49,19 @@ func (e *Engine) SaveState(ctx context.Context, id string, x any) {
4349
if xt != e.stateType {
4450
panic(fmt.Sprintf("SaveState: type mismatch: expected %v, got %v", e.stateType, xt))
4551
}
52+
if e.lastSaved != nil && structdiff.IsEqual(e.lastSaved, x) {
53+
log.Debugf(ctx, "SaveState: %s id=%s: skipping, state unchanged", e.resourceKey, id)
54+
return
55+
}
56+
b, _ := json.Marshal(x)
57+
preview := string(b)
58+
if len(preview) > 100 {
59+
preview = preview[:100]
60+
}
61+
log.Debugf(ctx, "SaveState: %s id=%s %d bytes: %s", e.resourceKey, id, len(b), preview)
4662
if err := e.saveFunc(e.id, x); err != nil {
4763
logdiag.LogError(ctx, err)
64+
return
4865
}
66+
e.lastSaved = x
4967
}

0 commit comments

Comments
 (0)