Skip to content

Commit 72e62da

Browse files
committed
direct: save state before wait/publish in sql_warehouse DoCreate and dashboard DoUpdate
Two more cases where state could be persisted earlier to avoid orphaning or stale-etag conflicts: sql_warehouse DoCreate: the warehouse is created, then DoCreate polls for RUNNING (and may Stop it). If interrupted during the wait, the warehouse was orphaned. Now SaveState is called right after Create returns the id, before the wait. dashboard DoUpdate: Update() bumps the server-side etag, then publishDashboard() can fail. Mirror the DoCreate fix — save the new etag with Published=false before publishing. This keeps the etag in sync (a stale etag makes the next Update fail with a conflict) and records published=false so the planner re-publishes next deploy. Resolves the pre-existing TODO. Add publish-failure-retry-on-update acceptance test: deploy, then trigger an update with an injected publish failure, and verify the update issued a PATCH (no CREATE) and the plan records published old=false. Co-authored-by: Denis Bilenko
1 parent 51392cd commit 72e62da

8 files changed

Lines changed: 130 additions & 4 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"pages":[{"name":"test-page","displayName":"Test Dashboard"}]}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
bundle:
2+
name: publish-failure-retry-on-update
3+
4+
resources:
5+
dashboards:
6+
dashboard1:
7+
display_name: my dashboard
8+
warehouse_id: someid
9+
file_path: ./dashboard.lvdash.json

acceptance/bundle/resources/dashboards/publish-failure-retry-on-update/out.test.toml

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
2+
>>> [CLI] bundle deploy
3+
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/publish-failure-retry-on-update/default/files...
4+
Deploying resources...
5+
Updating deployment state...
6+
Deployment complete!
7+
8+
>>> [CLI] bundle deploy
9+
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/publish-failure-retry-on-update/default/files...
10+
Deploying resources...
11+
Error: cannot update resources.dashboards.dashboard1: updating id=[DASHBOARD1_ID]: Fault injected by test. (400 INJECTED)
12+
13+
Endpoint: POST [DATABRICKS_URL]/api/2.0/lakeview/dashboards/[DASHBOARD1_ID]/published
14+
HTTP Status: 400 Bad Request
15+
API error_code: INJECTED
16+
API message: Fault injected by test.
17+
18+
Updating deployment state...
19+
20+
Exit code: 1
21+
{
22+
"method": "PATCH",
23+
"path": "/api/2.0/lakeview/dashboards/[DASHBOARD1_ID]",
24+
"body": {
25+
"display_name": "my dashboard renamed",
26+
"parent_path": "/Workspace/Users/[USERNAME]/.bundle/publish-failure-retry-on-update/default/resources",
27+
"serialized_dashboard": "{\"pages\":[{\"name\":\"test-page\",\"displayName\":\"Test Dashboard\"}]}\n",
28+
"warehouse_id": "someid"
29+
}
30+
}
31+
{
32+
"method": "POST",
33+
"path": "/api/2.0/lakeview/dashboards/[DASHBOARD1_ID]/published",
34+
"body": {
35+
"embed_credentials": false,
36+
"warehouse_id": "someid"
37+
}
38+
}
39+
40+
>>> [CLI] bundle plan -o json
41+
{
42+
"action": "skip",
43+
"reason": "remote_already_set",
44+
"old": false,
45+
"new": true,
46+
"remote": true
47+
}
48+
49+
>>> [CLI] bundle destroy --auto-approve
50+
The following resources will be deleted:
51+
delete resources.dashboards.dashboard1
52+
53+
All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/publish-failure-retry-on-update/default
54+
55+
Deleting files...
56+
Destroy complete!
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
envsubst < databricks.yml.tmpl > databricks.yml
2+
3+
cleanup() {
4+
trace $CLI bundle destroy --auto-approve
5+
rm -f out.requests.txt
6+
}
7+
trap cleanup EXIT
8+
9+
# unset MSYS_NO_PATHCONV so MSYS2 converts the script path to a Windows path
10+
# when invoking the Python interpreter (required for fault.py to be found on Windows).
11+
unset MSYS_NO_PATHCONV
12+
13+
# First deploy succeeds fully: the dashboard is created and published.
14+
trace $CLI bundle deploy
15+
replace_ids.py
16+
rm out.requests.txt
17+
18+
# Inject a single publish failure for the update below.
19+
fault.py "POST /api/2.0/lakeview/dashboards/*" 400 0 1
20+
21+
# Change the dashboard to trigger an Update, which bumps the server-side etag.
22+
update_file.py databricks.yml "my dashboard" "my dashboard renamed"
23+
24+
# Deploy: Update (PATCH) succeeds and bumps the etag, but the publish (POST) fails.
25+
errcode trace $CLI bundle deploy
26+
27+
# The failed deploy issued a PATCH (update) and a POST (publish), but no CREATE
28+
# (POST /api/2.0/lakeview/dashboards): the existing dashboard was updated in place.
29+
print_requests.py //lakeview/dashboards
30+
31+
# Plan shows published old=false: state was saved with published=false (and the bumped
32+
# etag) before the failed publish, so the next deploy knows publishing is still pending.
33+
trace $CLI bundle plan -o json | jq '.plan["resources.dashboards.dashboard1"].changes.published'
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
Cloud = false
2+
Local = true
3+
RecordRequests = true
4+
5+
# Only run with the direct engine: the test verifies direct engine's SaveState
6+
# behavior during DoUpdate (new etag + published=false persisted before a failed publish).
7+
[EnvMatrix]
8+
DATABRICKS_BUNDLE_ENGINE = ["direct"]
9+
10+
# Dashboard published URLs use ?o= (local testserver) or ?w= (cloud) for the workspace/org ID.
11+
[[Repls]]
12+
Old = '\?[ow]=\d+'
13+
New = "?[WSPARAM]=[NUMID]"

