Specialize error when the path points to a legacy dashboard

cmd/bundle/generate/dashboard.go

@@ -3,6 +3,7 @@ package generate
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"os"
 	"path"
@@ -14,6 +15,7 @@ import (
 	"github.com/databricks/cli/bundle/config/generate"
 	"github.com/databricks/cli/bundle/deploy/terraform"
 	"github.com/databricks/cli/bundle/phases"
+	"github.com/databricks/cli/bundle/render"
 	"github.com/databricks/cli/cmd/root"
 	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/dyn"
@@ -26,32 +28,62 @@ import (
 	"gopkg.in/yaml.v3"
 )
 
-type dashboardLookup struct {
+type dashboard struct {
+	// Lookup flags for one-time generate.
 	dashboardPath string
 	dashboardId   string
+
+	// Lookup flag for existing bundle resource.
 	resource string
+
+	// Where to write the configuration and dashboard representation.
+	resourceDir  string
+	dashboardDir string
+
+	// Force overwrite of existing files.
+	force bool
+
+	// Watch for changes to the dashboard.
+	watch bool
+
+	// Relative path from the resource directory to the dashboard directory.
+	relativeDashboardDir string
 }
 
-func (d *dashboardLookup) resolveID(ctx context.Context, b *bundle.Bundle) (string, diag.Diagnostics) {
+func (d *dashboard) resolveID(ctx context.Context, b *bundle.Bundle) (string, diag.Diagnostics) {
 	switch {
 	case d.dashboardPath != "":
 		return d.resolveFromPath(ctx, b)
 	case d.dashboardId != "":
 		return d.resolveFromID(ctx, b)
-	case d.resource != "":
-		return d.resolveFromResource(ctx, b)
 	}
 
-	return "", diag.Errorf("expected one of --dashboard-path, --dashboard-id, or --resource")
+	return "", diag.Errorf("expected one of --dashboard-path, --dashboard-id")
 }
 
-func (d *dashboardLookup) resolveFromPath(ctx context.Context, b *bundle.Bundle) (string, diag.Diagnostics) {
+func (d *dashboard) resolveFromPath(ctx context.Context, b *bundle.Bundle) (string, diag.Diagnostics) {
 	w := b.WorkspaceClient()
 	obj, err := w.Workspace.GetStatusByPath(ctx, d.dashboardPath)
 	if err != nil {
 		if apierr.IsMissing(err) {
-			return "", diag.Errorf("dashboard at path %q not found", d.dashboardPath)
+			return "", diag.Errorf("dashboard %q not found", path.Base(d.dashboardPath))
 		}
+
+		// Emit a more descriptive error message for legacy dashboards.
+		if errors.Is(err, apierr.ErrBadRequest) && strings.HasPrefix(err.Error(), "dbsqlDashboard ") {
+			return "", diag.Diagnostics{
+				{
+					Severity: diag.Error,
+					Summary:  fmt.Sprintf("dashboard %q is a legacy dashboard", path.Base(d.dashboardPath)),
+					Detail: "" +
+						"Databricks Asset Bundles work exclusively with AI/BI dashboards.\n" +
+						"\n" +
+						"Instructions on how to convert a legacy dashboard to an AI/BI dashboard\n" +
+						"can be found at: https://docs.databricks.com/en/dashboards/clone-legacy-to-aibi.html.",
+				},
+			}
+		}
+
 		return "", diag.FromErr(err)
 	}
 
@@ -77,7 +109,7 @@ func (d *dashboardLookup) resolveFromPath(ctx context.Context, b *bundle.Bundle)
 	return obj.ResourceId, nil
 }
 
-func (d *dashboardLookup) resolveFromID(ctx context.Context, b *bundle.Bundle) (string, diag.Diagnostics) {
+func (d *dashboard) resolveFromID(ctx context.Context, b *bundle.Bundle) (string, diag.Diagnostics) {
 	w := b.WorkspaceClient()
 	obj, err := w.Lakeview.GetByDashboardId(ctx, d.dashboardId)
 	if err != nil {
@@ -90,31 +122,6 @@ func (d *dashboardLookup) resolveFromID(ctx context.Context, b *bundle.Bundle) (
 	return obj.DashboardId, nil
 }
 
-func (d *dashboardLookup) resolveFromResource(_ context.Context, b *bundle.Bundle) (string, diag.Diagnostics) {
-	resource, ok := b.Config.Resources.Dashboards[d.resource]
-	if !ok {
-		return "", diag.Errorf("dashboard resource %q is not defined", d.resource)
-	}
-
-	if resource.ID == "" {
-		return "", diag.Errorf("dashboard resource hasn't been deployed yet")
-	}
-
-	return resource.ID, nil
-}
-
-type dashboard struct {
-	dashboardLookup
-
-	resourceDir  string
-	dashboardDir string
-	force        bool
-	watch        bool
-
-	// Relative path from the resource directory to the dashboard directory.
-	relativeDashboardDir string
-}
-
 func remarshalJSON(data []byte) ([]byte, error) {
 	var tmp any
 	var err error
@@ -166,12 +173,7 @@ func (d *dashboard) saveSerializedDashboard(_ context.Context, b *bundle.Bundle,
 	return os.WriteFile(filename, data, 0644)
 }
 
-func (d *dashboard) saveConfiguration(ctx context.Context, b *bundle.Bundle, dashboard *dashboards.Dashboard) error {
-	key := d.dashboardLookup.resource
-	if key == "" {
-		key = textutil.NormalizeString(dashboard.DisplayName)
-	}
-
+func (d *dashboard) saveConfiguration(ctx context.Context, b *bundle.Bundle, dashboard *dashboards.Dashboard, key string) error {
 	// Save serialized dashboard definition to the dashboard directory.
 	dashboardBasename := fmt.Sprintf("%s.lvdash.json", key)
 	dashboardPath := filepath.Join(d.dashboardDir, dashboardBasename)
@@ -220,7 +222,7 @@ func (d *dashboard) saveConfiguration(ctx context.Context, b *bundle.Bundle, das
 	return nil
 }
 
-func (d *dashboard) runWatch(ctx context.Context, b *bundle.Bundle, dashboardID string) diag.Diagnostics {
+func (d *dashboard) generateForResource(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	resource, ok := b.Config.Resources.Dashboards[d.resource]
 	if !ok {
 		return diag.Errorf("dashboard resource %q is not defined", d.resource)
@@ -230,6 +232,9 @@ func (d *dashboard) runWatch(ctx context.Context, b *bundle.Bundle, dashboardID
 		return diag.Errorf("dashboard resource %q has no file path defined", d.resource)
 	}
 
+	// Resolve the dashboard ID from the resource.
+	dashboardID := resource.ID
+
 	// Overwrite the dashboard at the path referenced from the resource.
 	dashboardPath := resource.FilePath
 
@@ -249,6 +254,11 @@ func (d *dashboard) runWatch(ctx context.Context, b *bundle.Bundle, dashboardID
 			}
 		}
 
+		// Abort if we are not watching for changes.
+		if !d.watch {
+			return nil
+		}
+
 		// Update the etag for the next iteration.
 		etag = dashboard.Etag
 
@@ -277,14 +287,15 @@ func (d *dashboard) runWatch(ctx context.Context, b *bundle.Bundle, dashboardID
 	}
 }
 
-func (d *dashboard) runOnce(ctx context.Context, b *bundle.Bundle, dashboardID string) diag.Diagnostics {
+func (d *dashboard) generateForExisting(ctx context.Context, b *bundle.Bundle, dashboardID string) diag.Diagnostics {
 	w := b.WorkspaceClient()
 	dashboard, err := w.Lakeview.GetByDashboardId(ctx, dashboardID)
 	if err != nil {
 		return diag.FromErr(err)
 	}
 
-	err = d.saveConfiguration(ctx, b, dashboard)
+	key := textutil.NormalizeString(dashboard.DisplayName)
+	err = d.saveConfiguration(ctx, b, dashboard, key)
 	if err != nil {
 		return diag.FromErr(err)
 	}
@@ -311,6 +322,31 @@ func (d *dashboard) initialize(b *bundle.Bundle) diag.Diagnostics {
 	return nil
 }
 
+func (d *dashboard) runForResource(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
+	diags := bundle.Apply(ctx, b, bundle.Seq(
+		phases.Initialize(),
+		terraform.Interpolate(),
+		terraform.Write(),
+		terraform.StatePull(),
+		terraform.Load(),
+	))
+	if diags.HasError() {
+		return diags
+	}
+
+	return d.generateForResource(ctx, b)
+}
+
+func (d *dashboard) runForExisting(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
+	// Resolve the ID of the dashboard to generate configuration for.
+	dashboardID, diags := d.resolveID(ctx, b)
+	if diags.HasError() {
+		return diags
+	}
+
+	return d.generateForExisting(ctx, b, dashboardID)
+}
+
 func (d *dashboard) RunE(cmd *cobra.Command, args []string) error {
 	ctx := cmd.Context()
 	b, diags := root.MustConfigureBundle(cmd)
@@ -323,30 +359,23 @@ func (d *dashboard) RunE(cmd *cobra.Command, args []string) error {
 		return diags.Error()
 	}
 
-	diags = bundle.Apply(ctx, b, bundle.Seq(
-		phases.Initialize(),
-		terraform.Interpolate(),
-		terraform.Write(),
-		terraform.StatePull(),
-		terraform.Load(),
-	))
-	if diags.HasError() {
-		return diags.Error()
-	}
-
-	// Resolve the ID of the dashboard to generate configuration for.
-	dashboardID, diags := d.resolveID(ctx, b)
-	if diags.HasError() {
-		return diags.Error()
-	}
-
-	if d.watch {
-		diags = d.runWatch(ctx, b, dashboardID)
+	if d.resource != "" {
+		diags = d.runForResource(ctx, b)
 	} else {
-		diags = d.runOnce(ctx, b, dashboardID)
+		diags = d.runForExisting(ctx, b)
 	}
 
-	return diags.Error()
+	renderOpts := render.RenderOptions{RenderSummaryTable: false}
+	err := render.RenderDiagnostics(cmd.OutOrStdout(), b, diags, renderOpts)
+	if err != nil {
+		return fmt.Errorf("failed to render output: %w", err)
+	}
+
+	if diags.HasError() {
+		return root.ErrAlreadyPrinted
+	}
+
+	return nil
 }
 
 func NewGenerateDashboardCommand() *cobra.Command {
@@ -360,7 +389,7 @@ func NewGenerateDashboardCommand() *cobra.Command {
 	// Lookup flags.
 	cmd.Flags().StringVar(&d.dashboardPath, "existing-path", "", `workspace path of the dashboard to generate configuration for`)
 	cmd.Flags().StringVar(&d.dashboardId, "existing-id", "", `ID of the dashboard to generate configuration for`)
-	cmd.Flags().StringVar(&d.resource, "existing-resource", "", `resource key of dashboard to watch for changes`)
+	cmd.Flags().StringVar(&d.resource, "resource", "", `resource key of dashboard to watch for changes`)
 
 	// Output flags.
 	cmd.Flags().StringVarP(&d.resourceDir, "resource-dir", "d", "./resources", `directory to write the configuration to`)
@@ -370,15 +399,15 @@ func NewGenerateDashboardCommand() *cobra.Command {
 	cmd.MarkFlagsOneRequired(
 		"existing-path",
 		"existing-id",
-		"existing-resource",
+		"resource",
 	)
 
 	// Watch flags.
 	cmd.Flags().BoolVar(&d.watch, "watch", false, `watch for changes to the dashboard and update the configuration`)
 
 	// Make sure the watch flag is only used with the existing-resource flag.
-	cmd.MarkFlagsRequiredTogether("watch", "existing-resource")
-	cmd.MarkFlagsOneRequired()
+	cmd.MarkFlagsMutuallyExclusive("watch", "existing-path")
+	cmd.MarkFlagsMutuallyExclusive("watch", "existing-id")
 
 	cmd.RunE = d.RunE
 	return cmd

cmd/bundle/generate/dashboard_test.go

@@ -1 +1,43 @@
 package generate
+
+import (
+	"context"
+	"testing"
+
+	"github.com/databricks/cli/bundle"
+	"github.com/databricks/databricks-sdk-go/apierr"
+	"github.com/databricks/databricks-sdk-go/experimental/mocks"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDashboard_ErrorOnLegacyDashboard(t *testing.T) {
+	// Response to a GetStatus request on a path pointing to a legacy dashboard.
+	//
+	// < HTTP/2.0 400 Bad Request
+	// < {
+	// <   "error_code": "BAD_REQUEST",
+	// <   "message": "dbsqlDashboard is not user-facing."
+	// < }
+
+	d := dashboard{
+		dashboardPath: "/path/to/legacy dashboard",
+	}
+
+	m := mocks.NewMockWorkspaceClient(t)
+	w := m.GetMockWorkspaceAPI()
+	w.On("GetStatusByPath", mock.Anything, "/path/to/legacy dashboard").Return(nil, &apierr.APIError{
+		StatusCode: 400,
+		ErrorCode:  "BAD_REQUEST",
+		Message:    "dbsqlDashboard is not user-facing.",
+	})
+
+	ctx := context.Background()
+	b := &bundle.Bundle{}
+	b.SetWorkpaceClient(m.WorkspaceClient)
+
+	_, diags := d.resolveID(ctx, b)
+	require.Len(t, diags, 1)
+	assert.Equal(t, diags[0].Summary, "dashboard \"legacy dashboard\" is a legacy dashboard")
+}