Skip to content

Commit 280e218

Browse files
Fix error propogation in the bundle generate dashboard command (#3354)
## Changes Fixes a regression introduced in #3175, where the errors in these functions would be dropped instead of being propagated. I noticed this regression while updating the CLI dashboard. This also highlights the downside returning diagnostics and using `logdiag` to log it later, outside mutators, since a linter would have caught it otherwise. ## Tests Added regression test. Also manually: Before: ``` ➜ cli_telemetry_dashboard git:(main) databricks bundle generate dashboard --resource cli_telemetry -p logfood ➜ cli_telemetry_dashboard git:(main) git status On branch main Your branch is up to date with 'origin/main'. ``` After: ``` ➜ cli_telemetry_dashboard git:(main) cli bundle generate dashboard --resource cli_telemetry -p logfood Error: src/cli_telemetry.lvdash.json already exists. Use --force to overwrite ``` Ideally, we would not use logdiag here and instead refactor these and other functions like these to return errors. It's out of scope for me for now.
1 parent 6049e8a commit 280e218

File tree

7 files changed

+108
-11
lines changed

7 files changed

+108
-11
lines changed
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"uiSettings": {
3+
"theme": {
4+
"widgetHeaderAlignment": "ALIGNMENT_UNSPECIFIED"
5+
}
6+
}
7+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
bundle:
2+
name: dashboard-replace-$UNIQUE_NAME
3+
4+
resources:
5+
dashboards:
6+
already_exists:
7+
file_path: ./dashboard.lvdash.json
8+
display_name: Already Exists
9+
warehouse_id: $TEST_DEFAULT_WAREHOUSE_ID
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Local = false
2+
Cloud = true
3+
4+
[EnvMatrix]
5+
DATABRICKS_CLI_DEPLOYMENT = ["terraform"]
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
2+
>>> [CLI] bundle deploy
3+
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/dashboard-replace-[UNIQUE_NAME]/default/files...
4+
Deploying resources...
5+
Updating deployment state...
6+
Deployment complete!
7+
8+
=== Get current dashboard
9+
"{\n \"uiSettings\": {\n \"theme\": {\n \"widgetHeaderAlignment\": \"ALIGNMENT_UNSPECIFIED\"\n }\n }\n}\n"
10+
11+
=== Update dashboard
12+
"{}\n"
13+
14+
>>> [CLI] bundle generate dashboard --resource already_exists
15+
Error: dashboard.lvdash.json already exists. Use --force to overwrite
16+
17+
18+
Exit code: 1
19+
20+
>>> cat dashboard.lvdash.json
21+
{
22+
"uiSettings": {
23+
"theme": {
24+
"widgetHeaderAlignment": "ALIGNMENT_UNSPECIFIED"
25+
}
26+
}
27+
}
28+
29+
>>> [CLI] bundle generate dashboard --resource already_exists --force
30+
Writing dashboard to "dashboard.lvdash.json"
31+
32+
>>> cat dashboard.lvdash.json
33+
{}
34+
35+
>>> [CLI] bundle destroy --auto-approve
36+
The following resources will be deleted:
37+
delete dashboard already_exists
38+
39+
All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/dashboard-replace-[UNIQUE_NAME]/default
40+
41+
Deleting files...
42+
Destroy complete!
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
envsubst < databricks.yml > databricks.yml.subst
2+
mv databricks.yml.subst databricks.yml
3+
4+
# Deploy bundle once to create the dashboard.
5+
trace $CLI bundle deploy
6+
7+
cleanup() {
8+
trace $CLI bundle destroy --auto-approve
9+
}
10+
trap cleanup EXIT
11+
12+
dashboard_id=$($CLI bundle summary -o json | jq .resources.dashboards.already_exists.id -r)
13+
14+
title "Get current dashboard\n"
15+
$CLI lakeview get $dashboard_id | jq .serialized_dashboard
16+
17+
title "Update dashboard\n"
18+
$CLI lakeview update $dashboard_id --display-name "Updated dashboard" | jq .serialized_dashboard
19+
20+
# This should error.
21+
errcode trace $CLI bundle generate dashboard --resource already_exists
22+
23+
trace cat dashboard.lvdash.json
24+
25+
# this should successfully update the dashboard.
26+
trace $CLI bundle generate dashboard --resource already_exists --force
27+
28+
trace cat dashboard.lvdash.json
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Cloud = true
2+
Local = false

cmd/bundle/generate/dashboard.go

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -238,17 +238,19 @@ func (d *dashboard) saveConfiguration(ctx context.Context, b *bundle.Bundle, das
238238
return nil
239239
}
240240

241-
func waitForChanges(ctx context.Context, w *databricks.WorkspaceClient, dashboard *dashboards.Dashboard) diag.Diagnostics {
241+
func waitForChanges(ctx context.Context, w *databricks.WorkspaceClient, dashboard *dashboards.Dashboard) {
242242
// Compute [time.Time] for the most recent update.
243243
tref, err := time.Parse(time.RFC3339, dashboard.UpdateTime)
244244
if err != nil {
245-
return diag.FromErr(err)
245+
logdiag.LogError(ctx, err)
246+
return
246247
}
247248

248249
for {
249250
obj, err := w.Workspace.GetStatusByPath(ctx, dashboard.Path)
250251
if err != nil {
251-
return diag.FromErr(err)
252+
logdiag.LogError(ctx, err)
253+
return
252254
}
253255

254256
// Compute [time.Time] from timestamp in millis since epoch.
@@ -259,18 +261,18 @@ func waitForChanges(ctx context.Context, w *databricks.WorkspaceClient, dashboar
259261

260262
time.Sleep(1 * time.Second)
261263
}
262-
263-
return nil
264264
}
265265

266-
func (d *dashboard) updateDashboardForResource(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
266+
func (d *dashboard) updateDashboardForResource(ctx context.Context, b *bundle.Bundle) {
267267
resource, ok := b.Config.Resources.Dashboards[d.resource]
268268
if !ok {
269-
return diag.Errorf("dashboard resource %q is not defined", d.resource)
269+
logdiag.LogError(ctx, fmt.Errorf("dashboard resource %q is not defined", d.resource))
270+
return
270271
}
271272

272273
if resource.FilePath == "" {
273-
return diag.Errorf("dashboard resource %q has no file path defined", d.resource)
274+
logdiag.LogError(ctx, fmt.Errorf("dashboard resource %q has no file path defined", d.resource))
275+
return
274276
}
275277

276278
// Resolve the dashboard ID from the resource.
@@ -286,19 +288,21 @@ func (d *dashboard) updateDashboardForResource(ctx context.Context, b *bundle.Bu
286288
for {
287289
dashboard, err := w.Lakeview.GetByDashboardId(ctx, dashboardID)
288290
if err != nil {
289-
return diag.FromErr(err)
291+
logdiag.LogError(ctx, err)
292+
return
290293
}
291294

292295
if etag != dashboard.Etag {
293296
err = d.saveSerializedDashboard(ctx, b, dashboard, dashboardPath)
294297
if err != nil {
295-
return diag.FromErr(err)
298+
logdiag.LogError(ctx, err)
299+
return
296300
}
297301
}
298302

299303
// Abort if we are not watching for changes.
300304
if !d.watch {
301-
return nil
305+
return
302306
}
303307

304308
// Update the etag for the next iteration.

0 commit comments

Comments
 (0)