bundle/direct/dresources/dashboard.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ func (r *ResourceDashboard) DoCreate(ctx context.Context, engine *Engine, config
319319
return createResp.DashboardId, responseToState(createResp, publishResp, dashboard.SerializedDashboard, config.Published), nil
320320
}
321321

322-
func (r *ResourceDashboard) DoUpdate(ctx context.Context, _ *Engine, id string, config *DashboardState, _ *PlanEntry) (*DashboardState, error) {
322+
func (r *ResourceDashboard) DoUpdate(ctx context.Context, engine *Engine, id string, config *DashboardState, _ *PlanEntry) (*DashboardState, error) {
323323
dashboard, err := prepareDashboardRequest(config)
324324
if err != nil {
325325
return nil, err
@@ -339,15 +339,21 @@ func (r *ResourceDashboard) DoUpdate(ctx context.Context, _ *Engine, id string,
339339
return nil, err
340340
}
341341

342-
// Persist the etag in state.
342+
// Persist the new etag with Published=false before publishing. Update() bumps the
343+
// etag on the server; if a subsequent publish fails, saving here keeps the etag in
344+
// sync (a stale etag would make the next Update fail with a conflict) and records
345+
// published=false so the planner re-publishes on the next deploy.
343346
config.Etag = updateResp.Etag
347+
savedPublished := config.Published
348+
config.Published = false
349+
engine.SaveState(ctx, id, config)
350+
config.Published = savedPublished
344351

345352
var publishResp *dashboards.PublishedDashboard
346353
// Note, today config.Published is always true (we do not have this field in input config).
347354
if config.Published {
348355
publishResp, err = r.publishDashboard(ctx, id, config)
349356
if err != nil {
350-
// TODO: store partial state with published=false?
351357
return nil, err
352358
}
353359
}

bundle/direct/dresources/sql_warehouse.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,17 @@ func (r *ResourceSqlWarehouse) DoRead(ctx context.Context, id string) (*SqlWareh
118118
}
119119

120120
// DoCreate creates the warehouse and returns its id.
121-
func (r *ResourceSqlWarehouse) DoCreate(ctx context.Context, _ *Engine, config *SqlWarehouseState) (string, *SqlWarehouseRemote, error) {
121+
func (r *ResourceSqlWarehouse) DoCreate(ctx context.Context, engine *Engine, config *SqlWarehouseState) (string, *SqlWarehouseRemote, error) {
122122
waiter, err := r.client.Warehouses.Create(ctx, config.CreateWarehouseRequest)
123123
if err != nil {
124124
return "", nil, err
125125
}
126126
id := waiter.Id
127127

128+
// Save state immediately after the warehouse is created so it is not orphaned
129+
// if the subsequent wait or stop is interrupted.
130+
engine.SaveState(ctx, id, config)
131+
128132
if config.Lifecycle == nil || config.Lifecycle.Started == nil {
129133
return id, nil, nil
130134
}

0 commit comments

Comments
 (0)