From 3461c66dc947615476f259ce5f61b4cd2f0ccb6d Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 3 Sep 2024 10:35:41 +0200 Subject: [PATCH 01/39] Add DABs support for AI/BI dashboards --- bundle/config/mutator/translate_paths.go | 15 +++ .../mutator/translate_paths_dashboards.go | 50 ++++++++ bundle/config/resources.go | 1 + bundle/config/resources/dashboard.go | 59 ++++++++++ bundle/deploy/terraform/convert.go | 10 ++ .../terraform/tfdyn/convert_dashboard.go | 55 +++++++++ .../terraform/tfdyn/convert_dashboard_test.go | 7 ++ bundle/permissions/mutator.go | 4 + internal/acc/workspace.go | 30 +++++ internal/test/dashboard_assumptions_test.go | 110 ++++++++++++++++++ 10 files changed, 341 insertions(+) create mode 100644 bundle/config/mutator/translate_paths_dashboards.go create mode 100644 bundle/config/resources/dashboard.go create mode 100644 bundle/deploy/terraform/tfdyn/convert_dashboard.go create mode 100644 bundle/deploy/terraform/tfdyn/convert_dashboard_test.go create mode 100644 internal/test/dashboard_assumptions_test.go diff --git a/bundle/config/mutator/translate_paths.go b/bundle/config/mutator/translate_paths.go index 5f22570e7f..82b0b3caa3 100644 --- a/bundle/config/mutator/translate_paths.go +++ b/bundle/config/mutator/translate_paths.go @@ -162,6 +162,20 @@ func (t *translateContext) translateNoOp(literal, localFullPath, localRelPath, r return localRelPath, nil } +func (t *translateContext) retainLocalAbsoluteFilePath(literal, localFullPath, localRelPath, remotePath string) (string, error) { + info, err := t.b.SyncRoot.Stat(localRelPath) + if errors.Is(err, fs.ErrNotExist) { + return "", fmt.Errorf("file %s not found", literal) + } + if err != nil { + return "", fmt.Errorf("unable to determine if %s is a file: %w", localFullPath, err) + } + if info.IsDir() { + return "", fmt.Errorf("expected %s to be a file but found a directory", literal) + } + return localFullPath, nil +} + func (t *translateContext) translateNoOpWithPrefix(literal, localFullPath, localRelPath, remotePath string) (string, error) { if !strings.HasPrefix(localRelPath, ".") { localRelPath = "." + string(filepath.Separator) + localRelPath @@ -215,6 +229,7 @@ func (m *translatePaths) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnos t.applyJobTranslations, t.applyPipelineTranslations, t.applyArtifactTranslations, + t.applyDashboardTranslations, } { v, err = fn(v) if err != nil { diff --git a/bundle/config/mutator/translate_paths_dashboards.go b/bundle/config/mutator/translate_paths_dashboards.go new file mode 100644 index 0000000000..341156163d --- /dev/null +++ b/bundle/config/mutator/translate_paths_dashboards.go @@ -0,0 +1,50 @@ +package mutator + +import ( + "fmt" + + "github.com/databricks/cli/libs/dyn" +) + +type dashboardRewritePattern struct { + pattern dyn.Pattern + fn rewriteFunc +} + +func (t *translateContext) dashboardRewritePatterns() []dashboardRewritePattern { + // Base pattern to match all dashboards. + base := dyn.NewPattern( + dyn.Key("resources"), + dyn.Key("dashboards"), + dyn.AnyKey(), + ) + + // Compile list of configuration paths to rewrite. + return []dashboardRewritePattern{ + { + base.Append(dyn.Key("definition_path")), + t.retainLocalAbsoluteFilePath, + }, + } +} + +func (t *translateContext) applyDashboardTranslations(v dyn.Value) (dyn.Value, error) { + var err error + + for _, rewritePattern := range t.dashboardRewritePatterns() { + v, err = dyn.MapByPattern(v, rewritePattern.pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + key := p[1].Key() + dir, err := v.Location().Directory() + if err != nil { + return dyn.InvalidValue, fmt.Errorf("unable to determine directory for dashboard %s: %w", key, err) + } + + return t.rewriteRelativeTo(p, v, rewritePattern.fn, dir, "") + }) + if err != nil { + return dyn.InvalidValue, err + } + } + + return v, nil +} diff --git a/bundle/config/resources.go b/bundle/config/resources.go index 22d69ffb53..3fa11f23e2 100644 --- a/bundle/config/resources.go +++ b/bundle/config/resources.go @@ -19,6 +19,7 @@ type Resources struct { RegisteredModels map[string]*resources.RegisteredModel `json:"registered_models,omitempty"` QualityMonitors map[string]*resources.QualityMonitor `json:"quality_monitors,omitempty"` Schemas map[string]*resources.Schema `json:"schemas,omitempty"` + Dashboards map[string]*resources.Dashboard `json:"dashboards,omitempty"` } type ConfigResource interface { diff --git a/bundle/config/resources/dashboard.go b/bundle/config/resources/dashboard.go new file mode 100644 index 0000000000..d46402717e --- /dev/null +++ b/bundle/config/resources/dashboard.go @@ -0,0 +1,59 @@ +package resources + +import ( + "context" + + "github.com/databricks/cli/libs/log" + "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/marshal" + "github.com/databricks/databricks-sdk-go/service/dashboards" +) + +type Dashboard struct { + ID string `json:"id,omitempty" bundle:"readonly"` + Permissions []Permission `json:"permissions,omitempty"` + ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` + + // =========================== + // === BEGIN OF API FIELDS === + // =========================== + + // DisplayName is the name of the dashboard (both as title and as basename in the workspace). + DisplayName string `json:"display_name,omitempty"` + + // ParentPath is the path to the parent directory of the dashboard. + ParentPath string `json:"parent_path,omitempty"` + + // WarehouseID is the ID of the warehouse to use for the dashboard. + WarehouseID string `json:"warehouse_id,omitempty"` + + // =========================== + // ==== END OF API FIELDS ==== + // =========================== + + // DefinitionPath points to the local `.lvdash.json` file containing the dashboard definition. + DefinitionPath string `json:"definition_path,omitempty"` +} + +func (s *Dashboard) UnmarshalJSON(b []byte) error { + return marshal.Unmarshal(b, s) +} + +func (s Dashboard) MarshalJSON() ([]byte, error) { + return marshal.Marshal(s) +} + +func (_ *Dashboard) Exists(ctx context.Context, w *databricks.WorkspaceClient, id string) (bool, error) { + _, err := w.Lakeview.Get(ctx, dashboards.GetDashboardRequest{ + DashboardId: id, + }) + if err != nil { + log.Debugf(ctx, "Dashboard %s does not exist", id) + return false, err + } + return true, nil +} + +func (_ *Dashboard) TerraformResourceName() string { + return "databricks_dashboard" +} diff --git a/bundle/deploy/terraform/convert.go b/bundle/deploy/terraform/convert.go index f13c241cee..adfc2c175e 100644 --- a/bundle/deploy/terraform/convert.go +++ b/bundle/deploy/terraform/convert.go @@ -394,6 +394,16 @@ func TerraformToBundle(state *resourcesState, config *config.Root) error { } cur.ID = instance.Attributes.ID config.Resources.Schemas[resource.Name] = cur + case "databricks_dashboard": + if config.Resources.Dashboards == nil { + config.Resources.Dashboards = make(map[string]*resources.Dashboard) + } + cur := config.Resources.Dashboards[resource.Name] + if cur == nil { + cur = &resources.Dashboard{ModifiedStatus: resources.ModifiedStatusDeleted} + } + cur.ID = instance.Attributes.ID + config.Resources.Dashboards[resource.Name] = cur case "databricks_permissions": case "databricks_grants": // Ignore; no need to pull these back into the configuration. diff --git a/bundle/deploy/terraform/tfdyn/convert_dashboard.go b/bundle/deploy/terraform/tfdyn/convert_dashboard.go new file mode 100644 index 0000000000..b173c14ed4 --- /dev/null +++ b/bundle/deploy/terraform/tfdyn/convert_dashboard.go @@ -0,0 +1,55 @@ +package tfdyn + +import ( + "context" + "fmt" + + "github.com/databricks/cli/bundle/internal/tf/schema" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/convert" + "github.com/databricks/cli/libs/log" +) + +func convertDashboardResource(ctx context.Context, vin dyn.Value) (dyn.Value, error) { + var err error + + // Normalize the output value to the target schema. + vout, diags := convert.Normalize(schema.ResourceDashboard{}, vin) + for _, diag := range diags { + log.Debugf(ctx, "dashboard normalization diagnostic: %s", diag.Summary) + } + + // Include "serialized_dashboard" field if "definition_path" is set. + if path, ok := vin.Get("definition_path").AsString(); ok { + vout, err = dyn.Set(vout, "serialized_dashboard", dyn.V(fmt.Sprintf("${file(\"%s\")}", path))) + if err != nil { + return dyn.InvalidValue, fmt.Errorf("failed to set serialized_dashboard: %w", err) + } + } + + return vout, nil +} + +type DashboardConverter struct{} + +func (DashboardConverter) Convert(ctx context.Context, key string, vin dyn.Value, out *schema.Resources) error { + vout, err := convertDashboardResource(ctx, vin) + if err != nil { + return err + } + + // Add the converted resource to the output. + out.Dashboard[key] = vout.AsAny() + + // Configure permissions for this resource. + if permissions := convertPermissionsResource(ctx, vin); permissions != nil { + permissions.DashboardId = fmt.Sprintf("${databricks_dashboard.%s.id}", key) + out.Permissions["dashboard_"+key] = permissions + } + + return nil +} + +func init() { + registerConverter("dashboards", DashboardConverter{}) +} diff --git a/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go b/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go new file mode 100644 index 0000000000..2c84967aeb --- /dev/null +++ b/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go @@ -0,0 +1,7 @@ +package tfdyn + +import "testing" + +func TestConvertDashboard(t *testing.T) { + +} diff --git a/bundle/permissions/mutator.go b/bundle/permissions/mutator.go index 7787bc0481..bc1392d932 100644 --- a/bundle/permissions/mutator.go +++ b/bundle/permissions/mutator.go @@ -39,6 +39,10 @@ var levelsMap = map[string](map[string]string){ CAN_VIEW: "CAN_VIEW", CAN_RUN: "CAN_QUERY", }, + "dashboards": { + CAN_MANAGE: "CAN_MANAGE", + CAN_VIEW: "CAN_READ", + }, } type bundlePermissions struct{} diff --git a/internal/acc/workspace.go b/internal/acc/workspace.go index 39374f229e..69ab0e715d 100644 --- a/internal/acc/workspace.go +++ b/internal/acc/workspace.go @@ -2,11 +2,14 @@ package acc import ( "context" + "fmt" "os" "testing" "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/apierr" "github.com/databricks/databricks-sdk-go/service/compute" + "github.com/databricks/databricks-sdk-go/service/workspace" "github.com/stretchr/testify/require" ) @@ -94,3 +97,30 @@ func (t *WorkspaceT) RunPython(code string) (string, error) { require.True(t, ok, "unexpected type %T", results.Data) return output, nil } + +func (t *WorkspaceT) TemporaryWorkspaceDir(name ...string) string { + ctx := context.Background() + me, err := t.W.CurrentUser.Me(ctx) + require.NoError(t, err) + + basePath := fmt.Sprintf("/Users/%s/%s", me.UserName, RandomName(name...)) + + t.Logf("Creating %s", basePath) + err = t.W.Workspace.MkdirsByPath(ctx, basePath) + require.NoError(t, err) + + // Remove test directory on test completion. + t.Cleanup(func() { + t.Logf("Removing %s", basePath) + err := t.W.Workspace.Delete(ctx, workspace.Delete{ + Path: basePath, + Recursive: true, + }) + if err == nil || apierr.IsMissing(err) { + return + } + t.Logf("Unable to remove temporary workspace directory %s: %#v", basePath, err) + }) + + return basePath +} diff --git a/internal/test/dashboard_assumptions_test.go b/internal/test/dashboard_assumptions_test.go new file mode 100644 index 0000000000..ddb3c5962d --- /dev/null +++ b/internal/test/dashboard_assumptions_test.go @@ -0,0 +1,110 @@ +package test + +import ( + "encoding/base64" + "testing" + + "github.com/databricks/cli/internal/acc" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/convert" + "github.com/databricks/cli/libs/dyn/merge" + "github.com/databricks/databricks-sdk-go/apierr" + "github.com/databricks/databricks-sdk-go/service/dashboards" + "github.com/databricks/databricks-sdk-go/service/workspace" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Verify that importing a dashboard through the Workspace API retains the identity of the underying resource, +// as well as properties exclusively accessible through the dashboards API. +func TestDashboardAssumptions_WorkspaceImport(t *testing.T) { + ctx, wt := acc.WorkspaceTest(t) + + t.Parallel() + + dashboardName := "New Dashboard" + dashboardPayload := []byte(`{"pages":[{"name":"2506f97a","displayName":"New Page"}]}`) + warehouseId := acc.GetEnvOrSkipTest(t, "TEST_DEFAULT_WAREHOUSE_ID") + + dir := wt.TemporaryWorkspaceDir("dashboard-assumptions-") + + dashboard, err := wt.W.Lakeview.Create(ctx, dashboards.CreateDashboardRequest{ + DisplayName: dashboardName, + ParentPath: dir, + SerializedDashboard: string(dashboardPayload), + WarehouseId: warehouseId, + }) + require.NoError(t, err) + t.Logf("Dashboard ID (per Lakeview API): %s", dashboard.DashboardId) + + // Overwrite the dashboard via the workspace API. + { + err := wt.W.Workspace.Import(ctx, workspace.Import{ + Format: workspace.ImportFormatAuto, + Path: dashboard.Path, + Content: base64.StdEncoding.EncodeToString(dashboardPayload), + Overwrite: true, + }) + require.NoError(t, err) + } + + // Cross-check consistency with the workspace object. + { + obj, err := wt.W.Workspace.GetStatusByPath(ctx, dashboard.Path) + require.NoError(t, err) + + // Confirm that the resource ID included in the response is equal to the dashboard ID. + require.Equal(t, dashboard.DashboardId, obj.ResourceId) + t.Logf("Dashboard ID (per workspace object status): %s", obj.ResourceId) + } + + // Try to overwrite the dashboard via the Lakeview API (and expect failure). + { + _, err := wt.W.Lakeview.Create(ctx, dashboards.CreateDashboardRequest{ + DisplayName: dashboardName, + ParentPath: dir, + SerializedDashboard: string(dashboardPayload), + }) + require.ErrorIs(t, err, apierr.ErrResourceAlreadyExists) + } + + // Retrieve the dashboard object and confirm that only select fields were updated by the import. + { + obj, err := wt.W.Lakeview.Get(ctx, dashboards.GetDashboardRequest{ + DashboardId: dashboard.DashboardId, + }) + require.NoError(t, err) + + // Convert the dashboard object to a [dyn.Value] to make comparison easier. + previous, err := convert.FromTyped(dashboard, dyn.NilValue) + require.NoError(t, err) + current, err := convert.FromTyped(obj, dyn.NilValue) + require.NoError(t, err) + + // Collect updated paths. + var updatedFieldPaths []string + _, err = merge.Override(previous, current, merge.OverrideVisitor{ + VisitDelete: func(basePath dyn.Path, left dyn.Value) error { + assert.Fail(t, "unexpected delete operation") + return nil + }, + VisitInsert: func(basePath dyn.Path, right dyn.Value) (dyn.Value, error) { + assert.Fail(t, "unexpected insert operation") + return right, nil + }, + VisitUpdate: func(basePath dyn.Path, left dyn.Value, right dyn.Value) (dyn.Value, error) { + updatedFieldPaths = append(updatedFieldPaths, basePath.String()) + return right, nil + }, + }) + require.NoError(t, err) + + // Confirm that only the expected fields have been updated. + assert.ElementsMatch(t, []string{ + "etag", + "serialized_dashboard", + "update_time", + }, updatedFieldPaths) + } +} From cc3bcf992d2a901c36274983bc85f9410f95da91 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 4 Sep 2024 11:30:42 +0200 Subject: [PATCH 02/39] dashboard example --- tmp/dashboard-dab/README.md | 6 +++++ tmp/dashboard-dab/dashboard.lvdash.json | 34 +++++++++++++++++++++++ tmp/dashboard-dab/databricks.yml | 36 +++++++++++++++++++++++++ 3 files changed, 76 insertions(+) create mode 100644 tmp/dashboard-dab/README.md create mode 100644 tmp/dashboard-dab/dashboard.lvdash.json create mode 100644 tmp/dashboard-dab/databricks.yml diff --git a/tmp/dashboard-dab/README.md b/tmp/dashboard-dab/README.md new file mode 100644 index 0000000000..572756e86f --- /dev/null +++ b/tmp/dashboard-dab/README.md @@ -0,0 +1,6 @@ +# Iterating + + +``` +databricks lakeview get 01ef69c6a1b61c85a97505155d58015e --output json | jq -r .serialized_dashboard | jq -S . > dashboard.lvdash.json +``` diff --git a/tmp/dashboard-dab/dashboard.lvdash.json b/tmp/dashboard-dab/dashboard.lvdash.json new file mode 100644 index 0000000000..ea590b5eee --- /dev/null +++ b/tmp/dashboard-dab/dashboard.lvdash.json @@ -0,0 +1,34 @@ +{ + "pages": [ + { + "displayName": "New Page", + "layout": [ + { + "position": { + "height": 2, + "width": 6, + "x": 0, + "y": 0 + }, + "widget": { + "name": "82eb9107", + "textbox_spec": "# hi another new foobar change! if I change this remotely" + } + }, + { + "position": { + "height": 2, + "width": 6, + "x": 0, + "y": 2 + }, + "widget": { + "name": "ffa6de4f", + "textbox_spec": "another widget" + } + } + ], + "name": "fdd21a3c" + } + ] +} diff --git a/tmp/dashboard-dab/databricks.yml b/tmp/dashboard-dab/databricks.yml new file mode 100644 index 0000000000..70a6d15c4e --- /dev/null +++ b/tmp/dashboard-dab/databricks.yml @@ -0,0 +1,36 @@ +bundle: + name: dashboard-eng-work + +workspace: + host: https://e2-dogfood.staging.cloud.databricks.com + +variables: + # 0 - Shared SQL Warehouse (ID: dd43ee29fedd958d) + warehouse_id: + default: dd43ee29fedd958d + +permissions: + - group_name: users + level: CAN_VIEW + +resources: + dashboards: + my_special_dashboard: + # TODO: + # * rename display_name to just "name" + # * remove parent_path, optionally let it be specified as part of "name", + # just like we do for mlflow experiments. + # * default the parent_path to ${workspace.resource_path}. + display_name: "Foobar" + parent_path: ${workspace.file_path} + warehouse_id: ${var.warehouse_id} + definition_path: ./dashboard.lvdash.json + + + # # file_path: ./dashboard.lvdash.json + + # catalog: ${var.default_catalog} + # schema: ${var.default_schema} + + +#https://e2-dogfood.staging.cloud.databricks.com/dashboardsv3/01ef692961381515beac094aa0a82cd5/published?o=6051921418418893 From b7a952d22e5d8ee807cbdd6f89bd9156008e63f3 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 6 Sep 2024 14:12:59 +0200 Subject: [PATCH 03/39] wip --- bundle/config/generate/dashboard.go | 19 + bundle/config/mutator/apply_presets.go | 5 + cmd/bundle/generate.go | 1 + cmd/bundle/generate/dashboard.go | 281 +++++++ cmd/bundle/generate/dashboard_test.go | 1 + .../nyc_taxi_trip_analysis.lvdash.json | 710 ++++++++++++++++++ tmp/dashboard-generate/databricks.yml | 16 + .../resources/nyc_taxi_trip_analysis.yml | 23 + 8 files changed, 1056 insertions(+) create mode 100644 bundle/config/generate/dashboard.go create mode 100644 cmd/bundle/generate/dashboard.go create mode 100644 cmd/bundle/generate/dashboard_test.go create mode 100644 tmp/dashboard-generate/dashboards/nyc_taxi_trip_analysis.lvdash.json create mode 100644 tmp/dashboard-generate/databricks.yml create mode 100644 tmp/dashboard-generate/resources/nyc_taxi_trip_analysis.yml diff --git a/bundle/config/generate/dashboard.go b/bundle/config/generate/dashboard.go new file mode 100644 index 0000000000..4f2c012db2 --- /dev/null +++ b/bundle/config/generate/dashboard.go @@ -0,0 +1,19 @@ +package generate + +import ( + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/databricks-sdk-go/service/dashboards" +) + +func ConvertDashboardToValue(dashboard *dashboards.Dashboard, filePath string) (dyn.Value, error) { + // The majority of fields of the dashboard struct are read-only. + // We copy the relevant fields manually. + dv := map[string]dyn.Value{ + "display_name": dyn.NewValue(dashboard.DisplayName, []dyn.Location{{Line: 1}}), + "parent_path": dyn.NewValue("${workspace.file_path}", []dyn.Location{{Line: 2}}), + "warehouse_id": dyn.NewValue(dashboard.WarehouseId, []dyn.Location{{Line: 3}}), + "definition_path": dyn.NewValue(filePath, []dyn.Location{{Line: 4}}), + } + + return dyn.V(dv), nil +} diff --git a/bundle/config/mutator/apply_presets.go b/bundle/config/mutator/apply_presets.go index 28d015c109..0a12174dc1 100644 --- a/bundle/config/mutator/apply_presets.go +++ b/bundle/config/mutator/apply_presets.go @@ -160,6 +160,11 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos // the Databricks UI and via the SQL API. } + // Dashboards: Prefix + for i := range r.Dashboards { + r.Dashboards[i].DisplayName = prefix + r.Dashboards[i].DisplayName + } + return nil } diff --git a/cmd/bundle/generate.go b/cmd/bundle/generate.go index 1e3d56e430..7dea19ff9d 100644 --- a/cmd/bundle/generate.go +++ b/cmd/bundle/generate.go @@ -16,6 +16,7 @@ func newGenerateCommand() *cobra.Command { cmd.AddCommand(generate.NewGenerateJobCommand()) cmd.AddCommand(generate.NewGeneratePipelineCommand()) + cmd.AddCommand(generate.NewGenerateDashboardCommand()) cmd.PersistentFlags().StringVar(&key, "key", "", `resource key to use for the generated configuration`) return cmd } diff --git a/cmd/bundle/generate/dashboard.go b/cmd/bundle/generate/dashboard.go new file mode 100644 index 0000000000..d0726ba632 --- /dev/null +++ b/cmd/bundle/generate/dashboard.go @@ -0,0 +1,281 @@ +package generate + +import ( + "context" + "encoding/json" + "fmt" + "os" + "path" + "path/filepath" + "strings" + "time" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/generate" + "github.com/databricks/cli/bundle/deploy/terraform" + "github.com/databricks/cli/bundle/phases" + "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/yamlsaver" + "github.com/databricks/cli/libs/textutil" + "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/service/dashboards" + "github.com/databricks/databricks-sdk-go/service/workspace" + "github.com/spf13/cobra" + "gopkg.in/yaml.v3" +) + +type dashboard struct { + resourceDir string + dashboardDir string + force bool + + // Relative path from the resource directory to the dashboard directory. + relativeDir string + + existingDashboardPath string + existingDashboardId string + watch string + + key string +} + +func (d *dashboard) resolveDashboardID(ctx context.Context, w *databricks.WorkspaceClient) diag.Diagnostics { + if d.existingDashboardPath == "" { + return nil + } + + obj, err := w.Workspace.GetStatusByPath(ctx, d.existingDashboardPath) + if err != nil { + return diag.FromErr(err) + } + + if obj.ObjectType != workspace.ObjectTypeDashboard { + found := strings.ToLower(obj.ObjectType.String()) + return diag.Diagnostics{ + { + Severity: diag.Error, + Summary: fmt.Sprintf("expected a dashboard, found a %s", found), + Locations: []dyn.Location{{ + File: d.existingDashboardPath, + }}, + }, + } + } + + if obj.ResourceId == "" { + return diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "expected resource ID to be set", + Locations: []dyn.Location{{ + File: d.existingDashboardPath, + }}, + }, + } + } + + d.existingDashboardId = obj.ResourceId + return nil +} + +func (d *dashboard) saveConfiguration(ctx context.Context, dashboard *dashboards.Dashboard) error { + // TODO: add flag + key := d.key + if key == "" { + key = textutil.NormalizeString(dashboard.DisplayName) + } + + dashboardFilePath := path.Join(d.relativeDir, fmt.Sprintf("%s.lvdash.json", key)) + v, err := generate.ConvertDashboardToValue(dashboard, dashboardFilePath) + if err != nil { + return err + } + + result := map[string]dyn.Value{ + "resources": dyn.V(map[string]dyn.Value{ + "dashboards": dyn.V(map[string]dyn.Value{ + key: v, + }), + }), + } + + // Make sure the output directory exists. + if err := os.MkdirAll(d.resourceDir, 0755); err != nil { + return err + } + + filename := filepath.Join(d.resourceDir, fmt.Sprintf("%s.yml", key)) + saver := yamlsaver.NewSaverWithStyle(map[string]yaml.Style{ + "display_name": yaml.DoubleQuotedStyle, + }) + err = saver.SaveAsYAML(result, filename, false) + if err != nil { + return err + } + + return nil +} + +func (d *dashboard) remarshal(data []byte) ([]byte, error) { + var tmp any + var err error + err = json.Unmarshal(data, &tmp) + if err != nil { + return nil, err + } + out, err := json.MarshalIndent(tmp, "", " ") + if err != nil { + return nil, err + } + return out, nil +} + +func (d *dashboard) saveSerializedDashboard(ctx context.Context, dashboard *dashboards.Dashboard, dst string) error { + // Unmarshal and remarshal the serialized dashboard to ensure it is formatted correctly. + // The result will have alphabetically sorted keys and be indented. + data, err := d.remarshal([]byte(dashboard.SerializedDashboard)) + if err != nil { + return err + } + + // Make sure the output directory exists. + if err := os.MkdirAll(filepath.Dir(dst), 0755); err != nil { + return err + } + + return os.WriteFile(dst, data, 0644) +} + +func (d *dashboard) watchForChanges(ctx context.Context, b *bundle.Bundle) error { + diags := bundle.Apply(ctx, b, bundle.Seq( + phases.Initialize(), + terraform.Interpolate(), + terraform.Write(), + terraform.StatePull(), + terraform.Load(terraform.ErrorOnEmptyState), + )) + if err := diags.Error(); err != nil { + return err + } + + dash, ok := b.Config.Resources.Dashboards[d.watch] + if !ok { + return fmt.Errorf("dashboard %s not found", d.watch) + } + + // fmt.Println(dash.DefinitionPath) + + w := b.WorkspaceClient() + etag := "" + + cwd, err := os.Getwd() + if err != nil { + return err + } + + relPath, err := filepath.Rel(cwd, dash.DefinitionPath) + if err != nil { + return err + } + + for { + dashboard, err := w.Lakeview.GetByDashboardId(ctx, dash.ID) + if err != nil { + return err + } + + // fmt.Println(dashboard.Path) + // fmt.Println(dashboard.Etag) + // fmt.Println(dashboard.UpdateTime) + + // obj, err := w.Workspace.GetStatusByPath(ctx, "/Users/pieter.noordhuis@databricks.com/.bundle/dashboard-eng-work-generate/dev/files/[dev pieter_noordhuis] NYC Taxi Trip Analysis.lvdash.json") + // if err != nil { + // return err + // } + + // fmt.Println(obj.ModifiedAt) + + if etag != dashboard.Etag { + fmt.Printf("[%s]: Updating dashboard at %s\n", dashboard.UpdateTime, relPath) + d.saveSerializedDashboard(ctx, dashboard, dash.DefinitionPath) + } + + etag = dashboard.Etag + time.Sleep(1 * time.Second) + } +} + +func (d *dashboard) RunE(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + b, diags := root.MustConfigureBundle(cmd) + if err := diags.Error(); err != nil { + return diags.Error() + } + + // Make sure we know how the dashboard path is relative to the resource path. + rel, err := filepath.Rel(d.resourceDir, d.dashboardDir) + if err != nil { + return err + } + + d.relativeDir = filepath.ToSlash(rel) + + w := b.WorkspaceClient() + + if d.watch != "" { + return d.watchForChanges(ctx, b) + } + + // Lookup the dashboard ID if the path is given + diags = d.resolveDashboardID(ctx, w) + if diags.HasError() { + return diags.Error() + } + + dashboard, err := w.Lakeview.GetByDashboardId(ctx, d.existingDashboardId) + if err != nil { + return err + } + + d.saveConfiguration(ctx, dashboard) + + // TODO: add flag + key := d.key + if key == "" { + key = textutil.NormalizeString(dashboard.DisplayName) + } + + filename := filepath.Join(d.dashboardDir, fmt.Sprintf("%s.lvdash.json", key)) + d.saveSerializedDashboard(ctx, dashboard, filename) + return nil +} + +func NewGenerateDashboardCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "dashboard", + Short: "Generate configuration for a dashboard", + } + + d := &dashboard{} + + cmd.Flags().StringVarP(&d.resourceDir, "resource-dir", "d", "./resources", `directory to write the configuration to`) + cmd.Flags().StringVarP(&d.dashboardDir, "dashboard-dir", "s", "./dashboards", `directory to write the dashboard representation to`) + cmd.Flags().BoolVarP(&d.force, "force", "f", false, `force overwrite existing files in the output directory`) + + // Specify dashboard by workspace path + + cmd.Flags().StringVar(&d.existingDashboardPath, "existing-dashboard-path", "", `workspace path of the dashboard to generate configuration for`) + cmd.Flags().StringVar(&d.existingDashboardId, "existing-dashboard-id", "", `ID of the dashboard to generate configuration for`) + cmd.Flags().StringVar(&d.watch, "watch-resource", "", `resource key of dashboard to watch for changes`) + + cmd.MarkFlagsOneRequired( + "existing-dashboard-path", + "existing-dashboard-id", + "watch-resource", + ) + + cmd.RunE = d.RunE + return cmd +} diff --git a/cmd/bundle/generate/dashboard_test.go b/cmd/bundle/generate/dashboard_test.go new file mode 100644 index 0000000000..eb8347795a --- /dev/null +++ b/cmd/bundle/generate/dashboard_test.go @@ -0,0 +1 @@ +package generate diff --git a/tmp/dashboard-generate/dashboards/nyc_taxi_trip_analysis.lvdash.json b/tmp/dashboard-generate/dashboards/nyc_taxi_trip_analysis.lvdash.json new file mode 100644 index 0000000000..162535efd3 --- /dev/null +++ b/tmp/dashboard-generate/dashboards/nyc_taxi_trip_analysis.lvdash.json @@ -0,0 +1,710 @@ +{ + "datasets": [ + { + "displayName": "route revenue", + "name": "fdefd57c", + "query": "SELECT\n T.pickup_zip,\n T.dropoff_zip,\n T.route as `Route`,\n T.frequency as `Number Trips`,\n T.total_fare as `Total Revenue`\nFROM\n (\n SELECT\n pickup_zip,\n dropoff_zip,\n concat(pickup_zip, '-', dropoff_zip) AS route,\n count(*) as frequency,\n SUM(fare_amount) as total_fare\n FROM\n `samples`.`nyctaxi`.`trips`\n GROUP BY\n 1,2,3\n ) T\nORDER BY\n 1 ASC" + }, + { + "displayName": "trips", + "name": "ecfcdc7c", + "query": "SELECT\n T.tpep_pickup_datetime,\n T.tpep_dropoff_datetime,\n T.fare_amount,\n T.pickup_zip,\n T.dropoff_zip,\n T.trip_distance,\n T.weekday,\n CASE\n WHEN T.weekday = 1 THEN 'Sunday'\n WHEN T.weekday = 2 THEN 'Monday'\n WHEN T.weekday = 3 THEN 'Tuesday'\n WHEN T.weekday = 4 THEN 'Wednesday'\n WHEN T.weekday = 5 THEN 'Thursday'\n WHEN T.weekday = 6 THEN 'Friday'\n WHEN T.weekday = 7 THEN 'Saturday'\n ELSE 'N/A'\n END AS day_of_week, \n T.fare_amount, \n T.trip_distance\nFROM\n (\n SELECT\n dayofweek(tpep_pickup_datetime) as weekday,\n *\n FROM\n `samples`.`nyctaxi`.`trips`\n WHERE\n trip_distance \u003e 0\n AND trip_distance \u003c 10\n AND fare_amount \u003e 0\n AND fare_amount \u003c 50\n ) T\nORDER BY\n T.weekday " + } + ], + "pages": [ + { + "displayName": "New Page", + "layout": [ + { + "position": { + "height": 1, + "width": 2, + "x": 0, + "y": 1 + }, + "widget": { + "name": "c4d87efe", + "queries": [ + { + "name": "dashboards/01ee564285a315dd80d473e76171660a/datasets/01ee564285a51daf810a8ffc5051bfee_tpep_dropoff_datetime", + "query": { + "datasetName": "ecfcdc7c", + "disaggregated": false, + "fields": [ + { + "expression": "`tpep_dropoff_datetime`", + "name": "tpep_dropoff_datetime" + }, + { + "expression": "COUNT_IF(`associative_filter_predicate_group`)", + "name": "tpep_dropoff_datetime_associativity" + } + ] + } + } + ], + "spec": { + "encodings": { + "fields": [ + { + "displayName": "tpep_dropoff_datetime", + "fieldName": "tpep_dropoff_datetime", + "queryName": "dashboards/01ee564285a315dd80d473e76171660a/datasets/01ee564285a51daf810a8ffc5051bfee_tpep_dropoff_datetime" + } + ] + }, + "frame": { + "showTitle": true, + "title": "Time Range" + }, + "version": 2, + "widgetType": "filter-date-range-picker" + } + } + }, + { + "position": { + "height": 4, + "width": 3, + "x": 0, + "y": 10 + }, + "widget": { + "name": "61e19e9c", + "queries": [ + { + "name": "main_query", + "query": { + "datasetName": "ecfcdc7c", + "disaggregated": false, + "fields": [ + { + "expression": "COUNT(`*`)", + "name": "count(*)" + }, + { + "expression": "DATE_TRUNC(\"HOUR\", `tpep_pickup_datetime`)", + "name": "hourly(tpep_pickup_datetime)" + } + ] + } + } + ], + "spec": { + "encodings": { + "label": { + "show": false + }, + "x": { + "axis": { + "title": "Pickup Hour" + }, + "displayName": "Pickup Hour", + "fieldName": "hourly(tpep_pickup_datetime)", + "scale": { + "type": "temporal" + } + }, + "y": { + "axis": { + "title": "Number of Rides" + }, + "displayName": "Number of Rides", + "fieldName": "count(*)", + "scale": { + "type": "quantitative" + } + } + }, + "frame": { + "showTitle": true, + "title": "Pickup Hour Distribution" + }, + "mark": { + "colors": [ + "#077A9D", + "#FFAB00", + "#00A972", + "#FF3621", + "#8BCAE7", + "#AB4057", + "#99DDB4", + "#FCA4A1", + "#919191", + "#BF7080" + ] + }, + "version": 3, + "widgetType": "bar" + } + } + }, + { + "position": { + "height": 8, + "width": 4, + "x": 2, + "y": 2 + }, + "widget": { + "name": "3b1dff20", + "queries": [ + { + "name": "main_query", + "query": { + "datasetName": "ecfcdc7c", + "disaggregated": true, + "fields": [ + { + "expression": "`day_of_week`", + "name": "day_of_week" + }, + { + "expression": "`fare_amount`", + "name": "fare_amount" + }, + { + "expression": "`trip_distance`", + "name": "trip_distance" + } + ] + } + } + ], + "spec": { + "encodings": { + "color": { + "displayName": "Day of Week", + "fieldName": "day_of_week", + "scale": { + "type": "categorical" + } + }, + "x": { + "axis": { + "title": "Trip Distance (miles)" + }, + "displayName": "trip_distance", + "fieldName": "trip_distance", + "scale": { + "type": "quantitative" + } + }, + "y": { + "axis": { + "title": "Fare Amount (USD)" + }, + "displayName": "fare_amount", + "fieldName": "fare_amount", + "scale": { + "type": "quantitative" + } + } + }, + "frame": { + "showTitle": true, + "title": "Daily Fare Trends by Day of Week" + }, + "version": 3, + "widgetType": "scatter" + } + } + }, + { + "position": { + "height": 1, + "width": 6, + "x": 0, + "y": 0 + }, + "widget": { + "name": "bd82f575", + "textbox_spec": "# NYC Taxi Trip Analysis" + } + }, + { + "position": { + "height": 4, + "width": 3, + "x": 3, + "y": 10 + }, + "widget": { + "name": "e7b33e79", + "queries": [ + { + "name": "main_query", + "query": { + "datasetName": "ecfcdc7c", + "disaggregated": false, + "fields": [ + { + "expression": "COUNT(`*`)", + "name": "count(*)" + }, + { + "expression": "DATE_TRUNC(\"HOUR\", `tpep_dropoff_datetime`)", + "name": "hourly(tpep_dropoff_datetime)" + } + ] + } + } + ], + "spec": { + "encodings": { + "x": { + "axis": { + "title": "Dropoff Hour" + }, + "displayName": "Dropoff Hour", + "fieldName": "hourly(tpep_dropoff_datetime)", + "scale": { + "type": "temporal" + } + }, + "y": { + "axis": { + "title": "Number of Rides" + }, + "displayName": "Number of Rides", + "fieldName": "count(*)", + "scale": { + "type": "quantitative" + } + } + }, + "frame": { + "showTitle": true, + "title": "Dropoff Hour Distribution" + }, + "mark": { + "colors": [ + "#FFAB00", + "#FFAB00", + "#00A972", + "#FF3621", + "#8BCAE7", + "#AB4057", + "#99DDB4", + "#FCA4A1", + "#919191", + "#BF7080" + ] + }, + "version": 3, + "widgetType": "bar" + } + } + }, + { + "position": { + "height": 2, + "width": 2, + "x": 0, + "y": 2 + }, + "widget": { + "name": "299e756c", + "queries": [ + { + "name": "main_query", + "query": { + "datasetName": "ecfcdc7c", + "disaggregated": false, + "fields": [ + { + "expression": "COUNT(`*`)", + "name": "count(*)" + } + ] + } + } + ], + "spec": { + "encodings": { + "value": { + "displayName": "Count of Records", + "fieldName": "count(*)", + "style": { + "bold": true, + "color": "#E92828" + } + } + }, + "frame": { + "showTitle": true, + "title": "Total Trips" + }, + "version": 2, + "widgetType": "counter" + } + } + }, + { + "position": { + "height": 1, + "width": 2, + "x": 2, + "y": 1 + }, + "widget": { + "name": "61a54236", + "queries": [ + { + "name": "dashboards/01eed0e4082a1c7e903cac7e74114376/datasets/01eed0e4082d1205adc131b86b10198e_pickup_zip", + "query": { + "datasetName": "fdefd57c", + "disaggregated": false, + "fields": [ + { + "expression": "`pickup_zip`", + "name": "pickup_zip" + }, + { + "expression": "COUNT_IF(`associative_filter_predicate_group`)", + "name": "pickup_zip_associativity" + } + ] + } + }, + { + "name": "dashboards/01eed0e4082a1c7e903cac7e74114376/datasets/01eed0e4082e1ff49c3209776820e82e_pickup_zip", + "query": { + "datasetName": "ecfcdc7c", + "disaggregated": false, + "fields": [ + { + "expression": "`pickup_zip`", + "name": "pickup_zip" + }, + { + "expression": "COUNT_IF(`associative_filter_predicate_group`)", + "name": "pickup_zip_associativity" + } + ] + } + } + ], + "spec": { + "encodings": { + "fields": [ + { + "displayName": "pickup_zip", + "fieldName": "pickup_zip", + "queryName": "dashboards/01eed0e4082a1c7e903cac7e74114376/datasets/01eed0e4082e1ff49c3209776820e82e_pickup_zip" + }, + { + "displayName": "pickup_zip", + "fieldName": "pickup_zip", + "queryName": "dashboards/01eed0e4082a1c7e903cac7e74114376/datasets/01eed0e4082d1205adc131b86b10198e_pickup_zip" + } + ] + }, + "frame": { + "showTitle": true, + "title": "Pickup Zip" + }, + "version": 2, + "widgetType": "filter-multi-select" + } + } + }, + { + "position": { + "height": 6, + "width": 2, + "x": 0, + "y": 4 + }, + "widget": { + "name": "985e7eb4", + "queries": [ + { + "name": "main_query", + "query": { + "datasetName": "fdefd57c", + "disaggregated": true, + "fields": [ + { + "expression": "`Number Trips`", + "name": "Number Trips" + }, + { + "expression": "`Route`", + "name": "Route" + }, + { + "expression": "`Total Revenue`", + "name": "Total Revenue" + } + ] + } + } + ], + "spec": { + "allowHTMLByDefault": false, + "condensed": true, + "encodings": { + "columns": [ + { + "alignContent": "left", + "allowHTML": false, + "allowSearch": false, + "booleanValues": [ + "false", + "true" + ], + "displayAs": "string", + "displayName": "Route", + "fieldName": "Route", + "highlightLinks": false, + "imageHeight": "", + "imageTitleTemplate": "{{ @ }}", + "imageUrlTemplate": "{{ @ }}", + "imageWidth": "", + "linkOpenInNewTab": true, + "linkTextTemplate": "{{ @ }}", + "linkTitleTemplate": "{{ @ }}", + "linkUrlTemplate": "{{ @ }}", + "order": 100000, + "preserveWhitespace": false, + "title": "Route", + "type": "string", + "useMonospaceFont": false, + "visible": true + }, + { + "alignContent": "right", + "allowHTML": false, + "allowSearch": false, + "booleanValues": [ + "false", + "true" + ], + "displayAs": "number", + "displayName": "Number Trips", + "fieldName": "Number Trips", + "highlightLinks": false, + "imageHeight": "", + "imageTitleTemplate": "{{ @ }}", + "imageUrlTemplate": "{{ @ }}", + "imageWidth": "", + "linkOpenInNewTab": true, + "linkTextTemplate": "{{ @ }}", + "linkTitleTemplate": "{{ @ }}", + "linkUrlTemplate": "{{ @ }}", + "numberFormat": "0", + "order": 100001, + "preserveWhitespace": false, + "title": "Number Trips", + "type": "integer", + "useMonospaceFont": false, + "visible": true + }, + { + "alignContent": "right", + "allowHTML": false, + "allowSearch": false, + "booleanValues": [ + "false", + "true" + ], + "cellFormat": { + "default": { + "foregroundColor": "#85CADE" + }, + "rules": [ + { + "if": { + "column": "Total Revenue", + "fn": "\u003c", + "literal": "51" + }, + "value": { + "foregroundColor": "#9C2638" + } + }, + { + "if": { + "column": "Total Revenue", + "fn": "\u003c", + "literal": "101" + }, + "value": { + "foregroundColor": "#FFD465" + } + }, + { + "if": { + "column": "Total Revenue", + "fn": "\u003c", + "literal": "6001" + }, + "value": { + "foregroundColor": "#1FA873" + } + } + ] + }, + "displayAs": "number", + "displayName": "Total Revenue", + "fieldName": "Total Revenue", + "highlightLinks": false, + "imageHeight": "", + "imageTitleTemplate": "{{ @ }}", + "imageUrlTemplate": "{{ @ }}", + "imageWidth": "", + "linkOpenInNewTab": true, + "linkTextTemplate": "{{ @ }}", + "linkTitleTemplate": "{{ @ }}", + "linkUrlTemplate": "{{ @ }}", + "numberFormat": "$0.00", + "order": 100002, + "preserveWhitespace": false, + "title": "Total Revenue", + "type": "float", + "useMonospaceFont": false, + "visible": true + } + ] + }, + "frame": { + "showTitle": true, + "title": "Route Revenue Attribution" + }, + "invisibleColumns": [ + { + "alignContent": "right", + "allowHTML": false, + "allowSearch": false, + "booleanValues": [ + "false", + "true" + ], + "displayAs": "number", + "highlightLinks": false, + "imageHeight": "", + "imageTitleTemplate": "{{ @ }}", + "imageUrlTemplate": "{{ @ }}", + "imageWidth": "", + "linkOpenInNewTab": true, + "linkTextTemplate": "{{ @ }}", + "linkTitleTemplate": "{{ @ }}", + "linkUrlTemplate": "{{ @ }}", + "name": "pickup_zip", + "numberFormat": "0", + "order": 100000, + "preserveWhitespace": false, + "title": "pickup_zip", + "type": "integer", + "useMonospaceFont": false + }, + { + "alignContent": "right", + "allowHTML": false, + "allowSearch": false, + "booleanValues": [ + "false", + "true" + ], + "displayAs": "number", + "highlightLinks": false, + "imageHeight": "", + "imageTitleTemplate": "{{ @ }}", + "imageUrlTemplate": "{{ @ }}", + "imageWidth": "", + "linkOpenInNewTab": true, + "linkTextTemplate": "{{ @ }}", + "linkTitleTemplate": "{{ @ }}", + "linkUrlTemplate": "{{ @ }}", + "name": "dropoff_zip", + "numberFormat": "0", + "order": 100001, + "preserveWhitespace": false, + "title": "dropoff_zip", + "type": "integer", + "useMonospaceFont": false + } + ], + "itemsPerPage": 25, + "paginationSize": "default", + "version": 1, + "widgetType": "table", + "withRowNumber": false + } + } + }, + { + "position": { + "height": 1, + "width": 2, + "x": 4, + "y": 1 + }, + "widget": { + "name": "b346c038", + "queries": [ + { + "name": "dashboards/01eed0e4082a1c7e903cac7e74114376/datasets/01eed0e4082d1205adc131b86b10198e_dropoff_zip", + "query": { + "datasetName": "fdefd57c", + "disaggregated": false, + "fields": [ + { + "expression": "`dropoff_zip`", + "name": "dropoff_zip" + }, + { + "expression": "COUNT_IF(`associative_filter_predicate_group`)", + "name": "dropoff_zip_associativity" + } + ] + } + }, + { + "name": "dashboards/01eed0e4082a1c7e903cac7e74114376/datasets/01eed0e4082e1ff49c3209776820e82e_dropoff_zip", + "query": { + "datasetName": "ecfcdc7c", + "disaggregated": false, + "fields": [ + { + "expression": "`dropoff_zip`", + "name": "dropoff_zip" + }, + { + "expression": "COUNT_IF(`associative_filter_predicate_group`)", + "name": "dropoff_zip_associativity" + } + ] + } + } + ], + "spec": { + "encodings": { + "fields": [ + { + "displayName": "dropoff_zip", + "fieldName": "dropoff_zip", + "queryName": "dashboards/01eed0e4082a1c7e903cac7e74114376/datasets/01eed0e4082e1ff49c3209776820e82e_dropoff_zip" + }, + { + "displayName": "dropoff_zip", + "fieldName": "dropoff_zip", + "queryName": "dashboards/01eed0e4082a1c7e903cac7e74114376/datasets/01eed0e4082d1205adc131b86b10198e_dropoff_zip" + } + ] + }, + "frame": { + "showTitle": true, + "title": "Dropoff Zip" + }, + "version": 2, + "widgetType": "filter-multi-select" + } + } + } + ], + "name": "b51b1363" + } + ] +} \ No newline at end of file diff --git a/tmp/dashboard-generate/databricks.yml b/tmp/dashboard-generate/databricks.yml new file mode 100644 index 0000000000..8670872cea --- /dev/null +++ b/tmp/dashboard-generate/databricks.yml @@ -0,0 +1,16 @@ +bundle: + name: dashboard-eng-work-generate + +workspace: + host: https://e2-dogfood.staging.cloud.databricks.com + +include: + - resources/*.yml + +permissions: + - group_name: users + level: CAN_VIEW + +targets: + dev: + mode: development diff --git a/tmp/dashboard-generate/resources/nyc_taxi_trip_analysis.yml b/tmp/dashboard-generate/resources/nyc_taxi_trip_analysis.yml new file mode 100644 index 0000000000..9a76969a12 --- /dev/null +++ b/tmp/dashboard-generate/resources/nyc_taxi_trip_analysis.yml @@ -0,0 +1,23 @@ +resources: + dashboards: + nyc_taxi_trip_analysis: + display_name: "NYC Taxi Trip Analysis" + parent_path: ${workspace.file_path} + warehouse_id: 4fe75792cd0d304c + definition_path: ../dashboards/nyc_taxi_trip_analysis.lvdash.json + + # To be implemented when ready in the product: + # + # catalog: ${var.default_catalog} + # schema: ${var.default_schema} + # schedules: + # - name: Daily + # # ... + permissions: + # Allow all users to view the dashboard + - group_name: users + level: CAN_READ + + # Allow all account users to view the dashboard + - group_name: account users + level: CAN_READ From 3a1d92c75c34379bdbd7b0138c3c2f0b8e3bba9e Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 27 Sep 2024 16:39:51 +0200 Subject: [PATCH 04/39] Comments --- bundle/config/resources/dashboard.go | 34 ++++++++++++++++++------- bundle/deploy/terraform/convert_test.go | 7 +++-- cmd/bundle/generate/dashboard.go | 4 +-- 3 files changed, 32 insertions(+), 13 deletions(-) diff --git a/bundle/config/resources/dashboard.go b/bundle/config/resources/dashboard.go index d46402717e..20acc9b072 100644 --- a/bundle/config/resources/dashboard.go +++ b/bundle/config/resources/dashboard.go @@ -18,21 +18,37 @@ type Dashboard struct { // === BEGIN OF API FIELDS === // =========================== - // DisplayName is the name of the dashboard (both as title and as basename in the workspace). - DisplayName string `json:"display_name,omitempty"` + // DisplayName is the display name of the dashboard (both as title and as basename in the workspace). + DisplayName string `json:"display_name"` - // ParentPath is the path to the parent directory of the dashboard. + // WarehouseID is the ID of the SQL Warehouse used to run the dashboard's queries. + WarehouseID string `json:"warehouse_id"` + + // SerializedDashboard is the contents of the dashboard in serialized JSON form. + // Note: its type is any and not string such that it can be inlined as YAML. + // If it is not a string, its contents will be marshalled as JSON. + SerializedDashboard any `json:"serialized_dashboard,omitempty"` + + // ParentPath is the workspace path of the folder containing the dashboard. + // Includes leading slash and no trailing slash. + // + // Defaults to ${workspace.resource_path} if not set. ParentPath string `json:"parent_path,omitempty"` - // WarehouseID is the ID of the warehouse to use for the dashboard. - WarehouseID string `json:"warehouse_id,omitempty"` + // EmbedCredentials is a flag to indicate if the publisher's credentials should + // be embedded in the published dashboard. These embedded credentials will be used + // to execute the published dashboard's queries. + // + // Defaults to false if not set. + EmbedCredentials bool `json:"embed_credentials,omitempty"` // =========================== // ==== END OF API FIELDS ==== // =========================== - // DefinitionPath points to the local `.lvdash.json` file containing the dashboard definition. - DefinitionPath string `json:"definition_path,omitempty"` + // FilePath points to the local `.lvdash.json` file containing the dashboard definition. + // If specified, it will populate the `SerializedDashboard` field. + FilePath string `json:"file_path,omitempty"` } func (s *Dashboard) UnmarshalJSON(b []byte) error { @@ -43,7 +59,7 @@ func (s Dashboard) MarshalJSON() ([]byte, error) { return marshal.Marshal(s) } -func (_ *Dashboard) Exists(ctx context.Context, w *databricks.WorkspaceClient, id string) (bool, error) { +func (*Dashboard) Exists(ctx context.Context, w *databricks.WorkspaceClient, id string) (bool, error) { _, err := w.Lakeview.Get(ctx, dashboards.GetDashboardRequest{ DashboardId: id, }) @@ -54,6 +70,6 @@ func (_ *Dashboard) Exists(ctx context.Context, w *databricks.WorkspaceClient, i return true, nil } -func (_ *Dashboard) TerraformResourceName() string { +func (*Dashboard) TerraformResourceName() string { return "databricks_dashboard" } diff --git a/bundle/deploy/terraform/convert_test.go b/bundle/deploy/terraform/convert_test.go index 4c6866d9d8..cc5073423b 100644 --- a/bundle/deploy/terraform/convert_test.go +++ b/bundle/deploy/terraform/convert_test.go @@ -58,9 +58,12 @@ func TestBundleToTerraformJob(t *testing.T) { }, } - out := BundleToTerraform(&config) - resource := out.Resource.Job["my_job"].(*schema.ResourceJob) + vin, err := convert.FromTyped(config, dyn.NilValue) + require.NoError(t, err) + out, err := BundleToTerraformWithDynValue(context.Background(), vin) + require.NoError(t, err) + resource := out.Resource.Job["my_job"].(*schema.ResourceJob) assert.Equal(t, "my job", resource.Name) assert.Len(t, resource.JobCluster, 1) assert.Equal(t, "https://github.com/foo/bar", resource.GitSource.Url) diff --git a/cmd/bundle/generate/dashboard.go b/cmd/bundle/generate/dashboard.go index d0726ba632..12a23b3f5b 100644 --- a/cmd/bundle/generate/dashboard.go +++ b/cmd/bundle/generate/dashboard.go @@ -175,7 +175,7 @@ func (d *dashboard) watchForChanges(ctx context.Context, b *bundle.Bundle) error return err } - relPath, err := filepath.Rel(cwd, dash.DefinitionPath) + relPath, err := filepath.Rel(cwd, dash.FilePath) if err != nil { return err } @@ -199,7 +199,7 @@ func (d *dashboard) watchForChanges(ctx context.Context, b *bundle.Bundle) error if etag != dashboard.Etag { fmt.Printf("[%s]: Updating dashboard at %s\n", dashboard.UpdateTime, relPath) - d.saveSerializedDashboard(ctx, dashboard, dash.DefinitionPath) + d.saveSerializedDashboard(ctx, dashboard, dash.FilePath) } etag = dashboard.Etag From 802be90687864d0aa9ac6046d76995b05a66a233 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 30 Sep 2024 11:54:08 +0200 Subject: [PATCH 05/39] Coverage for conversion logic --- .../mutator/translate_paths_dashboards.go | 2 +- bundle/config/resources/dashboard.go | 1 - .../terraform/tfdyn/convert_dashboard.go | 38 +++++- .../terraform/tfdyn/convert_dashboard_test.go | 110 +++++++++++++++++- 4 files changed, 143 insertions(+), 8 deletions(-) diff --git a/bundle/config/mutator/translate_paths_dashboards.go b/bundle/config/mutator/translate_paths_dashboards.go index 341156163d..2ead527c70 100644 --- a/bundle/config/mutator/translate_paths_dashboards.go +++ b/bundle/config/mutator/translate_paths_dashboards.go @@ -22,7 +22,7 @@ func (t *translateContext) dashboardRewritePatterns() []dashboardRewritePattern // Compile list of configuration paths to rewrite. return []dashboardRewritePattern{ { - base.Append(dyn.Key("definition_path")), + base.Append(dyn.Key("file_path")), t.retainLocalAbsoluteFilePath, }, } diff --git a/bundle/config/resources/dashboard.go b/bundle/config/resources/dashboard.go index 20acc9b072..93474e7a72 100644 --- a/bundle/config/resources/dashboard.go +++ b/bundle/config/resources/dashboard.go @@ -47,7 +47,6 @@ type Dashboard struct { // =========================== // FilePath points to the local `.lvdash.json` file containing the dashboard definition. - // If specified, it will populate the `SerializedDashboard` field. FilePath string `json:"file_path,omitempty"` } diff --git a/bundle/deploy/terraform/tfdyn/convert_dashboard.go b/bundle/deploy/terraform/tfdyn/convert_dashboard.go index b173c14ed4..13be530b82 100644 --- a/bundle/deploy/terraform/tfdyn/convert_dashboard.go +++ b/bundle/deploy/terraform/tfdyn/convert_dashboard.go @@ -19,20 +19,48 @@ func convertDashboardResource(ctx context.Context, vin dyn.Value) (dyn.Value, er log.Debugf(ctx, "dashboard normalization diagnostic: %s", diag.Summary) } - // Include "serialized_dashboard" field if "definition_path" is set. - if path, ok := vin.Get("definition_path").AsString(); ok { + // Include "serialized_dashboard" field if "file_path" is set. + // Note: the Terraform resource supports "file_path" natively, but its + // change detection mechanism doesn't work as expected at the time of writing (Sep 30). + if path, ok := vout.Get("file_path").AsString(); ok { vout, err = dyn.Set(vout, "serialized_dashboard", dyn.V(fmt.Sprintf("${file(\"%s\")}", path))) if err != nil { return dyn.InvalidValue, fmt.Errorf("failed to set serialized_dashboard: %w", err) } + // Drop the "file_path" field. It is mutually exclusive with "serialized_dashboard". + vout, err = dyn.Walk(vout, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + switch len(p) { + case 0: + return v, nil + case 1: + if p[0] == dyn.Key("file_path") { + return v, dyn.ErrDrop + } + } + + // Skip everything else. + return v, dyn.ErrSkip + }) + if err != nil { + return dyn.InvalidValue, fmt.Errorf("failed to drop file_path: %w", err) + } + } + + // Default the "embed_credentials" field to "false", if not already set. + // This is different from the behavior in the Terraform provider, so we make it explicit. + if _, ok := vout.Get("embed_credentials").AsBool(); !ok { + vout, err = dyn.Set(vout, "embed_credentials", dyn.V(false)) + if err != nil { + return dyn.InvalidValue, fmt.Errorf("failed to set embed_credentials: %w", err) + } } return vout, nil } -type DashboardConverter struct{} +type dashboardConverter struct{} -func (DashboardConverter) Convert(ctx context.Context, key string, vin dyn.Value, out *schema.Resources) error { +func (dashboardConverter) Convert(ctx context.Context, key string, vin dyn.Value, out *schema.Resources) error { vout, err := convertDashboardResource(ctx, vin) if err != nil { return err @@ -51,5 +79,5 @@ func (DashboardConverter) Convert(ctx context.Context, key string, vin dyn.Value } func init() { - registerConverter("dashboards", DashboardConverter{}) + registerConverter("dashboards", dashboardConverter{}) } diff --git a/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go b/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go index 2c84967aeb..886be16f1b 100644 --- a/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go +++ b/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go @@ -1,7 +1,115 @@ package tfdyn -import "testing" +import ( + "context" + "fmt" + "testing" + + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/internal/tf/schema" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/convert" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) func TestConvertDashboard(t *testing.T) { + var src = resources.Dashboard{ + DisplayName: "my dashboard", + WarehouseID: "f00dcafe", + ParentPath: "/some/path", + EmbedCredentials: true, + + Permissions: []resources.Permission{ + { + Level: "CAN_VIEW", + UserName: "jane@doe.com", + }, + }, + } + + vin, err := convert.FromTyped(src, dyn.NilValue) + require.NoError(t, err) + + ctx := context.Background() + out := schema.NewResources() + err = dashboardConverter{}.Convert(ctx, "my_dashboard", vin, out) + require.NoError(t, err) + + // Assert equality on the job + assert.Equal(t, map[string]any{ + "display_name": "my dashboard", + "warehouse_id": "f00dcafe", + "parent_path": "/some/path", + "embed_credentials": true, + }, out.Dashboard["my_dashboard"]) + + // Assert equality on the permissions + assert.Equal(t, &schema.ResourcePermissions{ + DashboardId: "${databricks_dashboard.my_dashboard.id}", + AccessControl: []schema.ResourcePermissionsAccessControl{ + { + PermissionLevel: "CAN_VIEW", + UserName: "jane@doe.com", + }, + }, + }, out.Permissions["dashboard_my_dashboard"]) +} + +func TestConvertDashboardFilePath(t *testing.T) { + var src = resources.Dashboard{ + FilePath: "some/path", + } + + vin, err := convert.FromTyped(src, dyn.NilValue) + require.NoError(t, err) + + ctx := context.Background() + out := schema.NewResources() + err = dashboardConverter{}.Convert(ctx, "my_dashboard", vin, out) + require.NoError(t, err) + + // Assert that the "serialized_dashboard" is included. + assert.Subset(t, out.Dashboard["my_dashboard"], map[string]any{ + "serialized_dashboard": "${file(\"some/path\")}", + }) + + // Assert that the "file_path" doesn't carry over. + assert.NotSubset(t, out.Dashboard["my_dashboard"], map[string]any{ + "file_path": "some/path", + }) +} + +func TestConvertDashboardEmbedCredentialsPassthrough(t *testing.T) { + for _, v := range []bool{true, false} { + t.Run(fmt.Sprintf("set to %v", v), func(t *testing.T) { + vin := dyn.V(map[string]dyn.Value{ + "embed_credentials": dyn.V(v), + }) + + ctx := context.Background() + out := schema.NewResources() + err := dashboardConverter{}.Convert(ctx, "my_dashboard", vin, out) + require.NoError(t, err) + + // Assert that the "embed_credentials" is set as configured. + assert.Subset(t, out.Dashboard["my_dashboard"], map[string]any{ + "embed_credentials": v, + }) + }) + } +} + +func TestConvertDashboardEmbedCredentialsDefault(t *testing.T) { + vin := dyn.V(map[string]dyn.Value{}) + + ctx := context.Background() + out := schema.NewResources() + err := dashboardConverter{}.Convert(ctx, "my_dashboard", vin, out) + require.NoError(t, err) + // Assert that the "embed_credentials" is set to false (by default). + assert.Subset(t, out.Dashboard["my_dashboard"], map[string]any{ + "embed_credentials": false, + }) } From 08f7f3b6b77d00c24e1a0ec291a667110fc16510 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 30 Sep 2024 14:29:16 +0200 Subject: [PATCH 06/39] Add resource path field to bundle workspace configuration --- bundle/config/mutator/default_workspace_paths.go | 4 ++++ bundle/config/mutator/default_workspace_paths_test.go | 3 +++ bundle/config/mutator/process_target_mode.go | 9 ++++++--- bundle/config/workspace.go | 5 +++++ 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/bundle/config/mutator/default_workspace_paths.go b/bundle/config/mutator/default_workspace_paths.go index 71e562b51f..02a1ddb3b1 100644 --- a/bundle/config/mutator/default_workspace_paths.go +++ b/bundle/config/mutator/default_workspace_paths.go @@ -29,6 +29,10 @@ func (m *defineDefaultWorkspacePaths) Apply(ctx context.Context, b *bundle.Bundl b.Config.Workspace.FilePath = path.Join(root, "files") } + if b.Config.Workspace.ResourcePath == "" { + b.Config.Workspace.ResourcePath = path.Join(root, "resources") + } + if b.Config.Workspace.ArtifactPath == "" { b.Config.Workspace.ArtifactPath = path.Join(root, "artifacts") } diff --git a/bundle/config/mutator/default_workspace_paths_test.go b/bundle/config/mutator/default_workspace_paths_test.go index 0ba20ea2bd..6779c37320 100644 --- a/bundle/config/mutator/default_workspace_paths_test.go +++ b/bundle/config/mutator/default_workspace_paths_test.go @@ -22,6 +22,7 @@ func TestDefineDefaultWorkspacePaths(t *testing.T) { diags := bundle.Apply(context.Background(), b, mutator.DefineDefaultWorkspacePaths()) require.NoError(t, diags.Error()) assert.Equal(t, "/files", b.Config.Workspace.FilePath) + assert.Equal(t, "/resources", b.Config.Workspace.ResourcePath) assert.Equal(t, "/artifacts", b.Config.Workspace.ArtifactPath) assert.Equal(t, "/state", b.Config.Workspace.StatePath) } @@ -32,6 +33,7 @@ func TestDefineDefaultWorkspacePathsAlreadySet(t *testing.T) { Workspace: config.Workspace{ RootPath: "/", FilePath: "/foo/bar", + ResourcePath: "/foo/bar", ArtifactPath: "/foo/bar", StatePath: "/foo/bar", }, @@ -40,6 +42,7 @@ func TestDefineDefaultWorkspacePathsAlreadySet(t *testing.T) { diags := bundle.Apply(context.Background(), b, mutator.DefineDefaultWorkspacePaths()) require.NoError(t, diags.Error()) assert.Equal(t, "/foo/bar", b.Config.Workspace.FilePath) + assert.Equal(t, "/foo/bar", b.Config.Workspace.ResourcePath) assert.Equal(t, "/foo/bar", b.Config.Workspace.ArtifactPath) assert.Equal(t, "/foo/bar", b.Config.Workspace.StatePath) } diff --git a/bundle/config/mutator/process_target_mode.go b/bundle/config/mutator/process_target_mode.go index 70382f054b..9944f6ffd0 100644 --- a/bundle/config/mutator/process_target_mode.go +++ b/bundle/config/mutator/process_target_mode.go @@ -118,15 +118,18 @@ func findNonUserPath(b *bundle.Bundle) string { if b.Config.Workspace.RootPath != "" && !containsName(b.Config.Workspace.RootPath) { return "root_path" } - if b.Config.Workspace.StatePath != "" && !containsName(b.Config.Workspace.StatePath) { - return "state_path" - } if b.Config.Workspace.FilePath != "" && !containsName(b.Config.Workspace.FilePath) { return "file_path" } + if b.Config.Workspace.ResourcePath != "" && !containsName(b.Config.Workspace.ResourcePath) { + return "resource_path" + } if b.Config.Workspace.ArtifactPath != "" && !containsName(b.Config.Workspace.ArtifactPath) { return "artifact_path" } + if b.Config.Workspace.StatePath != "" && !containsName(b.Config.Workspace.StatePath) { + return "state_path" + } return "" } diff --git a/bundle/config/workspace.go b/bundle/config/workspace.go index efc5caa663..878d07838d 100644 --- a/bundle/config/workspace.go +++ b/bundle/config/workspace.go @@ -54,6 +54,11 @@ type Workspace struct { // This defaults to "${workspace.root}/files". FilePath string `json:"file_path,omitempty"` + // Remote workspace path for resources with a presence in the workspace. + // These are kept outside [FilePath] to avoid potential naming collisions. + // This defaults to "${workspace.root}/resources". + ResourcePath string `json:"resource_path,omitempty"` + // Remote workspace path for build artifacts. // This defaults to "${workspace.root}/artifacts". ArtifactPath string `json:"artifact_path,omitempty"` From 0f22ec61166dfba3b3ce3662c36d13a7096cd6c6 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 1 Oct 2024 04:51:50 -0700 Subject: [PATCH 07/39] Remove generate changes from this branch --- bundle/config/generate/dashboard.go | 19 -- cmd/bundle/generate.go | 1 - cmd/bundle/generate/dashboard.go | 281 -------------------------- cmd/bundle/generate/dashboard_test.go | 1 - 4 files changed, 302 deletions(-) delete mode 100644 bundle/config/generate/dashboard.go delete mode 100644 cmd/bundle/generate/dashboard.go delete mode 100644 cmd/bundle/generate/dashboard_test.go diff --git a/bundle/config/generate/dashboard.go b/bundle/config/generate/dashboard.go deleted file mode 100644 index 4f2c012db2..0000000000 --- a/bundle/config/generate/dashboard.go +++ /dev/null @@ -1,19 +0,0 @@ -package generate - -import ( - "github.com/databricks/cli/libs/dyn" - "github.com/databricks/databricks-sdk-go/service/dashboards" -) - -func ConvertDashboardToValue(dashboard *dashboards.Dashboard, filePath string) (dyn.Value, error) { - // The majority of fields of the dashboard struct are read-only. - // We copy the relevant fields manually. - dv := map[string]dyn.Value{ - "display_name": dyn.NewValue(dashboard.DisplayName, []dyn.Location{{Line: 1}}), - "parent_path": dyn.NewValue("${workspace.file_path}", []dyn.Location{{Line: 2}}), - "warehouse_id": dyn.NewValue(dashboard.WarehouseId, []dyn.Location{{Line: 3}}), - "definition_path": dyn.NewValue(filePath, []dyn.Location{{Line: 4}}), - } - - return dyn.V(dv), nil -} diff --git a/cmd/bundle/generate.go b/cmd/bundle/generate.go index 7dea19ff9d..1e3d56e430 100644 --- a/cmd/bundle/generate.go +++ b/cmd/bundle/generate.go @@ -16,7 +16,6 @@ func newGenerateCommand() *cobra.Command { cmd.AddCommand(generate.NewGenerateJobCommand()) cmd.AddCommand(generate.NewGeneratePipelineCommand()) - cmd.AddCommand(generate.NewGenerateDashboardCommand()) cmd.PersistentFlags().StringVar(&key, "key", "", `resource key to use for the generated configuration`) return cmd } diff --git a/cmd/bundle/generate/dashboard.go b/cmd/bundle/generate/dashboard.go deleted file mode 100644 index 12a23b3f5b..0000000000 --- a/cmd/bundle/generate/dashboard.go +++ /dev/null @@ -1,281 +0,0 @@ -package generate - -import ( - "context" - "encoding/json" - "fmt" - "os" - "path" - "path/filepath" - "strings" - "time" - - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config/generate" - "github.com/databricks/cli/bundle/deploy/terraform" - "github.com/databricks/cli/bundle/phases" - "github.com/databricks/cli/cmd/root" - "github.com/databricks/cli/libs/diag" - "github.com/databricks/cli/libs/dyn" - "github.com/databricks/cli/libs/dyn/yamlsaver" - "github.com/databricks/cli/libs/textutil" - "github.com/databricks/databricks-sdk-go" - "github.com/databricks/databricks-sdk-go/service/dashboards" - "github.com/databricks/databricks-sdk-go/service/workspace" - "github.com/spf13/cobra" - "gopkg.in/yaml.v3" -) - -type dashboard struct { - resourceDir string - dashboardDir string - force bool - - // Relative path from the resource directory to the dashboard directory. - relativeDir string - - existingDashboardPath string - existingDashboardId string - watch string - - key string -} - -func (d *dashboard) resolveDashboardID(ctx context.Context, w *databricks.WorkspaceClient) diag.Diagnostics { - if d.existingDashboardPath == "" { - return nil - } - - obj, err := w.Workspace.GetStatusByPath(ctx, d.existingDashboardPath) - if err != nil { - return diag.FromErr(err) - } - - if obj.ObjectType != workspace.ObjectTypeDashboard { - found := strings.ToLower(obj.ObjectType.String()) - return diag.Diagnostics{ - { - Severity: diag.Error, - Summary: fmt.Sprintf("expected a dashboard, found a %s", found), - Locations: []dyn.Location{{ - File: d.existingDashboardPath, - }}, - }, - } - } - - if obj.ResourceId == "" { - return diag.Diagnostics{ - { - Severity: diag.Error, - Summary: "expected resource ID to be set", - Locations: []dyn.Location{{ - File: d.existingDashboardPath, - }}, - }, - } - } - - d.existingDashboardId = obj.ResourceId - return nil -} - -func (d *dashboard) saveConfiguration(ctx context.Context, dashboard *dashboards.Dashboard) error { - // TODO: add flag - key := d.key - if key == "" { - key = textutil.NormalizeString(dashboard.DisplayName) - } - - dashboardFilePath := path.Join(d.relativeDir, fmt.Sprintf("%s.lvdash.json", key)) - v, err := generate.ConvertDashboardToValue(dashboard, dashboardFilePath) - if err != nil { - return err - } - - result := map[string]dyn.Value{ - "resources": dyn.V(map[string]dyn.Value{ - "dashboards": dyn.V(map[string]dyn.Value{ - key: v, - }), - }), - } - - // Make sure the output directory exists. - if err := os.MkdirAll(d.resourceDir, 0755); err != nil { - return err - } - - filename := filepath.Join(d.resourceDir, fmt.Sprintf("%s.yml", key)) - saver := yamlsaver.NewSaverWithStyle(map[string]yaml.Style{ - "display_name": yaml.DoubleQuotedStyle, - }) - err = saver.SaveAsYAML(result, filename, false) - if err != nil { - return err - } - - return nil -} - -func (d *dashboard) remarshal(data []byte) ([]byte, error) { - var tmp any - var err error - err = json.Unmarshal(data, &tmp) - if err != nil { - return nil, err - } - out, err := json.MarshalIndent(tmp, "", " ") - if err != nil { - return nil, err - } - return out, nil -} - -func (d *dashboard) saveSerializedDashboard(ctx context.Context, dashboard *dashboards.Dashboard, dst string) error { - // Unmarshal and remarshal the serialized dashboard to ensure it is formatted correctly. - // The result will have alphabetically sorted keys and be indented. - data, err := d.remarshal([]byte(dashboard.SerializedDashboard)) - if err != nil { - return err - } - - // Make sure the output directory exists. - if err := os.MkdirAll(filepath.Dir(dst), 0755); err != nil { - return err - } - - return os.WriteFile(dst, data, 0644) -} - -func (d *dashboard) watchForChanges(ctx context.Context, b *bundle.Bundle) error { - diags := bundle.Apply(ctx, b, bundle.Seq( - phases.Initialize(), - terraform.Interpolate(), - terraform.Write(), - terraform.StatePull(), - terraform.Load(terraform.ErrorOnEmptyState), - )) - if err := diags.Error(); err != nil { - return err - } - - dash, ok := b.Config.Resources.Dashboards[d.watch] - if !ok { - return fmt.Errorf("dashboard %s not found", d.watch) - } - - // fmt.Println(dash.DefinitionPath) - - w := b.WorkspaceClient() - etag := "" - - cwd, err := os.Getwd() - if err != nil { - return err - } - - relPath, err := filepath.Rel(cwd, dash.FilePath) - if err != nil { - return err - } - - for { - dashboard, err := w.Lakeview.GetByDashboardId(ctx, dash.ID) - if err != nil { - return err - } - - // fmt.Println(dashboard.Path) - // fmt.Println(dashboard.Etag) - // fmt.Println(dashboard.UpdateTime) - - // obj, err := w.Workspace.GetStatusByPath(ctx, "/Users/pieter.noordhuis@databricks.com/.bundle/dashboard-eng-work-generate/dev/files/[dev pieter_noordhuis] NYC Taxi Trip Analysis.lvdash.json") - // if err != nil { - // return err - // } - - // fmt.Println(obj.ModifiedAt) - - if etag != dashboard.Etag { - fmt.Printf("[%s]: Updating dashboard at %s\n", dashboard.UpdateTime, relPath) - d.saveSerializedDashboard(ctx, dashboard, dash.FilePath) - } - - etag = dashboard.Etag - time.Sleep(1 * time.Second) - } -} - -func (d *dashboard) RunE(cmd *cobra.Command, args []string) error { - ctx := cmd.Context() - b, diags := root.MustConfigureBundle(cmd) - if err := diags.Error(); err != nil { - return diags.Error() - } - - // Make sure we know how the dashboard path is relative to the resource path. - rel, err := filepath.Rel(d.resourceDir, d.dashboardDir) - if err != nil { - return err - } - - d.relativeDir = filepath.ToSlash(rel) - - w := b.WorkspaceClient() - - if d.watch != "" { - return d.watchForChanges(ctx, b) - } - - // Lookup the dashboard ID if the path is given - diags = d.resolveDashboardID(ctx, w) - if diags.HasError() { - return diags.Error() - } - - dashboard, err := w.Lakeview.GetByDashboardId(ctx, d.existingDashboardId) - if err != nil { - return err - } - - d.saveConfiguration(ctx, dashboard) - - // TODO: add flag - key := d.key - if key == "" { - key = textutil.NormalizeString(dashboard.DisplayName) - } - - filename := filepath.Join(d.dashboardDir, fmt.Sprintf("%s.lvdash.json", key)) - d.saveSerializedDashboard(ctx, dashboard, filename) - return nil -} - -func NewGenerateDashboardCommand() *cobra.Command { - cmd := &cobra.Command{ - Use: "dashboard", - Short: "Generate configuration for a dashboard", - } - - d := &dashboard{} - - cmd.Flags().StringVarP(&d.resourceDir, "resource-dir", "d", "./resources", `directory to write the configuration to`) - cmd.Flags().StringVarP(&d.dashboardDir, "dashboard-dir", "s", "./dashboards", `directory to write the dashboard representation to`) - cmd.Flags().BoolVarP(&d.force, "force", "f", false, `force overwrite existing files in the output directory`) - - // Specify dashboard by workspace path - - cmd.Flags().StringVar(&d.existingDashboardPath, "existing-dashboard-path", "", `workspace path of the dashboard to generate configuration for`) - cmd.Flags().StringVar(&d.existingDashboardId, "existing-dashboard-id", "", `ID of the dashboard to generate configuration for`) - cmd.Flags().StringVar(&d.watch, "watch-resource", "", `resource key of dashboard to watch for changes`) - - cmd.MarkFlagsOneRequired( - "existing-dashboard-path", - "existing-dashboard-id", - "watch-resource", - ) - - cmd.RunE = d.RunE - return cmd -} diff --git a/cmd/bundle/generate/dashboard_test.go b/cmd/bundle/generate/dashboard_test.go deleted file mode 100644 index eb8347795a..0000000000 --- a/cmd/bundle/generate/dashboard_test.go +++ /dev/null @@ -1 +0,0 @@ -package generate From 0e0f4217ecf64be52e4860585c48198e9bd998a1 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 1 Oct 2024 04:55:51 -0700 Subject: [PATCH 08/39] Remove examples --- tmp/dashboard-dab/README.md | 6 - tmp/dashboard-dab/dashboard.lvdash.json | 34 - tmp/dashboard-dab/databricks.yml | 36 - .../nyc_taxi_trip_analysis.lvdash.json | 710 ------------------ tmp/dashboard-generate/databricks.yml | 16 - .../resources/nyc_taxi_trip_analysis.yml | 23 - 6 files changed, 825 deletions(-) delete mode 100644 tmp/dashboard-dab/README.md delete mode 100644 tmp/dashboard-dab/dashboard.lvdash.json delete mode 100644 tmp/dashboard-dab/databricks.yml delete mode 100644 tmp/dashboard-generate/dashboards/nyc_taxi_trip_analysis.lvdash.json delete mode 100644 tmp/dashboard-generate/databricks.yml delete mode 100644 tmp/dashboard-generate/resources/nyc_taxi_trip_analysis.yml diff --git a/tmp/dashboard-dab/README.md b/tmp/dashboard-dab/README.md deleted file mode 100644 index 572756e86f..0000000000 --- a/tmp/dashboard-dab/README.md +++ /dev/null @@ -1,6 +0,0 @@ -# Iterating - - -``` -databricks lakeview get 01ef69c6a1b61c85a97505155d58015e --output json | jq -r .serialized_dashboard | jq -S . > dashboard.lvdash.json -``` diff --git a/tmp/dashboard-dab/dashboard.lvdash.json b/tmp/dashboard-dab/dashboard.lvdash.json deleted file mode 100644 index ea590b5eee..0000000000 --- a/tmp/dashboard-dab/dashboard.lvdash.json +++ /dev/null @@ -1,34 +0,0 @@ -{ - "pages": [ - { - "displayName": "New Page", - "layout": [ - { - "position": { - "height": 2, - "width": 6, - "x": 0, - "y": 0 - }, - "widget": { - "name": "82eb9107", - "textbox_spec": "# hi another new foobar change! if I change this remotely" - } - }, - { - "position": { - "height": 2, - "width": 6, - "x": 0, - "y": 2 - }, - "widget": { - "name": "ffa6de4f", - "textbox_spec": "another widget" - } - } - ], - "name": "fdd21a3c" - } - ] -} diff --git a/tmp/dashboard-dab/databricks.yml b/tmp/dashboard-dab/databricks.yml deleted file mode 100644 index 70a6d15c4e..0000000000 --- a/tmp/dashboard-dab/databricks.yml +++ /dev/null @@ -1,36 +0,0 @@ -bundle: - name: dashboard-eng-work - -workspace: - host: https://e2-dogfood.staging.cloud.databricks.com - -variables: - # 0 - Shared SQL Warehouse (ID: dd43ee29fedd958d) - warehouse_id: - default: dd43ee29fedd958d - -permissions: - - group_name: users - level: CAN_VIEW - -resources: - dashboards: - my_special_dashboard: - # TODO: - # * rename display_name to just "name" - # * remove parent_path, optionally let it be specified as part of "name", - # just like we do for mlflow experiments. - # * default the parent_path to ${workspace.resource_path}. - display_name: "Foobar" - parent_path: ${workspace.file_path} - warehouse_id: ${var.warehouse_id} - definition_path: ./dashboard.lvdash.json - - - # # file_path: ./dashboard.lvdash.json - - # catalog: ${var.default_catalog} - # schema: ${var.default_schema} - - -#https://e2-dogfood.staging.cloud.databricks.com/dashboardsv3/01ef692961381515beac094aa0a82cd5/published?o=6051921418418893 diff --git a/tmp/dashboard-generate/dashboards/nyc_taxi_trip_analysis.lvdash.json b/tmp/dashboard-generate/dashboards/nyc_taxi_trip_analysis.lvdash.json deleted file mode 100644 index 162535efd3..0000000000 --- a/tmp/dashboard-generate/dashboards/nyc_taxi_trip_analysis.lvdash.json +++ /dev/null @@ -1,710 +0,0 @@ -{ - "datasets": [ - { - "displayName": "route revenue", - "name": "fdefd57c", - "query": "SELECT\n T.pickup_zip,\n T.dropoff_zip,\n T.route as `Route`,\n T.frequency as `Number Trips`,\n T.total_fare as `Total Revenue`\nFROM\n (\n SELECT\n pickup_zip,\n dropoff_zip,\n concat(pickup_zip, '-', dropoff_zip) AS route,\n count(*) as frequency,\n SUM(fare_amount) as total_fare\n FROM\n `samples`.`nyctaxi`.`trips`\n GROUP BY\n 1,2,3\n ) T\nORDER BY\n 1 ASC" - }, - { - "displayName": "trips", - "name": "ecfcdc7c", - "query": "SELECT\n T.tpep_pickup_datetime,\n T.tpep_dropoff_datetime,\n T.fare_amount,\n T.pickup_zip,\n T.dropoff_zip,\n T.trip_distance,\n T.weekday,\n CASE\n WHEN T.weekday = 1 THEN 'Sunday'\n WHEN T.weekday = 2 THEN 'Monday'\n WHEN T.weekday = 3 THEN 'Tuesday'\n WHEN T.weekday = 4 THEN 'Wednesday'\n WHEN T.weekday = 5 THEN 'Thursday'\n WHEN T.weekday = 6 THEN 'Friday'\n WHEN T.weekday = 7 THEN 'Saturday'\n ELSE 'N/A'\n END AS day_of_week, \n T.fare_amount, \n T.trip_distance\nFROM\n (\n SELECT\n dayofweek(tpep_pickup_datetime) as weekday,\n *\n FROM\n `samples`.`nyctaxi`.`trips`\n WHERE\n trip_distance \u003e 0\n AND trip_distance \u003c 10\n AND fare_amount \u003e 0\n AND fare_amount \u003c 50\n ) T\nORDER BY\n T.weekday " - } - ], - "pages": [ - { - "displayName": "New Page", - "layout": [ - { - "position": { - "height": 1, - "width": 2, - "x": 0, - "y": 1 - }, - "widget": { - "name": "c4d87efe", - "queries": [ - { - "name": "dashboards/01ee564285a315dd80d473e76171660a/datasets/01ee564285a51daf810a8ffc5051bfee_tpep_dropoff_datetime", - "query": { - "datasetName": "ecfcdc7c", - "disaggregated": false, - "fields": [ - { - "expression": "`tpep_dropoff_datetime`", - "name": "tpep_dropoff_datetime" - }, - { - "expression": "COUNT_IF(`associative_filter_predicate_group`)", - "name": "tpep_dropoff_datetime_associativity" - } - ] - } - } - ], - "spec": { - "encodings": { - "fields": [ - { - "displayName": "tpep_dropoff_datetime", - "fieldName": "tpep_dropoff_datetime", - "queryName": "dashboards/01ee564285a315dd80d473e76171660a/datasets/01ee564285a51daf810a8ffc5051bfee_tpep_dropoff_datetime" - } - ] - }, - "frame": { - "showTitle": true, - "title": "Time Range" - }, - "version": 2, - "widgetType": "filter-date-range-picker" - } - } - }, - { - "position": { - "height": 4, - "width": 3, - "x": 0, - "y": 10 - }, - "widget": { - "name": "61e19e9c", - "queries": [ - { - "name": "main_query", - "query": { - "datasetName": "ecfcdc7c", - "disaggregated": false, - "fields": [ - { - "expression": "COUNT(`*`)", - "name": "count(*)" - }, - { - "expression": "DATE_TRUNC(\"HOUR\", `tpep_pickup_datetime`)", - "name": "hourly(tpep_pickup_datetime)" - } - ] - } - } - ], - "spec": { - "encodings": { - "label": { - "show": false - }, - "x": { - "axis": { - "title": "Pickup Hour" - }, - "displayName": "Pickup Hour", - "fieldName": "hourly(tpep_pickup_datetime)", - "scale": { - "type": "temporal" - } - }, - "y": { - "axis": { - "title": "Number of Rides" - }, - "displayName": "Number of Rides", - "fieldName": "count(*)", - "scale": { - "type": "quantitative" - } - } - }, - "frame": { - "showTitle": true, - "title": "Pickup Hour Distribution" - }, - "mark": { - "colors": [ - "#077A9D", - "#FFAB00", - "#00A972", - "#FF3621", - "#8BCAE7", - "#AB4057", - "#99DDB4", - "#FCA4A1", - "#919191", - "#BF7080" - ] - }, - "version": 3, - "widgetType": "bar" - } - } - }, - { - "position": { - "height": 8, - "width": 4, - "x": 2, - "y": 2 - }, - "widget": { - "name": "3b1dff20", - "queries": [ - { - "name": "main_query", - "query": { - "datasetName": "ecfcdc7c", - "disaggregated": true, - "fields": [ - { - "expression": "`day_of_week`", - "name": "day_of_week" - }, - { - "expression": "`fare_amount`", - "name": "fare_amount" - }, - { - "expression": "`trip_distance`", - "name": "trip_distance" - } - ] - } - } - ], - "spec": { - "encodings": { - "color": { - "displayName": "Day of Week", - "fieldName": "day_of_week", - "scale": { - "type": "categorical" - } - }, - "x": { - "axis": { - "title": "Trip Distance (miles)" - }, - "displayName": "trip_distance", - "fieldName": "trip_distance", - "scale": { - "type": "quantitative" - } - }, - "y": { - "axis": { - "title": "Fare Amount (USD)" - }, - "displayName": "fare_amount", - "fieldName": "fare_amount", - "scale": { - "type": "quantitative" - } - } - }, - "frame": { - "showTitle": true, - "title": "Daily Fare Trends by Day of Week" - }, - "version": 3, - "widgetType": "scatter" - } - } - }, - { - "position": { - "height": 1, - "width": 6, - "x": 0, - "y": 0 - }, - "widget": { - "name": "bd82f575", - "textbox_spec": "# NYC Taxi Trip Analysis" - } - }, - { - "position": { - "height": 4, - "width": 3, - "x": 3, - "y": 10 - }, - "widget": { - "name": "e7b33e79", - "queries": [ - { - "name": "main_query", - "query": { - "datasetName": "ecfcdc7c", - "disaggregated": false, - "fields": [ - { - "expression": "COUNT(`*`)", - "name": "count(*)" - }, - { - "expression": "DATE_TRUNC(\"HOUR\", `tpep_dropoff_datetime`)", - "name": "hourly(tpep_dropoff_datetime)" - } - ] - } - } - ], - "spec": { - "encodings": { - "x": { - "axis": { - "title": "Dropoff Hour" - }, - "displayName": "Dropoff Hour", - "fieldName": "hourly(tpep_dropoff_datetime)", - "scale": { - "type": "temporal" - } - }, - "y": { - "axis": { - "title": "Number of Rides" - }, - "displayName": "Number of Rides", - "fieldName": "count(*)", - "scale": { - "type": "quantitative" - } - } - }, - "frame": { - "showTitle": true, - "title": "Dropoff Hour Distribution" - }, - "mark": { - "colors": [ - "#FFAB00", - "#FFAB00", - "#00A972", - "#FF3621", - "#8BCAE7", - "#AB4057", - "#99DDB4", - "#FCA4A1", - "#919191", - "#BF7080" - ] - }, - "version": 3, - "widgetType": "bar" - } - } - }, - { - "position": { - "height": 2, - "width": 2, - "x": 0, - "y": 2 - }, - "widget": { - "name": "299e756c", - "queries": [ - { - "name": "main_query", - "query": { - "datasetName": "ecfcdc7c", - "disaggregated": false, - "fields": [ - { - "expression": "COUNT(`*`)", - "name": "count(*)" - } - ] - } - } - ], - "spec": { - "encodings": { - "value": { - "displayName": "Count of Records", - "fieldName": "count(*)", - "style": { - "bold": true, - "color": "#E92828" - } - } - }, - "frame": { - "showTitle": true, - "title": "Total Trips" - }, - "version": 2, - "widgetType": "counter" - } - } - }, - { - "position": { - "height": 1, - "width": 2, - "x": 2, - "y": 1 - }, - "widget": { - "name": "61a54236", - "queries": [ - { - "name": "dashboards/01eed0e4082a1c7e903cac7e74114376/datasets/01eed0e4082d1205adc131b86b10198e_pickup_zip", - "query": { - "datasetName": "fdefd57c", - "disaggregated": false, - "fields": [ - { - "expression": "`pickup_zip`", - "name": "pickup_zip" - }, - { - "expression": "COUNT_IF(`associative_filter_predicate_group`)", - "name": "pickup_zip_associativity" - } - ] - } - }, - { - "name": "dashboards/01eed0e4082a1c7e903cac7e74114376/datasets/01eed0e4082e1ff49c3209776820e82e_pickup_zip", - "query": { - "datasetName": "ecfcdc7c", - "disaggregated": false, - "fields": [ - { - "expression": "`pickup_zip`", - "name": "pickup_zip" - }, - { - "expression": "COUNT_IF(`associative_filter_predicate_group`)", - "name": "pickup_zip_associativity" - } - ] - } - } - ], - "spec": { - "encodings": { - "fields": [ - { - "displayName": "pickup_zip", - "fieldName": "pickup_zip", - "queryName": "dashboards/01eed0e4082a1c7e903cac7e74114376/datasets/01eed0e4082e1ff49c3209776820e82e_pickup_zip" - }, - { - "displayName": "pickup_zip", - "fieldName": "pickup_zip", - "queryName": "dashboards/01eed0e4082a1c7e903cac7e74114376/datasets/01eed0e4082d1205adc131b86b10198e_pickup_zip" - } - ] - }, - "frame": { - "showTitle": true, - "title": "Pickup Zip" - }, - "version": 2, - "widgetType": "filter-multi-select" - } - } - }, - { - "position": { - "height": 6, - "width": 2, - "x": 0, - "y": 4 - }, - "widget": { - "name": "985e7eb4", - "queries": [ - { - "name": "main_query", - "query": { - "datasetName": "fdefd57c", - "disaggregated": true, - "fields": [ - { - "expression": "`Number Trips`", - "name": "Number Trips" - }, - { - "expression": "`Route`", - "name": "Route" - }, - { - "expression": "`Total Revenue`", - "name": "Total Revenue" - } - ] - } - } - ], - "spec": { - "allowHTMLByDefault": false, - "condensed": true, - "encodings": { - "columns": [ - { - "alignContent": "left", - "allowHTML": false, - "allowSearch": false, - "booleanValues": [ - "false", - "true" - ], - "displayAs": "string", - "displayName": "Route", - "fieldName": "Route", - "highlightLinks": false, - "imageHeight": "", - "imageTitleTemplate": "{{ @ }}", - "imageUrlTemplate": "{{ @ }}", - "imageWidth": "", - "linkOpenInNewTab": true, - "linkTextTemplate": "{{ @ }}", - "linkTitleTemplate": "{{ @ }}", - "linkUrlTemplate": "{{ @ }}", - "order": 100000, - "preserveWhitespace": false, - "title": "Route", - "type": "string", - "useMonospaceFont": false, - "visible": true - }, - { - "alignContent": "right", - "allowHTML": false, - "allowSearch": false, - "booleanValues": [ - "false", - "true" - ], - "displayAs": "number", - "displayName": "Number Trips", - "fieldName": "Number Trips", - "highlightLinks": false, - "imageHeight": "", - "imageTitleTemplate": "{{ @ }}", - "imageUrlTemplate": "{{ @ }}", - "imageWidth": "", - "linkOpenInNewTab": true, - "linkTextTemplate": "{{ @ }}", - "linkTitleTemplate": "{{ @ }}", - "linkUrlTemplate": "{{ @ }}", - "numberFormat": "0", - "order": 100001, - "preserveWhitespace": false, - "title": "Number Trips", - "type": "integer", - "useMonospaceFont": false, - "visible": true - }, - { - "alignContent": "right", - "allowHTML": false, - "allowSearch": false, - "booleanValues": [ - "false", - "true" - ], - "cellFormat": { - "default": { - "foregroundColor": "#85CADE" - }, - "rules": [ - { - "if": { - "column": "Total Revenue", - "fn": "\u003c", - "literal": "51" - }, - "value": { - "foregroundColor": "#9C2638" - } - }, - { - "if": { - "column": "Total Revenue", - "fn": "\u003c", - "literal": "101" - }, - "value": { - "foregroundColor": "#FFD465" - } - }, - { - "if": { - "column": "Total Revenue", - "fn": "\u003c", - "literal": "6001" - }, - "value": { - "foregroundColor": "#1FA873" - } - } - ] - }, - "displayAs": "number", - "displayName": "Total Revenue", - "fieldName": "Total Revenue", - "highlightLinks": false, - "imageHeight": "", - "imageTitleTemplate": "{{ @ }}", - "imageUrlTemplate": "{{ @ }}", - "imageWidth": "", - "linkOpenInNewTab": true, - "linkTextTemplate": "{{ @ }}", - "linkTitleTemplate": "{{ @ }}", - "linkUrlTemplate": "{{ @ }}", - "numberFormat": "$0.00", - "order": 100002, - "preserveWhitespace": false, - "title": "Total Revenue", - "type": "float", - "useMonospaceFont": false, - "visible": true - } - ] - }, - "frame": { - "showTitle": true, - "title": "Route Revenue Attribution" - }, - "invisibleColumns": [ - { - "alignContent": "right", - "allowHTML": false, - "allowSearch": false, - "booleanValues": [ - "false", - "true" - ], - "displayAs": "number", - "highlightLinks": false, - "imageHeight": "", - "imageTitleTemplate": "{{ @ }}", - "imageUrlTemplate": "{{ @ }}", - "imageWidth": "", - "linkOpenInNewTab": true, - "linkTextTemplate": "{{ @ }}", - "linkTitleTemplate": "{{ @ }}", - "linkUrlTemplate": "{{ @ }}", - "name": "pickup_zip", - "numberFormat": "0", - "order": 100000, - "preserveWhitespace": false, - "title": "pickup_zip", - "type": "integer", - "useMonospaceFont": false - }, - { - "alignContent": "right", - "allowHTML": false, - "allowSearch": false, - "booleanValues": [ - "false", - "true" - ], - "displayAs": "number", - "highlightLinks": false, - "imageHeight": "", - "imageTitleTemplate": "{{ @ }}", - "imageUrlTemplate": "{{ @ }}", - "imageWidth": "", - "linkOpenInNewTab": true, - "linkTextTemplate": "{{ @ }}", - "linkTitleTemplate": "{{ @ }}", - "linkUrlTemplate": "{{ @ }}", - "name": "dropoff_zip", - "numberFormat": "0", - "order": 100001, - "preserveWhitespace": false, - "title": "dropoff_zip", - "type": "integer", - "useMonospaceFont": false - } - ], - "itemsPerPage": 25, - "paginationSize": "default", - "version": 1, - "widgetType": "table", - "withRowNumber": false - } - } - }, - { - "position": { - "height": 1, - "width": 2, - "x": 4, - "y": 1 - }, - "widget": { - "name": "b346c038", - "queries": [ - { - "name": "dashboards/01eed0e4082a1c7e903cac7e74114376/datasets/01eed0e4082d1205adc131b86b10198e_dropoff_zip", - "query": { - "datasetName": "fdefd57c", - "disaggregated": false, - "fields": [ - { - "expression": "`dropoff_zip`", - "name": "dropoff_zip" - }, - { - "expression": "COUNT_IF(`associative_filter_predicate_group`)", - "name": "dropoff_zip_associativity" - } - ] - } - }, - { - "name": "dashboards/01eed0e4082a1c7e903cac7e74114376/datasets/01eed0e4082e1ff49c3209776820e82e_dropoff_zip", - "query": { - "datasetName": "ecfcdc7c", - "disaggregated": false, - "fields": [ - { - "expression": "`dropoff_zip`", - "name": "dropoff_zip" - }, - { - "expression": "COUNT_IF(`associative_filter_predicate_group`)", - "name": "dropoff_zip_associativity" - } - ] - } - } - ], - "spec": { - "encodings": { - "fields": [ - { - "displayName": "dropoff_zip", - "fieldName": "dropoff_zip", - "queryName": "dashboards/01eed0e4082a1c7e903cac7e74114376/datasets/01eed0e4082e1ff49c3209776820e82e_dropoff_zip" - }, - { - "displayName": "dropoff_zip", - "fieldName": "dropoff_zip", - "queryName": "dashboards/01eed0e4082a1c7e903cac7e74114376/datasets/01eed0e4082d1205adc131b86b10198e_dropoff_zip" - } - ] - }, - "frame": { - "showTitle": true, - "title": "Dropoff Zip" - }, - "version": 2, - "widgetType": "filter-multi-select" - } - } - } - ], - "name": "b51b1363" - } - ] -} \ No newline at end of file diff --git a/tmp/dashboard-generate/databricks.yml b/tmp/dashboard-generate/databricks.yml deleted file mode 100644 index 8670872cea..0000000000 --- a/tmp/dashboard-generate/databricks.yml +++ /dev/null @@ -1,16 +0,0 @@ -bundle: - name: dashboard-eng-work-generate - -workspace: - host: https://e2-dogfood.staging.cloud.databricks.com - -include: - - resources/*.yml - -permissions: - - group_name: users - level: CAN_VIEW - -targets: - dev: - mode: development diff --git a/tmp/dashboard-generate/resources/nyc_taxi_trip_analysis.yml b/tmp/dashboard-generate/resources/nyc_taxi_trip_analysis.yml deleted file mode 100644 index 9a76969a12..0000000000 --- a/tmp/dashboard-generate/resources/nyc_taxi_trip_analysis.yml +++ /dev/null @@ -1,23 +0,0 @@ -resources: - dashboards: - nyc_taxi_trip_analysis: - display_name: "NYC Taxi Trip Analysis" - parent_path: ${workspace.file_path} - warehouse_id: 4fe75792cd0d304c - definition_path: ../dashboards/nyc_taxi_trip_analysis.lvdash.json - - # To be implemented when ready in the product: - # - # catalog: ${var.default_catalog} - # schema: ${var.default_schema} - # schedules: - # - name: Daily - # # ... - permissions: - # Allow all users to view the dashboard - - group_name: users - level: CAN_READ - - # Allow all account users to view the dashboard - - group_name: account users - level: CAN_READ From c911f7922e91ab496c7c996b92b15238164793b7 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 1 Oct 2024 09:47:40 -0700 Subject: [PATCH 09/39] Move test --- internal/{test => }/dashboard_assumptions_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) rename internal/{test => }/dashboard_assumptions_test.go (95%) diff --git a/internal/test/dashboard_assumptions_test.go b/internal/dashboard_assumptions_test.go similarity index 95% rename from internal/test/dashboard_assumptions_test.go rename to internal/dashboard_assumptions_test.go index ddb3c5962d..cd6a3602c9 100644 --- a/internal/test/dashboard_assumptions_test.go +++ b/internal/dashboard_assumptions_test.go @@ -1,4 +1,4 @@ -package test +package internal import ( "encoding/base64" @@ -17,8 +17,8 @@ import ( ) // Verify that importing a dashboard through the Workspace API retains the identity of the underying resource, -// as well as properties exclusively accessible through the dashboards API. -func TestDashboardAssumptions_WorkspaceImport(t *testing.T) { +// as well as properties exclusively accessible through the dashboards API. +func TestAccDashboardAssumptions_WorkspaceImport(t *testing.T) { ctx, wt := acc.WorkspaceTest(t) t.Parallel() From a2a794e5fa592ffe5c4d13cf152328b4e7e06289 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 1 Oct 2024 10:46:53 -0700 Subject: [PATCH 10/39] Configure the default parent path for dashboards --- .../mutator/configure_default_parent_path.go | 62 +++++++++++++++++++ .../configure_default_parent_path_test.go | 56 +++++++++++++++++ bundle/internal/bundletest/mutate.go | 19 ++++++ bundle/phases/initialize.go | 1 + 4 files changed, 138 insertions(+) create mode 100644 bundle/config/mutator/configure_default_parent_path.go create mode 100644 bundle/config/mutator/configure_default_parent_path_test.go create mode 100644 bundle/internal/bundletest/mutate.go diff --git a/bundle/config/mutator/configure_default_parent_path.go b/bundle/config/mutator/configure_default_parent_path.go new file mode 100644 index 0000000000..691a637aaf --- /dev/null +++ b/bundle/config/mutator/configure_default_parent_path.go @@ -0,0 +1,62 @@ +package mutator + +import ( + "context" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" +) + +type configureDefaultParentPath struct{} + +func ConfigureDefaultParentPath() bundle.Mutator { + return &configureDefaultParentPath{} +} + +func (m *configureDefaultParentPath) Name() string { + return "ConfigureDefaultParentPath" +} + +func (m *configureDefaultParentPath) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + var diags diag.Diagnostics + + pattern := dyn.NewPattern( + dyn.Key("resources"), + dyn.Key("dashboards"), + dyn.AnyKey(), + ) + + // Default value for the parent path. + defaultValue := b.Config.Workspace.ResourcePath + + // Configure the default parent path for all dashboards. + err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + return dyn.MapByPattern(v, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + // Get the current parent path (if set). + f, err := dyn.Get(v, "parent_path") + switch { + case dyn.IsNoSuchKeyError(err): + // OK, we'll set the default value. + break + case dyn.IsCannotTraverseNilError(err): + // Cannot traverse the value, skip it. + return v, nil + default: + // Return the error. + return v, err + } + + // Configure the default value (if not set). + if !f.IsValid() { + f = dyn.V(defaultValue) + } + + // Set the parent path. + return dyn.Set(v, "parent_path", f) + }) + }) + + diags = diags.Extend(diag.FromErr(err)) + return diags +} diff --git a/bundle/config/mutator/configure_default_parent_path_test.go b/bundle/config/mutator/configure_default_parent_path_test.go new file mode 100644 index 0000000000..a9571c850a --- /dev/null +++ b/bundle/config/mutator/configure_default_parent_path_test.go @@ -0,0 +1,56 @@ +package mutator_test + +import ( + "context" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/internal/bundletest" + "github.com/databricks/cli/libs/dyn" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestConfigureDefaultParentPath(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + ResourcePath: "/foo/bar", + }, + Resources: config.Resources{ + Dashboards: map[string]*resources.Dashboard{ + "d1": { + // Empty string is skipped. + // See below for how it is set. + ParentPath: "", + }, + "d2": { + // Non-empty string is skipped. + ParentPath: "already-set", + }, + "d3": { + // No parent path set. + }, + "d4": nil, + }, + }, + }, + } + + // We can't set an empty string in the typed configuration. + // Do it on the dyn.Value directly. + bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) { + return dyn.Set(v, "resources.dashboards.d1.parent_path", dyn.V("")) + }) + + diags := bundle.Apply(context.Background(), b, mutator.ConfigureDefaultParentPath()) + require.NoError(t, diags.Error()) + + assert.Equal(t, "", b.Config.Resources.Dashboards["d1"].ParentPath) + assert.Equal(t, "already-set", b.Config.Resources.Dashboards["d2"].ParentPath) + assert.Equal(t, "/foo/bar", b.Config.Resources.Dashboards["d3"].ParentPath) + assert.Nil(t, b.Config.Resources.Dashboards["d4"]) +} diff --git a/bundle/internal/bundletest/mutate.go b/bundle/internal/bundletest/mutate.go new file mode 100644 index 0000000000..c30870dd78 --- /dev/null +++ b/bundle/internal/bundletest/mutate.go @@ -0,0 +1,19 @@ +package bundletest + +import ( + "context" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" + "github.com/stretchr/testify/require" +) + +func Mutate(t *testing.T, b *bundle.Bundle, f func(v dyn.Value) (dyn.Value, error)) { + bundle.ApplyFunc(context.Background(), b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + err := b.Config.Mutate(f) + require.NoError(t, err) + return nil + }) +} diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index 93ce61b258..206fe73c52 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -57,6 +57,7 @@ func Initialize() bundle.Mutator { ), mutator.SetRunAs(), mutator.OverrideCompute(), + mutator.ConfigureDefaultParentPath(), mutator.ProcessTargetMode(), mutator.ApplyPresets(), mutator.DefaultQueueing(), From 8186da8e67975d4d7a86e05ae5d943d4d096fb99 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 1 Oct 2024 10:47:08 -0700 Subject: [PATCH 11/39] Simplify --- .../mutator/translate_paths_dashboards.go | 44 +++++-------------- 1 file changed, 11 insertions(+), 33 deletions(-) diff --git a/bundle/config/mutator/translate_paths_dashboards.go b/bundle/config/mutator/translate_paths_dashboards.go index 2ead527c70..2dfe3ef9d0 100644 --- a/bundle/config/mutator/translate_paths_dashboards.go +++ b/bundle/config/mutator/translate_paths_dashboards.go @@ -6,45 +6,23 @@ import ( "github.com/databricks/cli/libs/dyn" ) -type dashboardRewritePattern struct { - pattern dyn.Pattern - fn rewriteFunc -} - -func (t *translateContext) dashboardRewritePatterns() []dashboardRewritePattern { - // Base pattern to match all dashboards. - base := dyn.NewPattern( +func (t *translateContext) applyDashboardTranslations(v dyn.Value) (dyn.Value, error) { + // Convert the `file_path` field to a local absolute path. + // Terraform will load the file at this path and use its contents for the dashboard contents. + pattern := dyn.NewPattern( dyn.Key("resources"), dyn.Key("dashboards"), dyn.AnyKey(), + dyn.Key("file_path"), ) - // Compile list of configuration paths to rewrite. - return []dashboardRewritePattern{ - { - base.Append(dyn.Key("file_path")), - t.retainLocalAbsoluteFilePath, - }, - } -} - -func (t *translateContext) applyDashboardTranslations(v dyn.Value) (dyn.Value, error) { - var err error - - for _, rewritePattern := range t.dashboardRewritePatterns() { - v, err = dyn.MapByPattern(v, rewritePattern.pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { - key := p[1].Key() - dir, err := v.Location().Directory() - if err != nil { - return dyn.InvalidValue, fmt.Errorf("unable to determine directory for dashboard %s: %w", key, err) - } - - return t.rewriteRelativeTo(p, v, rewritePattern.fn, dir, "") - }) + return dyn.MapByPattern(v, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + key := p[2].Key() + dir, err := v.Location().Directory() if err != nil { - return dyn.InvalidValue, err + return dyn.InvalidValue, fmt.Errorf("unable to determine directory for dashboard %s: %w", key, err) } - } - return v, nil + return t.rewriteRelativeTo(p, v, t.retainLocalAbsoluteFilePath, dir, "") + }) } From 0301854a43aa2dc0134582703cfd0a19f7145a09 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 1 Oct 2024 10:47:20 -0700 Subject: [PATCH 12/39] Comment --- bundle/config/resources/dashboard.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/config/resources/dashboard.go b/bundle/config/resources/dashboard.go index 93474e7a72..1e2f2ae759 100644 --- a/bundle/config/resources/dashboard.go +++ b/bundle/config/resources/dashboard.go @@ -24,7 +24,7 @@ type Dashboard struct { // WarehouseID is the ID of the SQL Warehouse used to run the dashboard's queries. WarehouseID string `json:"warehouse_id"` - // SerializedDashboard is the contents of the dashboard in serialized JSON form. + // SerializedDashboard holds the contents of the dashboard in serialized JSON form. // Note: its type is any and not string such that it can be inlined as YAML. // If it is not a string, its contents will be marshalled as JSON. SerializedDashboard any `json:"serialized_dashboard,omitempty"` From b63e94366a47b0504166d35c8bada529001b9509 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 1 Oct 2024 11:19:43 -0700 Subject: [PATCH 13/39] Rename mutator --- .../mutator/configure_dashboard_defaults.go | 70 ++++++++++ .../configure_dashboard_defaults_test.go | 125 ++++++++++++++++++ .../mutator/configure_default_parent_path.go | 62 --------- .../configure_default_parent_path_test.go | 56 -------- bundle/phases/initialize.go | 2 +- 5 files changed, 196 insertions(+), 119 deletions(-) create mode 100644 bundle/config/mutator/configure_dashboard_defaults.go create mode 100644 bundle/config/mutator/configure_dashboard_defaults_test.go delete mode 100644 bundle/config/mutator/configure_default_parent_path.go delete mode 100644 bundle/config/mutator/configure_default_parent_path_test.go diff --git a/bundle/config/mutator/configure_dashboard_defaults.go b/bundle/config/mutator/configure_dashboard_defaults.go new file mode 100644 index 0000000000..36ec279ded --- /dev/null +++ b/bundle/config/mutator/configure_dashboard_defaults.go @@ -0,0 +1,70 @@ +package mutator + +import ( + "context" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" +) + +type configureDashboardDefaults struct{} + +func ConfigureDashboardDefaults() bundle.Mutator { + return &configureDashboardDefaults{} +} + +func (m *configureDashboardDefaults) Name() string { + return "ConfigureDashboardDefaults" +} + +func (m *configureDashboardDefaults) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + var diags diag.Diagnostics + + pattern := dyn.NewPattern( + dyn.Key("resources"), + dyn.Key("dashboards"), + dyn.AnyKey(), + ) + + // Configure defaults for all dashboards. + err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + return dyn.MapByPattern(v, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + var err error + v, err = setIfNotExists(v, dyn.NewPath(dyn.Key("parent_path")), dyn.V(b.Config.Workspace.ResourcePath)) + if err != nil { + return dyn.InvalidValue, err + } + v, err = setIfNotExists(v, dyn.NewPath(dyn.Key("embed_credentials")), dyn.V(false)) + if err != nil { + return dyn.InvalidValue, err + } + return v, nil + }) + }) + + diags = diags.Extend(diag.FromErr(err)) + return diags +} + +func setIfNotExists(v dyn.Value, path dyn.Path, defaultValue dyn.Value) (dyn.Value, error) { + // Get the field at the specified path (if set). + _, err := dyn.GetByPath(v, path) + switch { + case dyn.IsNoSuchKeyError(err): + // OK, we'll set the default value. + break + case dyn.IsCannotTraverseNilError(err): + // Cannot traverse the value, skip it. + return v, nil + case err == nil: + // The field is set, skip it. + return v, nil + default: + // Return the error. + return v, err + } + + // Set the field at the specified path. + return dyn.SetByPath(v, path, defaultValue) +} diff --git a/bundle/config/mutator/configure_dashboard_defaults_test.go b/bundle/config/mutator/configure_dashboard_defaults_test.go new file mode 100644 index 0000000000..98b0e37e15 --- /dev/null +++ b/bundle/config/mutator/configure_dashboard_defaults_test.go @@ -0,0 +1,125 @@ +package mutator_test + +import ( + "context" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/internal/bundletest" + "github.com/databricks/cli/libs/dyn" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestConfigureDashboardDefaultsParentPath(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + ResourcePath: "/foo/bar", + }, + Resources: config.Resources{ + Dashboards: map[string]*resources.Dashboard{ + "d1": { + // Empty string is skipped. + // See below for how it is set. + ParentPath: "", + }, + "d2": { + // Non-empty string is skipped. + ParentPath: "already-set", + }, + "d3": { + // No parent path set. + }, + "d4": nil, + }, + }, + }, + } + + // We can't set an empty string in the typed configuration. + // Do it on the dyn.Value directly. + bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) { + return dyn.Set(v, "resources.dashboards.d1.parent_path", dyn.V("")) + }) + + diags := bundle.Apply(context.Background(), b, mutator.ConfigureDashboardDefaults()) + require.NoError(t, diags.Error()) + + var v dyn.Value + var err error + + // Set to empty string; unchanged. + v, err = dyn.Get(b.Config.Value(), "resources.dashboards.d1.parent_path") + if assert.NoError(t, err) { + assert.Equal(t, "", v.MustString()) + } + + // Set to "already-set"; unchanged. + v, err = dyn.Get(b.Config.Value(), "resources.dashboards.d2.parent_path") + if assert.NoError(t, err) { + assert.Equal(t, "already-set", v.MustString()) + } + + // Not set; now set to the workspace resource path. + v, err = dyn.Get(b.Config.Value(), "resources.dashboards.d3.parent_path") + if assert.NoError(t, err) { + assert.Equal(t, "/foo/bar", v.MustString()) + } + + // No valid dashboard; no change. + _, err = dyn.Get(b.Config.Value(), "resources.dashboards.d4.parent_path") + assert.True(t, dyn.IsCannotTraverseNilError(err)) +} + +func TestConfigureDashboardDefaultsEmbedCredentials(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Dashboards: map[string]*resources.Dashboard{ + "d1": { + EmbedCredentials: true, + }, + "d2": { + EmbedCredentials: false, + }, + "d3": { + // No parent path set. + }, + "d4": nil, + }, + }, + }, + } + + diags := bundle.Apply(context.Background(), b, mutator.ConfigureDashboardDefaults()) + require.NoError(t, diags.Error()) + + var v dyn.Value + var err error + + // Set to true; still true. + v, err = dyn.Get(b.Config.Value(), "resources.dashboards.d1.embed_credentials") + if assert.NoError(t, err) { + assert.Equal(t, true, v.MustBool()) + } + + // Set to false; still false. + v, err = dyn.Get(b.Config.Value(), "resources.dashboards.d2.embed_credentials") + if assert.NoError(t, err) { + assert.Equal(t, false, v.MustBool()) + } + + // Not set; now false. + v, err = dyn.Get(b.Config.Value(), "resources.dashboards.d3.embed_credentials") + if assert.NoError(t, err) { + assert.Equal(t, false, v.MustBool()) + } + + // No valid dashboard; no change. + _, err = dyn.Get(b.Config.Value(), "resources.dashboards.d4.embed_credentials") + assert.True(t, dyn.IsCannotTraverseNilError(err)) +} diff --git a/bundle/config/mutator/configure_default_parent_path.go b/bundle/config/mutator/configure_default_parent_path.go deleted file mode 100644 index 691a637aaf..0000000000 --- a/bundle/config/mutator/configure_default_parent_path.go +++ /dev/null @@ -1,62 +0,0 @@ -package mutator - -import ( - "context" - - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/libs/diag" - "github.com/databricks/cli/libs/dyn" -) - -type configureDefaultParentPath struct{} - -func ConfigureDefaultParentPath() bundle.Mutator { - return &configureDefaultParentPath{} -} - -func (m *configureDefaultParentPath) Name() string { - return "ConfigureDefaultParentPath" -} - -func (m *configureDefaultParentPath) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - var diags diag.Diagnostics - - pattern := dyn.NewPattern( - dyn.Key("resources"), - dyn.Key("dashboards"), - dyn.AnyKey(), - ) - - // Default value for the parent path. - defaultValue := b.Config.Workspace.ResourcePath - - // Configure the default parent path for all dashboards. - err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { - return dyn.MapByPattern(v, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { - // Get the current parent path (if set). - f, err := dyn.Get(v, "parent_path") - switch { - case dyn.IsNoSuchKeyError(err): - // OK, we'll set the default value. - break - case dyn.IsCannotTraverseNilError(err): - // Cannot traverse the value, skip it. - return v, nil - default: - // Return the error. - return v, err - } - - // Configure the default value (if not set). - if !f.IsValid() { - f = dyn.V(defaultValue) - } - - // Set the parent path. - return dyn.Set(v, "parent_path", f) - }) - }) - - diags = diags.Extend(diag.FromErr(err)) - return diags -} diff --git a/bundle/config/mutator/configure_default_parent_path_test.go b/bundle/config/mutator/configure_default_parent_path_test.go deleted file mode 100644 index a9571c850a..0000000000 --- a/bundle/config/mutator/configure_default_parent_path_test.go +++ /dev/null @@ -1,56 +0,0 @@ -package mutator_test - -import ( - "context" - "testing" - - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config" - "github.com/databricks/cli/bundle/config/mutator" - "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/bundle/internal/bundletest" - "github.com/databricks/cli/libs/dyn" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestConfigureDefaultParentPath(t *testing.T) { - b := &bundle.Bundle{ - Config: config.Root{ - Workspace: config.Workspace{ - ResourcePath: "/foo/bar", - }, - Resources: config.Resources{ - Dashboards: map[string]*resources.Dashboard{ - "d1": { - // Empty string is skipped. - // See below for how it is set. - ParentPath: "", - }, - "d2": { - // Non-empty string is skipped. - ParentPath: "already-set", - }, - "d3": { - // No parent path set. - }, - "d4": nil, - }, - }, - }, - } - - // We can't set an empty string in the typed configuration. - // Do it on the dyn.Value directly. - bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) { - return dyn.Set(v, "resources.dashboards.d1.parent_path", dyn.V("")) - }) - - diags := bundle.Apply(context.Background(), b, mutator.ConfigureDefaultParentPath()) - require.NoError(t, diags.Error()) - - assert.Equal(t, "", b.Config.Resources.Dashboards["d1"].ParentPath) - assert.Equal(t, "already-set", b.Config.Resources.Dashboards["d2"].ParentPath) - assert.Equal(t, "/foo/bar", b.Config.Resources.Dashboards["d3"].ParentPath) - assert.Nil(t, b.Config.Resources.Dashboards["d4"]) -} diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index 206fe73c52..1c1958a38c 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -57,7 +57,7 @@ func Initialize() bundle.Mutator { ), mutator.SetRunAs(), mutator.OverrideCompute(), - mutator.ConfigureDefaultParentPath(), + mutator.ConfigureDashboardDefaults(), mutator.ProcessTargetMode(), mutator.ApplyPresets(), mutator.DefaultQueueing(), From b54c10b832105da5a48685fa312cc90a59d9591d Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 1 Oct 2024 11:22:44 -0700 Subject: [PATCH 14/39] Remove embed_credentials default from tfdyn --- .../terraform/tfdyn/convert_dashboard.go | 9 ----- .../terraform/tfdyn/convert_dashboard_test.go | 35 ------------------- 2 files changed, 44 deletions(-) diff --git a/bundle/deploy/terraform/tfdyn/convert_dashboard.go b/bundle/deploy/terraform/tfdyn/convert_dashboard.go index 13be530b82..f905ba9996 100644 --- a/bundle/deploy/terraform/tfdyn/convert_dashboard.go +++ b/bundle/deploy/terraform/tfdyn/convert_dashboard.go @@ -46,15 +46,6 @@ func convertDashboardResource(ctx context.Context, vin dyn.Value) (dyn.Value, er } } - // Default the "embed_credentials" field to "false", if not already set. - // This is different from the behavior in the Terraform provider, so we make it explicit. - if _, ok := vout.Get("embed_credentials").AsBool(); !ok { - vout, err = dyn.Set(vout, "embed_credentials", dyn.V(false)) - if err != nil { - return dyn.InvalidValue, fmt.Errorf("failed to set embed_credentials: %w", err) - } - } - return vout, nil } diff --git a/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go b/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go index 886be16f1b..dfb2ffa44c 100644 --- a/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go +++ b/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go @@ -2,7 +2,6 @@ package tfdyn import ( "context" - "fmt" "testing" "github.com/databricks/cli/bundle/config/resources" @@ -79,37 +78,3 @@ func TestConvertDashboardFilePath(t *testing.T) { "file_path": "some/path", }) } - -func TestConvertDashboardEmbedCredentialsPassthrough(t *testing.T) { - for _, v := range []bool{true, false} { - t.Run(fmt.Sprintf("set to %v", v), func(t *testing.T) { - vin := dyn.V(map[string]dyn.Value{ - "embed_credentials": dyn.V(v), - }) - - ctx := context.Background() - out := schema.NewResources() - err := dashboardConverter{}.Convert(ctx, "my_dashboard", vin, out) - require.NoError(t, err) - - // Assert that the "embed_credentials" is set as configured. - assert.Subset(t, out.Dashboard["my_dashboard"], map[string]any{ - "embed_credentials": v, - }) - }) - } -} - -func TestConvertDashboardEmbedCredentialsDefault(t *testing.T) { - vin := dyn.V(map[string]dyn.Value{}) - - ctx := context.Background() - out := schema.NewResources() - err := dashboardConverter{}.Convert(ctx, "my_dashboard", vin, out) - require.NoError(t, err) - - // Assert that the "embed_credentials" is set to false (by default). - assert.Subset(t, out.Dashboard["my_dashboard"], map[string]any{ - "embed_credentials": false, - }) -} From 37b0039c1c041a89e0567678f9f327b897858c3b Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 1 Oct 2024 11:22:53 -0700 Subject: [PATCH 15/39] Never swallow errors --- bundle/internal/bundletest/mutate.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bundle/internal/bundletest/mutate.go b/bundle/internal/bundletest/mutate.go index c30870dd78..c0ac630cec 100644 --- a/bundle/internal/bundletest/mutate.go +++ b/bundle/internal/bundletest/mutate.go @@ -11,9 +11,10 @@ import ( ) func Mutate(t *testing.T, b *bundle.Bundle, f func(v dyn.Value) (dyn.Value, error)) { - bundle.ApplyFunc(context.Background(), b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + diags := bundle.ApplyFunc(context.Background(), b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { err := b.Config.Mutate(f) require.NoError(t, err) return nil }) + require.NoError(t, diags.Error()) } From 5fb35e358ff069aa51e446c114f32da6b3cfca34 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 18 Oct 2024 15:41:10 +0200 Subject: [PATCH 16/39] Add dashboards to summary output --- bundle/config/mutator/initialize_urls_test.go | 7 +++++ bundle/config/resources.go | 7 +++++ bundle/config/resources/dashboard.go | 30 +++++++++++++++---- 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/bundle/config/mutator/initialize_urls_test.go b/bundle/config/mutator/initialize_urls_test.go index 71cc153ab7..bfc0b56490 100644 --- a/bundle/config/mutator/initialize_urls_test.go +++ b/bundle/config/mutator/initialize_urls_test.go @@ -85,6 +85,12 @@ func TestInitializeURLs(t *testing.T) { }, }, }, + Dashboards: map[string]*resources.Dashboard{ + "dashboard1": { + ID: "01ef8d56871e1d50ae30ce7375e42478", + DisplayName: "My special dashboard", + }, + }, }, }, } @@ -99,6 +105,7 @@ func TestInitializeURLs(t *testing.T) { "qualityMonitor1": "https://mycompany.databricks.com/explore/data/catalog/schema/qualityMonitor1?o=123456", "schema1": "https://mycompany.databricks.com/explore/data/catalog/schema?o=123456", "cluster1": "https://mycompany.databricks.com/compute/clusters/1017-103929-vlr7jzcf?o=123456", + "dashboard1": "https://mycompany.databricks.com/dashboardsv3/01ef8d56871e1d50ae30ce7375e42478/published?o=123456", } initializeForWorkspace(b, "123456", "https://mycompany.databricks.com/") diff --git a/bundle/config/resources.go b/bundle/config/resources.go index c29ff11da2..0affb6ef01 100644 --- a/bundle/config/resources.go +++ b/bundle/config/resources.go @@ -78,6 +78,7 @@ func (r *Resources) AllResources() []ResourceGroup { collectResourceMap(descriptions["quality_monitors"], r.QualityMonitors), collectResourceMap(descriptions["schemas"], r.Schemas), collectResourceMap(descriptions["clusters"], r.Clusters), + collectResourceMap(descriptions["dashboards"], r.Dashboards), } } @@ -176,5 +177,11 @@ func SupportedResources() map[string]ResourceDescription { SingularTitle: "Cluster", PluralTitle: "Clusters", }, + "dashboards": { + SingularName: "dashboard", + PluralName: "dashboards", + SingularTitle: "Dashboard", + PluralTitle: "Dashboards", + }, } } diff --git a/bundle/config/resources/dashboard.go b/bundle/config/resources/dashboard.go index 1e2f2ae759..0eb104c6a0 100644 --- a/bundle/config/resources/dashboard.go +++ b/bundle/config/resources/dashboard.go @@ -2,6 +2,8 @@ package resources import ( "context" + "fmt" + "net/url" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go" @@ -13,6 +15,7 @@ type Dashboard struct { ID string `json:"id,omitempty" bundle:"readonly"` Permissions []Permission `json:"permissions,omitempty"` ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` + URL string `json:"url,omitempty" bundle:"internal"` // =========================== // === BEGIN OF API FIELDS === @@ -50,12 +53,12 @@ type Dashboard struct { FilePath string `json:"file_path,omitempty"` } -func (s *Dashboard) UnmarshalJSON(b []byte) error { - return marshal.Unmarshal(b, s) +func (r *Dashboard) UnmarshalJSON(b []byte) error { + return marshal.Unmarshal(b, r) } -func (s Dashboard) MarshalJSON() ([]byte, error) { - return marshal.Marshal(s) +func (r Dashboard) MarshalJSON() ([]byte, error) { + return marshal.Marshal(r) } func (*Dashboard) Exists(ctx context.Context, w *databricks.WorkspaceClient, id string) (bool, error) { @@ -63,7 +66,7 @@ func (*Dashboard) Exists(ctx context.Context, w *databricks.WorkspaceClient, id DashboardId: id, }) if err != nil { - log.Debugf(ctx, "Dashboard %s does not exist", id) + log.Debugf(ctx, "dashboard %s does not exist", id) return false, err } return true, nil @@ -72,3 +75,20 @@ func (*Dashboard) Exists(ctx context.Context, w *databricks.WorkspaceClient, id func (*Dashboard) TerraformResourceName() string { return "databricks_dashboard" } + +func (r *Dashboard) InitializeURL(baseURL url.URL) { + if r.ID == "" { + return + } + + baseURL.Path = fmt.Sprintf("dashboardsv3/%s/published", r.ID) + r.URL = baseURL.String() +} + +func (r *Dashboard) GetName() string { + return r.DisplayName +} + +func (r *Dashboard) GetURL() string { + return r.URL +} From 1b51c0cf0e29dbd7502d642bc5b85fe48c766014 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 18 Oct 2024 15:48:27 +0200 Subject: [PATCH 17/39] Update run_as tests for dashboards --- bundle/config/mutator/run_as.go | 10 ++++++++++ bundle/config/mutator/run_as_test.go | 2 ++ 2 files changed, 12 insertions(+) diff --git a/bundle/config/mutator/run_as.go b/bundle/config/mutator/run_as.go index 6b3069d44e..0ca71e28e6 100644 --- a/bundle/config/mutator/run_as.go +++ b/bundle/config/mutator/run_as.go @@ -110,6 +110,16 @@ func validateRunAs(b *bundle.Bundle) diag.Diagnostics { )) } + // Dashboards do not support run_as in the API. + if len(b.Config.Resources.Dashboards) > 0 { + diags = diags.Extend(reportRunAsNotSupported( + "dashboards", + b.Config.GetLocation("resources.dashboards"), + b.Config.Workspace.CurrentUser.UserName, + identity, + )) + } + return diags } diff --git a/bundle/config/mutator/run_as_test.go b/bundle/config/mutator/run_as_test.go index 8076b82f19..acb6c3a43f 100644 --- a/bundle/config/mutator/run_as_test.go +++ b/bundle/config/mutator/run_as_test.go @@ -33,6 +33,7 @@ func allResourceTypes(t *testing.T) []string { // also update this check when adding a new resource require.Equal(t, []string{ "clusters", + "dashboards", "experiments", "jobs", "model_serving_endpoints", @@ -188,6 +189,7 @@ func TestRunAsErrorForUnsupportedResources(t *testing.T) { Config: *r, } diags := bundle.Apply(context.Background(), b, SetRunAs()) + require.Error(t, diags.Error()) assert.Contains(t, diags.Error().Error(), "do not support a setting a run_as user that is different from the owner.\n"+ "Current identity: alice. Run as identity: bob.\n"+ "See https://docs.databricks.com/dev-tools/bundles/run-as.html to learn more about the run_as property.", rt) From 379d26390733ebeb1b21f245c08ea46f7d6e51a1 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 18 Oct 2024 16:21:22 +0200 Subject: [PATCH 18/39] Undo changes to convert_test.go --- bundle/deploy/terraform/convert_test.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/bundle/deploy/terraform/convert_test.go b/bundle/deploy/terraform/convert_test.go index cc5073423b..4c6866d9d8 100644 --- a/bundle/deploy/terraform/convert_test.go +++ b/bundle/deploy/terraform/convert_test.go @@ -58,12 +58,9 @@ func TestBundleToTerraformJob(t *testing.T) { }, } - vin, err := convert.FromTyped(config, dyn.NilValue) - require.NoError(t, err) - out, err := BundleToTerraformWithDynValue(context.Background(), vin) - require.NoError(t, err) - + out := BundleToTerraform(&config) resource := out.Resource.Job["my_job"].(*schema.ResourceJob) + assert.Equal(t, "my job", resource.Name) assert.Len(t, resource.JobCluster, 1) assert.Equal(t, "https://github.com/foo/bar", resource.GitSource.Url) From 3201821a628fb9e31645139d5fb111b7c38c33c4 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 18 Oct 2024 16:21:39 +0200 Subject: [PATCH 19/39] Don't double-index --- bundle/config/mutator/apply_presets.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/bundle/config/mutator/apply_presets.go b/bundle/config/mutator/apply_presets.go index e235413091..3d1f3d766b 100644 --- a/bundle/config/mutator/apply_presets.go +++ b/bundle/config/mutator/apply_presets.go @@ -213,8 +213,12 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos } // Dashboards: Prefix - for i := range r.Dashboards { - r.Dashboards[i].DisplayName = prefix + r.Dashboards[i].DisplayName + for key, dashboard := range r.Dashboards { + if dashboard == nil { + diags = diags.Extend(diag.Errorf("dashboard %s s is not defined", key)) + continue + } + dashboard.DisplayName = prefix + dashboard.DisplayName } return diags From e13fae7a1d57530bb4901b6d51c9c3d296c17ab3 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 18 Oct 2024 16:21:52 +0200 Subject: [PATCH 20/39] Update comment --- bundle/config/mutator/translate_paths_dashboards.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/config/mutator/translate_paths_dashboards.go b/bundle/config/mutator/translate_paths_dashboards.go index 2dfe3ef9d0..93822a5991 100644 --- a/bundle/config/mutator/translate_paths_dashboards.go +++ b/bundle/config/mutator/translate_paths_dashboards.go @@ -8,7 +8,7 @@ import ( func (t *translateContext) applyDashboardTranslations(v dyn.Value) (dyn.Value, error) { // Convert the `file_path` field to a local absolute path. - // Terraform will load the file at this path and use its contents for the dashboard contents. + // We load the file at this path and use its contents for the dashboard contents. pattern := dyn.NewPattern( dyn.Key("resources"), dyn.Key("dashboards"), From 25a151cd60326eb5a139c28dfe31cba6f37d75fd Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 18 Oct 2024 16:22:07 +0200 Subject: [PATCH 21/39] Include dashboards in Terraform conversion logic --- bundle/deploy/terraform/convert.go | 5 +++ bundle/deploy/terraform/convert_test.go | 50 +++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/bundle/deploy/terraform/convert.go b/bundle/deploy/terraform/convert.go index 86ebf9f698..f413e363ea 100644 --- a/bundle/deploy/terraform/convert.go +++ b/bundle/deploy/terraform/convert.go @@ -480,6 +480,11 @@ func TerraformToBundle(state *resourcesState, config *config.Root) error { src.ModifiedStatus = resources.ModifiedStatusCreated } } + for _, src := range config.Resources.Dashboards { + if src.ModifiedStatus == "" && src.ID == "" { + src.ModifiedStatus = resources.ModifiedStatusCreated + } + } return nil } diff --git a/bundle/deploy/terraform/convert_test.go b/bundle/deploy/terraform/convert_test.go index 4c6866d9d8..4aeb5efe09 100644 --- a/bundle/deploy/terraform/convert_test.go +++ b/bundle/deploy/terraform/convert_test.go @@ -671,6 +671,14 @@ func TestTerraformToBundleEmptyLocalResources(t *testing.T) { {Attributes: stateInstanceAttributes{ID: "1"}}, }, }, + { + Type: "databricks_dashboard", + Mode: "managed", + Name: "test_dashboard", + Instances: []stateResourceInstance{ + {Attributes: stateInstanceAttributes{ID: "1"}}, + }, + }, }, } err := TerraformToBundle(&tfState, &config) @@ -703,6 +711,9 @@ func TestTerraformToBundleEmptyLocalResources(t *testing.T) { assert.Equal(t, "1", config.Resources.Clusters["test_cluster"].ID) assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Clusters["test_cluster"].ModifiedStatus) + assert.Equal(t, "1", config.Resources.Dashboards["test_dashboard"].ID) + assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Dashboards["test_dashboard"].ModifiedStatus) + AssertFullResourceCoverage(t, &config) } @@ -772,6 +783,11 @@ func TestTerraformToBundleEmptyRemoteResources(t *testing.T) { }, }, }, + Dashboards: map[string]*resources.Dashboard{ + "test_dashboard": { + DisplayName: "test_dashboard", + }, + }, }, } var tfState = resourcesState{ @@ -807,6 +823,9 @@ func TestTerraformToBundleEmptyRemoteResources(t *testing.T) { assert.Equal(t, "", config.Resources.Clusters["test_cluster"].ID) assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Clusters["test_cluster"].ModifiedStatus) + assert.Equal(t, "", config.Resources.Dashboards["test_dashboard"].ID) + assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Dashboards["test_dashboard"].ModifiedStatus) + AssertFullResourceCoverage(t, &config) } @@ -921,6 +940,14 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { }, }, }, + Dashboards: map[string]*resources.Dashboard{ + "test_dashboard": { + DisplayName: "test_dashboard", + }, + "test_dashboard_new": { + DisplayName: "test_dashboard_new", + }, + }, }, } var tfState = resourcesState{ @@ -1069,6 +1096,22 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { {Attributes: stateInstanceAttributes{ID: "2"}}, }, }, + { + Type: "databricks_dashboard", + Mode: "managed", + Name: "test_dashboard", + Instances: []stateResourceInstance{ + {Attributes: stateInstanceAttributes{ID: "1"}}, + }, + }, + { + Type: "databricks_dashboard", + Mode: "managed", + Name: "test_dashboard_old", + Instances: []stateResourceInstance{ + {Attributes: stateInstanceAttributes{ID: "2"}}, + }, + }, }, } err := TerraformToBundle(&tfState, &config) @@ -1137,6 +1180,13 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { assert.Equal(t, "", config.Resources.Clusters["test_cluster_new"].ID) assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Clusters["test_cluster_new"].ModifiedStatus) + assert.Equal(t, "1", config.Resources.Dashboards["test_dashboard"].ID) + assert.Equal(t, "", config.Resources.Dashboards["test_dashboard"].ModifiedStatus) + assert.Equal(t, "2", config.Resources.Dashboards["test_dashboard_old"].ID) + assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Dashboards["test_dashboard_old"].ModifiedStatus) + assert.Equal(t, "", config.Resources.Dashboards["test_dashboard_new"].ID) + assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Dashboards["test_dashboard_new"].ModifiedStatus) + AssertFullResourceCoverage(t, &config) } From ba182c4f843033d02bf2896c4c5722933b8a3f5a Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 18 Oct 2024 16:45:38 +0200 Subject: [PATCH 22/39] Remove assumptions tests from core PR --- internal/acc/workspace.go | 30 ------- internal/dashboard_assumptions_test.go | 110 ------------------------- 2 files changed, 140 deletions(-) delete mode 100644 internal/dashboard_assumptions_test.go diff --git a/internal/acc/workspace.go b/internal/acc/workspace.go index 69ab0e715d..39374f229e 100644 --- a/internal/acc/workspace.go +++ b/internal/acc/workspace.go @@ -2,14 +2,11 @@ package acc import ( "context" - "fmt" "os" "testing" "github.com/databricks/databricks-sdk-go" - "github.com/databricks/databricks-sdk-go/apierr" "github.com/databricks/databricks-sdk-go/service/compute" - "github.com/databricks/databricks-sdk-go/service/workspace" "github.com/stretchr/testify/require" ) @@ -97,30 +94,3 @@ func (t *WorkspaceT) RunPython(code string) (string, error) { require.True(t, ok, "unexpected type %T", results.Data) return output, nil } - -func (t *WorkspaceT) TemporaryWorkspaceDir(name ...string) string { - ctx := context.Background() - me, err := t.W.CurrentUser.Me(ctx) - require.NoError(t, err) - - basePath := fmt.Sprintf("/Users/%s/%s", me.UserName, RandomName(name...)) - - t.Logf("Creating %s", basePath) - err = t.W.Workspace.MkdirsByPath(ctx, basePath) - require.NoError(t, err) - - // Remove test directory on test completion. - t.Cleanup(func() { - t.Logf("Removing %s", basePath) - err := t.W.Workspace.Delete(ctx, workspace.Delete{ - Path: basePath, - Recursive: true, - }) - if err == nil || apierr.IsMissing(err) { - return - } - t.Logf("Unable to remove temporary workspace directory %s: %#v", basePath, err) - }) - - return basePath -} diff --git a/internal/dashboard_assumptions_test.go b/internal/dashboard_assumptions_test.go deleted file mode 100644 index cd6a3602c9..0000000000 --- a/internal/dashboard_assumptions_test.go +++ /dev/null @@ -1,110 +0,0 @@ -package internal - -import ( - "encoding/base64" - "testing" - - "github.com/databricks/cli/internal/acc" - "github.com/databricks/cli/libs/dyn" - "github.com/databricks/cli/libs/dyn/convert" - "github.com/databricks/cli/libs/dyn/merge" - "github.com/databricks/databricks-sdk-go/apierr" - "github.com/databricks/databricks-sdk-go/service/dashboards" - "github.com/databricks/databricks-sdk-go/service/workspace" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// Verify that importing a dashboard through the Workspace API retains the identity of the underying resource, -// as well as properties exclusively accessible through the dashboards API. -func TestAccDashboardAssumptions_WorkspaceImport(t *testing.T) { - ctx, wt := acc.WorkspaceTest(t) - - t.Parallel() - - dashboardName := "New Dashboard" - dashboardPayload := []byte(`{"pages":[{"name":"2506f97a","displayName":"New Page"}]}`) - warehouseId := acc.GetEnvOrSkipTest(t, "TEST_DEFAULT_WAREHOUSE_ID") - - dir := wt.TemporaryWorkspaceDir("dashboard-assumptions-") - - dashboard, err := wt.W.Lakeview.Create(ctx, dashboards.CreateDashboardRequest{ - DisplayName: dashboardName, - ParentPath: dir, - SerializedDashboard: string(dashboardPayload), - WarehouseId: warehouseId, - }) - require.NoError(t, err) - t.Logf("Dashboard ID (per Lakeview API): %s", dashboard.DashboardId) - - // Overwrite the dashboard via the workspace API. - { - err := wt.W.Workspace.Import(ctx, workspace.Import{ - Format: workspace.ImportFormatAuto, - Path: dashboard.Path, - Content: base64.StdEncoding.EncodeToString(dashboardPayload), - Overwrite: true, - }) - require.NoError(t, err) - } - - // Cross-check consistency with the workspace object. - { - obj, err := wt.W.Workspace.GetStatusByPath(ctx, dashboard.Path) - require.NoError(t, err) - - // Confirm that the resource ID included in the response is equal to the dashboard ID. - require.Equal(t, dashboard.DashboardId, obj.ResourceId) - t.Logf("Dashboard ID (per workspace object status): %s", obj.ResourceId) - } - - // Try to overwrite the dashboard via the Lakeview API (and expect failure). - { - _, err := wt.W.Lakeview.Create(ctx, dashboards.CreateDashboardRequest{ - DisplayName: dashboardName, - ParentPath: dir, - SerializedDashboard: string(dashboardPayload), - }) - require.ErrorIs(t, err, apierr.ErrResourceAlreadyExists) - } - - // Retrieve the dashboard object and confirm that only select fields were updated by the import. - { - obj, err := wt.W.Lakeview.Get(ctx, dashboards.GetDashboardRequest{ - DashboardId: dashboard.DashboardId, - }) - require.NoError(t, err) - - // Convert the dashboard object to a [dyn.Value] to make comparison easier. - previous, err := convert.FromTyped(dashboard, dyn.NilValue) - require.NoError(t, err) - current, err := convert.FromTyped(obj, dyn.NilValue) - require.NoError(t, err) - - // Collect updated paths. - var updatedFieldPaths []string - _, err = merge.Override(previous, current, merge.OverrideVisitor{ - VisitDelete: func(basePath dyn.Path, left dyn.Value) error { - assert.Fail(t, "unexpected delete operation") - return nil - }, - VisitInsert: func(basePath dyn.Path, right dyn.Value) (dyn.Value, error) { - assert.Fail(t, "unexpected insert operation") - return right, nil - }, - VisitUpdate: func(basePath dyn.Path, left dyn.Value, right dyn.Value) (dyn.Value, error) { - updatedFieldPaths = append(updatedFieldPaths, basePath.String()) - return right, nil - }, - }) - require.NoError(t, err) - - // Confirm that only the expected fields have been updated. - assert.ElementsMatch(t, []string{ - "etag", - "serialized_dashboard", - "update_time", - }, updatedFieldPaths) - } -} From c1e43602141f4d6f63844670b1977cd774d1cc6d Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 21 Oct 2024 10:10:14 +0200 Subject: [PATCH 23/39] Add test coverage for dashboard prefix --- bundle/config/mutator/process_target_mode_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go index b0eb57ee13..3b2aff00ed 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -123,6 +123,9 @@ func mockBundle(mode config.Mode) *bundle.Bundle { Clusters: map[string]*resources.Cluster{ "cluster1": {ClusterSpec: &compute.ClusterSpec{ClusterName: "cluster1", SparkVersion: "13.2.x", NumWorkers: 1}}, }, + Dashboards: map[string]*resources.Dashboard{ + "dashboard1": {DisplayName: "dashboard1"}, + }, }, }, // Use AWS implementation for testing. @@ -184,6 +187,9 @@ func TestProcessTargetModeDevelopment(t *testing.T) { // Clusters assert.Equal(t, "[dev lennart] cluster1", b.Config.Resources.Clusters["cluster1"].ClusterName) + + // Dashboards + assert.Equal(t, "[dev lennart] dashboard1", b.Config.Resources.Dashboards["dashboard1"].DisplayName) } func TestProcessTargetModeDevelopmentTagNormalizationForAws(t *testing.T) { From 96c6f52e237c232a8fceaed9ce2d0da32060a74b Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 21 Oct 2024 12:26:14 +0200 Subject: [PATCH 24/39] Check for remote modification of dashboards --- .../terraform/check_modified_dashboards.go | 127 ++++++++++++ .../check_modified_dashboards_test.go | 189 ++++++++++++++++++ bundle/deploy/terraform/util.go | 5 +- bundle/phases/deploy.go | 1 + 4 files changed, 320 insertions(+), 2 deletions(-) create mode 100644 bundle/deploy/terraform/check_modified_dashboards.go create mode 100644 bundle/deploy/terraform/check_modified_dashboards_test.go diff --git a/bundle/deploy/terraform/check_modified_dashboards.go b/bundle/deploy/terraform/check_modified_dashboards.go new file mode 100644 index 0000000000..1a0bbcafca --- /dev/null +++ b/bundle/deploy/terraform/check_modified_dashboards.go @@ -0,0 +1,127 @@ +package terraform + +import ( + "context" + "fmt" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" + tfjson "github.com/hashicorp/terraform-json" +) + +type dashboardState struct { + Name string + ID string + ETag string +} + +func collectDashboards(ctx context.Context, b *bundle.Bundle) ([]dashboardState, error) { + state, err := ParseResourcesState(ctx, b) + if err != nil && state == nil { + return nil, err + } + + var dashboards []dashboardState + for _, resource := range state.Resources { + if resource.Mode != tfjson.ManagedResourceMode { + continue + } + for _, instance := range resource.Instances { + id := instance.Attributes.ID + if id == "" { + continue + } + + switch resource.Type { + case "databricks_dashboard": + etag := instance.Attributes.ETag + if etag == "" { + continue + } + + dashboards = append(dashboards, dashboardState{ + Name: resource.Name, + ID: id, + ETag: etag, + }) + } + } + } + + return dashboards, nil +} + +type checkModifiedDashboards struct { +} + +func (l *checkModifiedDashboards) Name() string { + return "CheckModifiedDashboards" +} + +func (l *checkModifiedDashboards) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + // This mutator is relevant only if the bundle includes dashboards. + if len(b.Config.Resources.Dashboards) == 0 { + return nil + } + + // If the user has forced the deployment, skip this check. + if b.Config.Bundle.Force { + return nil + } + + dashboards, err := collectDashboards(ctx, b) + if err != nil { + return diag.FromErr(err) + } + + var diags diag.Diagnostics + for _, dashboard := range dashboards { + // Skip dashboards that are not defined in the bundle. + // These will be destroyed upon deployment. + if _, ok := b.Config.Resources.Dashboards[dashboard.Name]; !ok { + continue + } + + path := dyn.MustPathFromString(fmt.Sprintf("resources.dashboards.%s", dashboard.Name)) + loc := b.Config.GetLocation(path.String()) + actual, err := b.WorkspaceClient().Lakeview.GetByDashboardId(ctx, dashboard.ID) + if err != nil { + diags = diags.Append(diag.Diagnostic{ + Severity: diag.Error, + Summary: fmt.Sprintf("failed to get dashboard %q", dashboard.Name), + Detail: err.Error(), + Paths: []dyn.Path{path}, + Locations: []dyn.Location{loc}, + }) + continue + } + + // If the ETag is the same, the dashboard has not been modified. + if actual.Etag == dashboard.ETag { + continue + } + + diags = diags.Append(diag.Diagnostic{ + Severity: diag.Error, + Summary: fmt.Sprintf("dashboard %q has been modified remotely", dashboard.Name), + Detail: "" + + "This dashboard has been modified remotely since the last bundle deployment.\n" + + "These modifications are untracked and will be overwritten on deploy.\n" + + "\n" + + "Make sure that the local dashboard definition matches what you intend to deploy\n" + + "before proceeding with the deployment.\n" + + "\n" + + "Run `databricks bundle deploy --force` to bypass this error." + + "", + Paths: []dyn.Path{path}, + Locations: []dyn.Location{loc}, + }) + } + + return diags +} + +func CheckModifiedDashboards() *checkModifiedDashboards { + return &checkModifiedDashboards{} +} diff --git a/bundle/deploy/terraform/check_modified_dashboards_test.go b/bundle/deploy/terraform/check_modified_dashboards_test.go new file mode 100644 index 0000000000..ff6f19b83c --- /dev/null +++ b/bundle/deploy/terraform/check_modified_dashboards_test.go @@ -0,0 +1,189 @@ +package terraform + +import ( + "context" + "fmt" + "path/filepath" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/internal/testutil" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/databricks-sdk-go/experimental/mocks" + "github.com/databricks/databricks-sdk-go/service/dashboards" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func mockDashboardBundle(t *testing.T) *bundle.Bundle { + dir := t.TempDir() + b := &bundle.Bundle{ + BundleRootPath: dir, + Config: config.Root{ + Bundle: config.Bundle{ + Target: "test", + }, + Resources: config.Resources{ + Dashboards: map[string]*resources.Dashboard{ + "dash1": { + DisplayName: "My Special Dashboard", + }, + }, + }, + }, + } + return b +} + +func TestCheckModifiedDashboards_NoDashboards(t *testing.T) { + dir := t.TempDir() + b := &bundle.Bundle{ + BundleRootPath: dir, + Config: config.Root{ + Bundle: config.Bundle{ + Target: "test", + }, + Resources: config.Resources{}, + }, + } + + diags := bundle.Apply(context.Background(), b, CheckModifiedDashboards()) + assert.Empty(t, diags) +} + +func TestCheckModifiedDashboards_FirstDeployment(t *testing.T) { + b := mockDashboardBundle(t) + diags := bundle.Apply(context.Background(), b, CheckModifiedDashboards()) + assert.Empty(t, diags) +} + +func TestCheckModifiedDashboards_ExistingStateNoChange(t *testing.T) { + ctx := context.Background() + + b := mockDashboardBundle(t) + writeFakeDashboardState(t, ctx, b) + + // Mock the call to the API. + m := mocks.NewMockWorkspaceClient(t) + dashboardsAPI := m.GetMockLakeviewAPI() + dashboardsAPI.EXPECT(). + GetByDashboardId(mock.Anything, "id1"). + Return(&dashboards.Dashboard{ + DisplayName: "My Special Dashboard", + Etag: "1000", + }, nil). + Once() + b.SetWorkpaceClient(m.WorkspaceClient) + + // No changes, so no diags. + diags := bundle.Apply(ctx, b, CheckModifiedDashboards()) + assert.Empty(t, diags) +} + +func TestCheckModifiedDashboards_ExistingStateChange(t *testing.T) { + ctx := context.Background() + + b := mockDashboardBundle(t) + writeFakeDashboardState(t, ctx, b) + + // Mock the call to the API. + m := mocks.NewMockWorkspaceClient(t) + dashboardsAPI := m.GetMockLakeviewAPI() + dashboardsAPI.EXPECT(). + GetByDashboardId(mock.Anything, "id1"). + Return(&dashboards.Dashboard{ + DisplayName: "My Special Dashboard", + Etag: "1234", + }, nil). + Once() + b.SetWorkpaceClient(m.WorkspaceClient) + + // The dashboard has changed, so expect an error. + diags := bundle.Apply(ctx, b, CheckModifiedDashboards()) + if assert.Len(t, diags, 1) { + assert.Equal(t, diag.Error, diags[0].Severity) + assert.Equal(t, `dashboard "dash1" has been modified remotely`, diags[0].Summary) + } +} + +func TestCheckModifiedDashboards_ExistingStateFailureToGet(t *testing.T) { + ctx := context.Background() + + b := mockDashboardBundle(t) + writeFakeDashboardState(t, ctx, b) + + // Mock the call to the API. + m := mocks.NewMockWorkspaceClient(t) + dashboardsAPI := m.GetMockLakeviewAPI() + dashboardsAPI.EXPECT(). + GetByDashboardId(mock.Anything, "id1"). + Return(nil, fmt.Errorf("failure")). + Once() + b.SetWorkpaceClient(m.WorkspaceClient) + + // Unable to get the dashboard, so expect an error. + diags := bundle.Apply(ctx, b, CheckModifiedDashboards()) + if assert.Len(t, diags, 1) { + assert.Equal(t, diag.Error, diags[0].Severity) + assert.Equal(t, `failed to get dashboard "dash1"`, diags[0].Summary) + } +} + +func writeFakeDashboardState(t *testing.T, ctx context.Context, b *bundle.Bundle) { + tfDir, err := Dir(ctx, b) + require.NoError(t, err) + + // Write fake state file. + testutil.WriteFile(t, ` + { + "version": 4, + "terraform_version": "1.5.5", + "resources": [ + { + "mode": "managed", + "type": "databricks_dashboard", + "name": "dash1", + "instances": [ + { + "schema_version": 0, + "attributes": { + "etag": "1000", + "id": "id1" + } + } + ] + }, + { + "mode": "managed", + "type": "databricks_job", + "name": "job", + "instances": [ + { + "schema_version": 0, + "attributes": { + "id": "1234" + } + } + ] + }, + { + "mode": "managed", + "type": "databricks_dashboard", + "name": "dash2", + "instances": [ + { + "schema_version": 0, + "attributes": { + "etag": "1001", + "id": "id2" + } + } + ] + } + ] + } + `, filepath.Join(tfDir, TerraformStateFileName)) +} diff --git a/bundle/deploy/terraform/util.go b/bundle/deploy/terraform/util.go index 64d667b5f6..4da015c23f 100644 --- a/bundle/deploy/terraform/util.go +++ b/bundle/deploy/terraform/util.go @@ -13,7 +13,7 @@ import ( // Partial representation of the Terraform state file format. // We are only interested global version and serial numbers, -// plus resource types, names, modes, and ids. +// plus resource types, names, modes, IDs, and ETags (for dashboards). type resourcesState struct { Version int `json:"version"` Resources []stateResource `json:"resources"` @@ -33,7 +33,8 @@ type stateResourceInstance struct { } type stateInstanceAttributes struct { - ID string `json:"id"` + ID string `json:"id"` + ETag string `json:"etag,omitempty"` } func ParseResourcesState(ctx context.Context, b *bundle.Bundle) (*resourcesState, error) { diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index cb0ecf75d2..514a02ee79 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -152,6 +152,7 @@ func Deploy(outputHandler sync.OutputHandler) bundle.Mutator { bundle.Defer( bundle.Seq( terraform.StatePull(), + terraform.CheckModifiedDashboards(), deploy.StatePull(), mutator.ValidateGitDetails(), artifacts.CleanUp(), From 0f48c98ba04ae56a67b94b7aee13b552993a66a3 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 21 Oct 2024 23:06:26 +0200 Subject: [PATCH 25/39] Work on bundle generate command --- bundle/config/generate/dashboard.go | 18 ++ cmd/bundle/generate.go | 1 + cmd/bundle/generate/dashboard.go | 385 ++++++++++++++++++++++++++ cmd/bundle/generate/dashboard_test.go | 1 + 4 files changed, 405 insertions(+) create mode 100644 bundle/config/generate/dashboard.go create mode 100644 cmd/bundle/generate/dashboard.go create mode 100644 cmd/bundle/generate/dashboard_test.go diff --git a/bundle/config/generate/dashboard.go b/bundle/config/generate/dashboard.go new file mode 100644 index 0000000000..4601408069 --- /dev/null +++ b/bundle/config/generate/dashboard.go @@ -0,0 +1,18 @@ +package generate + +import ( + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/databricks-sdk-go/service/dashboards" +) + +func ConvertDashboardToValue(dashboard *dashboards.Dashboard, filePath string) (dyn.Value, error) { + // The majority of fields of the dashboard struct are read-only. + // We copy the relevant fields manually. + dv := map[string]dyn.Value{ + "display_name": dyn.NewValue(dashboard.DisplayName, []dyn.Location{{Line: 1}}), + "warehouse_id": dyn.NewValue(dashboard.WarehouseId, []dyn.Location{{Line: 2}}), + "file_path": dyn.NewValue(filePath, []dyn.Location{{Line: 3}}), + } + + return dyn.V(dv), nil +} diff --git a/cmd/bundle/generate.go b/cmd/bundle/generate.go index 1e3d56e430..7dea19ff9d 100644 --- a/cmd/bundle/generate.go +++ b/cmd/bundle/generate.go @@ -16,6 +16,7 @@ func newGenerateCommand() *cobra.Command { cmd.AddCommand(generate.NewGenerateJobCommand()) cmd.AddCommand(generate.NewGeneratePipelineCommand()) + cmd.AddCommand(generate.NewGenerateDashboardCommand()) cmd.PersistentFlags().StringVar(&key, "key", "", `resource key to use for the generated configuration`) return cmd } diff --git a/cmd/bundle/generate/dashboard.go b/cmd/bundle/generate/dashboard.go new file mode 100644 index 0000000000..c3178147ed --- /dev/null +++ b/cmd/bundle/generate/dashboard.go @@ -0,0 +1,385 @@ +package generate + +import ( + "context" + "encoding/json" + "fmt" + "os" + "path" + "path/filepath" + "strings" + "time" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/generate" + "github.com/databricks/cli/bundle/deploy/terraform" + "github.com/databricks/cli/bundle/phases" + "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/yamlsaver" + "github.com/databricks/cli/libs/textutil" + "github.com/databricks/databricks-sdk-go/apierr" + "github.com/databricks/databricks-sdk-go/service/dashboards" + "github.com/databricks/databricks-sdk-go/service/workspace" + "github.com/spf13/cobra" + "gopkg.in/yaml.v3" +) + +type dashboardLookup struct { + dashboardPath string + dashboardId string + resource string +} + +func (d *dashboardLookup) 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") +} + +func (d *dashboardLookup) 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.FromErr(err) + } + + if obj.ObjectType != workspace.ObjectTypeDashboard { + found := strings.ToLower(obj.ObjectType.String()) + return "", diag.Diagnostics{ + { + Severity: diag.Error, + Summary: fmt.Sprintf("expected a dashboard, found a %s", found), + }, + } + } + + if obj.ResourceId == "" { + return "", diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "expected a non-empty dashboard resource ID", + }, + } + } + + return obj.ResourceId, nil +} + +func (d *dashboardLookup) resolveFromID(ctx context.Context, b *bundle.Bundle) (string, diag.Diagnostics) { + w := b.WorkspaceClient() + obj, err := w.Lakeview.GetByDashboardId(ctx, d.dashboardId) + if err != nil { + if apierr.IsMissing(err) { + return "", diag.Errorf("dashboard with ID %s not found", d.dashboardId) + } + return "", diag.FromErr(err) + } + + 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 + err = json.Unmarshal(data, &tmp) + if err != nil { + return nil, err + } + out, err := json.MarshalIndent(tmp, "", " ") + if err != nil { + return nil, err + } + return out, nil +} + +func (d *dashboard) saveSerializedDashboard(_ context.Context, b *bundle.Bundle, dashboard *dashboards.Dashboard, filename string) error { + // Unmarshal and remarshal the serialized dashboard to ensure it is formatted correctly. + // The result will have alphabetically sorted keys and be indented. + data, err := remarshalJSON([]byte(dashboard.SerializedDashboard)) + if err != nil { + return err + } + + // Make sure the output directory exists. + if err := os.MkdirAll(filepath.Dir(filename), 0755); err != nil { + return err + } + + // Clean the filename to ensure it is a valid path (and can be used on this OS). + filename = filepath.Clean(filename) + + // Attempt to make the path relative to the bundle root. + rel, err := filepath.Rel(b.BundleRootPath, filename) + if err != nil { + rel = filename + } + + // Verify that the file does not already exist. + info, err := os.Stat(filename) + if err == nil { + if info.IsDir() { + return fmt.Errorf("%s is a directory", rel) + } + if !d.force { + return fmt.Errorf("%s already exists. Use --force to overwrite", rel) + } + } + + fmt.Printf("Writing dashboard to %q\n", rel) + 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) + } + + // Save serialized dashboard definition to the dashboard directory. + dashboardBasename := fmt.Sprintf("%s.lvdash.json", key) + dashboardPath := filepath.Join(d.dashboardDir, dashboardBasename) + err := d.saveSerializedDashboard(ctx, b, dashboard, dashboardPath) + if err != nil { + return err + } + + // Synthesize resource configuration. + v, err := generate.ConvertDashboardToValue(dashboard, path.Join(d.relativeDashboardDir, dashboardBasename)) + if err != nil { + return err + } + + result := map[string]dyn.Value{ + "resources": dyn.V(map[string]dyn.Value{ + "dashboards": dyn.V(map[string]dyn.Value{ + key: v, + }), + }), + } + + // Make sure the output directory exists. + if err := os.MkdirAll(d.resourceDir, 0755); err != nil { + return err + } + + // Save the configuration to the resource directory. + resourcePath := filepath.Join(d.resourceDir, fmt.Sprintf("%s.yml", key)) + saver := yamlsaver.NewSaverWithStyle(map[string]yaml.Style{ + "display_name": yaml.DoubleQuotedStyle, + }) + + // Attempt to make the path relative to the bundle root. + rel, err := filepath.Rel(b.BundleRootPath, resourcePath) + if err != nil { + rel = resourcePath + } + + fmt.Printf("Writing configuration to %q\n", rel) + err = saver.SaveAsYAML(result, resourcePath, d.force) + if err != nil { + return err + } + + return nil +} + +func (d *dashboard) runWatch(ctx context.Context, b *bundle.Bundle, dashboardID 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.FilePath == "" { + return diag.Errorf("dashboard resource %q has no file path defined", d.resource) + } + + // Overwrite the dashboard at the path referenced from the resource. + dashboardPath := resource.FilePath + + // Start polling the underlying dashboard for changes. + var etag string + for { + w := b.WorkspaceClient() + dashboard, err := w.Lakeview.GetByDashboardId(ctx, dashboardID) + if err != nil { + return diag.FromErr(err) + } + + if etag != dashboard.Etag { + err = d.saveSerializedDashboard(ctx, b, dashboard, dashboardPath) + if err != nil { + return diag.FromErr(err) + } + } + + // Update the etag for the next iteration. + etag = dashboard.Etag + + // Compute [time.Time] for the most recent update. + tref, err := time.Parse(time.RFC3339, dashboard.UpdateTime) + if err != nil { + return diag.FromErr(err) + } + + // Now poll the workspace API for changes. + // This is much more efficient than polling the dashboard API. + for { + obj, err := w.Workspace.GetStatusByPath(ctx, dashboard.Path) + if err != nil { + return diag.FromErr(err) + } + + // Compute [time.Time] from timestamp in millis since epoch. + tcur := time.Unix(0, obj.ModifiedAt*int64(time.Millisecond)) + if tcur.After(tref) { + break + } + + time.Sleep(1 * time.Second) + } + } +} + +func (d *dashboard) runOnce(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) + if err != nil { + return diag.FromErr(err) + } + + return nil +} + +func (d *dashboard) initialize(b *bundle.Bundle) diag.Diagnostics { + // Make the paths absolute if they aren't already. + if !filepath.IsAbs(d.resourceDir) { + d.resourceDir = filepath.Join(b.BundleRootPath, d.resourceDir) + } + if !filepath.IsAbs(d.dashboardDir) { + d.dashboardDir = filepath.Join(b.BundleRootPath, d.dashboardDir) + } + + // Make sure we know how the dashboard path is relative to the resource path. + rel, err := filepath.Rel(d.resourceDir, d.dashboardDir) + if err != nil { + return diag.FromErr(err) + } + + d.relativeDashboardDir = filepath.ToSlash(rel) + return nil +} + +func (d *dashboard) RunE(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + b, diags := root.MustConfigureBundle(cmd) + if diags.HasError() { + return diags.Error() + } + + diags = d.initialize(b) + if diags.HasError() { + 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) + } else { + diags = d.runOnce(ctx, b, dashboardID) + } + + return diags.Error() +} + +func NewGenerateDashboardCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "dashboard", + Short: "Generate configuration for a dashboard", + } + + d := &dashboard{} + + // 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`) + + // Output flags. + cmd.Flags().StringVarP(&d.resourceDir, "resource-dir", "d", "./resources", `directory to write the configuration to`) + cmd.Flags().StringVarP(&d.dashboardDir, "dashboard-dir", "s", "./src", `directory to write the dashboard representation to`) + cmd.Flags().BoolVarP(&d.force, "force", "f", false, `force overwrite existing files in the output directory`) + + cmd.MarkFlagsOneRequired( + "existing-path", + "existing-id", + "existing-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.RunE = d.RunE + return cmd +} diff --git a/cmd/bundle/generate/dashboard_test.go b/cmd/bundle/generate/dashboard_test.go new file mode 100644 index 0000000000..eb8347795a --- /dev/null +++ b/cmd/bundle/generate/dashboard_test.go @@ -0,0 +1 @@ +package generate From e19f9b71f0dc641b6eb5a9fda9f8ee196bc9446f Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 22 Oct 2024 10:12:09 +0200 Subject: [PATCH 26/39] Work on generate command --- cmd/bundle/generate/dashboard.go | 127 ++++++++++++++++--------------- 1 file changed, 64 insertions(+), 63 deletions(-) diff --git a/cmd/bundle/generate/dashboard.go b/cmd/bundle/generate/dashboard.go index c3178147ed..416ca64d1b 100644 --- a/cmd/bundle/generate/dashboard.go +++ b/cmd/bundle/generate/dashboard.go @@ -26,26 +26,40 @@ import ( "gopkg.in/yaml.v3" ) -type dashboardLookup struct { +type dashboard struct { + // Lookup flags for one-time generate. dashboardPath string dashboardId string - resource 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 { @@ -77,7 +91,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 +104,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 +155,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 +204,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 +214,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 +236,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 +269,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,19 +304,8 @@ func (d *dashboard) initialize(b *bundle.Bundle) diag.Diagnostics { return nil } -func (d *dashboard) RunE(cmd *cobra.Command, args []string) error { - ctx := cmd.Context() - b, diags := root.MustConfigureBundle(cmd) - if diags.HasError() { - return diags.Error() - } - - diags = d.initialize(b) - if diags.HasError() { - return diags.Error() - } - - diags = bundle.Apply(ctx, b, bundle.Seq( +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(), @@ -331,19 +313,38 @@ func (d *dashboard) RunE(cmd *cobra.Command, args []string) error { terraform.Load(), )) if diags.HasError() { - return diags.Error() + 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) + if diags.HasError() { + return diags.Error() + } + + diags = d.initialize(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() @@ -360,7 +361,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 +371,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 From 6057c135cf85e02597bb541da3e7ceb7bfb132ce Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 22 Oct 2024 14:55:51 +0200 Subject: [PATCH 27/39] Specialize error when the path points to a legacy dashboard --- cmd/bundle/generate/dashboard.go | 32 ++++++++++++++++++-- cmd/bundle/generate/dashboard_test.go | 42 +++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/cmd/bundle/generate/dashboard.go b/cmd/bundle/generate/dashboard.go index 416ca64d1b..5e4c84ed6e 100644 --- a/cmd/bundle/generate/dashboard.go +++ b/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" @@ -64,8 +66,24 @@ func (d *dashboard) resolveFromPath(ctx context.Context, b *bundle.Bundle) (stri 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) } @@ -347,7 +365,17 @@ func (d *dashboard) RunE(cmd *cobra.Command, args []string) error { 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 { diff --git a/cmd/bundle/generate/dashboard_test.go b/cmd/bundle/generate/dashboard_test.go index eb8347795a..8df6650795 100644 --- a/cmd/bundle/generate/dashboard_test.go +++ b/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") +} From 5b6a7b2e7c19106e520e7fc135f667e54959ac6e Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 22 Oct 2024 14:56:48 +0200 Subject: [PATCH 28/39] Interpolate references to the dashboard resource correctly --- bundle/deploy/terraform/interpolate.go | 2 ++ bundle/deploy/terraform/interpolate_test.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/bundle/deploy/terraform/interpolate.go b/bundle/deploy/terraform/interpolate.go index 12894c6843..eb15c63ec7 100644 --- a/bundle/deploy/terraform/interpolate.go +++ b/bundle/deploy/terraform/interpolate.go @@ -60,6 +60,8 @@ func (m *interpolateMutator) Apply(ctx context.Context, b *bundle.Bundle) diag.D path = dyn.NewPath(dyn.Key("databricks_schema")).Append(path[2:]...) case dyn.Key("clusters"): path = dyn.NewPath(dyn.Key("databricks_cluster")).Append(path[2:]...) + case dyn.Key("dashboards"): + path = dyn.NewPath(dyn.Key("databricks_dashboard")).Append(path[2:]...) default: // Trigger "key not found" for unknown resource types. return dyn.GetByPath(root, path) diff --git a/bundle/deploy/terraform/interpolate_test.go b/bundle/deploy/terraform/interpolate_test.go index 630a904acd..b26ef928da 100644 --- a/bundle/deploy/terraform/interpolate_test.go +++ b/bundle/deploy/terraform/interpolate_test.go @@ -32,6 +32,7 @@ func TestInterpolate(t *testing.T) { "other_registered_model": "${resources.registered_models.other_registered_model.id}", "other_schema": "${resources.schemas.other_schema.id}", "other_cluster": "${resources.clusters.other_cluster.id}", + "other_dashboard": "${resources.dashboards.other_dashboard.id}", }, Tasks: []jobs.Task{ { @@ -69,6 +70,7 @@ func TestInterpolate(t *testing.T) { assert.Equal(t, "${databricks_registered_model.other_registered_model.id}", j.Tags["other_registered_model"]) assert.Equal(t, "${databricks_schema.other_schema.id}", j.Tags["other_schema"]) assert.Equal(t, "${databricks_cluster.other_cluster.id}", j.Tags["other_cluster"]) + assert.Equal(t, "${databricks_dashboard.other_dashboard.id}", j.Tags["other_dashboard"]) m := b.Config.Resources.Models["my_model"] assert.Equal(t, "my_model", m.Model.Name) From ef6b2ea07b67c55db3815eaf286837785a9235ce Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 22 Oct 2024 14:57:57 +0200 Subject: [PATCH 29/39] Generate file as key.dashboard.yml --- cmd/bundle/generate/dashboard.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/bundle/generate/dashboard.go b/cmd/bundle/generate/dashboard.go index 5e4c84ed6e..e139c4b1c7 100644 --- a/cmd/bundle/generate/dashboard.go +++ b/cmd/bundle/generate/dashboard.go @@ -202,7 +202,7 @@ func (d *dashboard) saveConfiguration(ctx context.Context, b *bundle.Bundle, das } // Save the configuration to the resource directory. - resourcePath := filepath.Join(d.resourceDir, fmt.Sprintf("%s.yml", key)) + resourcePath := filepath.Join(d.resourceDir, fmt.Sprintf("%s.dashboard.yml", key)) saver := yamlsaver.NewSaverWithStyle(map[string]yaml.Style{ "display_name": yaml.DoubleQuotedStyle, }) From 866bfc5be7f9b87e562872c7473ac7e054ef5cd4 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 24 Oct 2024 13:35:22 +0200 Subject: [PATCH 30/39] Include JSON schema for dashboards --- bundle/schema/jsonschema.json | 56 +++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/bundle/schema/jsonschema.json b/bundle/schema/jsonschema.json index 178656fe05..788b0e1f06 100644 --- a/bundle/schema/jsonschema.json +++ b/bundle/schema/jsonschema.json @@ -180,6 +180,45 @@ } ] }, + "resources.Dashboard": { + "anyOf": [ + { + "type": "object", + "properties": { + "display_name": { + "$ref": "#/$defs/string" + }, + "embed_credentials": { + "$ref": "#/$defs/bool" + }, + "file_path": { + "$ref": "#/$defs/string" + }, + "parent_path": { + "$ref": "#/$defs/string" + }, + "permissions": { + "$ref": "#/$defs/slice/github.com/databricks/cli/bundle/config/resources.Permission" + }, + "serialized_dashboard": { + "$ref": "#/$defs/interface" + }, + "warehouse_id": { + "$ref": "#/$defs/string" + } + }, + "additionalProperties": false, + "required": [ + "display_name", + "warehouse_id" + ] + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, "resources.Grant": { "anyOf": [ { @@ -1054,6 +1093,9 @@ "clusters": { "$ref": "#/$defs/map/github.com/databricks/cli/bundle/config/resources.Cluster" }, + "dashboards": { + "$ref": "#/$defs/map/github.com/databricks/cli/bundle/config/resources.Dashboard" + }, "experiments": { "$ref": "#/$defs/map/github.com/databricks/cli/bundle/config/resources.MlflowExperiment" }, @@ -5292,6 +5334,20 @@ } ] }, + "resources.Dashboard": { + "anyOf": [ + { + "type": "object", + "additionalProperties": { + "$ref": "#/$defs/github.com/databricks/cli/bundle/config/resources.Dashboard" + } + }, + { + "type": "string", + "pattern": "\\$\\{(var(\\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\\[[0-9]+\\])*)+)\\}" + } + ] + }, "resources.Job": { "anyOf": [ { From 93155f1c77ef2093187eb81c22d30b680f03e91e Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 24 Oct 2024 16:24:06 +0200 Subject: [PATCH 31/39] Inline SDK struct --- bundle/config/mutator/apply_presets.go | 2 +- .../configure_dashboard_defaults_test.go | 9 +++++-- bundle/config/mutator/initialize_urls_test.go | 7 +++-- .../mutator/process_target_mode_test.go | 7 ++++- bundle/config/resources/dashboard.go | 27 +++++-------------- .../check_modified_dashboards_test.go | 4 ++- bundle/deploy/terraform/convert_test.go | 13 ++++++--- .../terraform/tfdyn/convert_dashboard_test.go | 10 ++++--- 8 files changed, 46 insertions(+), 33 deletions(-) diff --git a/bundle/config/mutator/apply_presets.go b/bundle/config/mutator/apply_presets.go index 3d1f3d766b..d2a1d0c7da 100644 --- a/bundle/config/mutator/apply_presets.go +++ b/bundle/config/mutator/apply_presets.go @@ -214,7 +214,7 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos // Dashboards: Prefix for key, dashboard := range r.Dashboards { - if dashboard == nil { + if dashboard == nil || dashboard.CreateDashboardRequest == nil { diags = diags.Extend(diag.Errorf("dashboard %s s is not defined", key)) continue } diff --git a/bundle/config/mutator/configure_dashboard_defaults_test.go b/bundle/config/mutator/configure_dashboard_defaults_test.go index 98b0e37e15..4804b7159a 100644 --- a/bundle/config/mutator/configure_dashboard_defaults_test.go +++ b/bundle/config/mutator/configure_dashboard_defaults_test.go @@ -10,6 +10,7 @@ import ( "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/bundle/internal/bundletest" "github.com/databricks/cli/libs/dyn" + "github.com/databricks/databricks-sdk-go/service/dashboards" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -25,11 +26,15 @@ func TestConfigureDashboardDefaultsParentPath(t *testing.T) { "d1": { // Empty string is skipped. // See below for how it is set. - ParentPath: "", + CreateDashboardRequest: &dashboards.CreateDashboardRequest{ + ParentPath: "", + }, }, "d2": { // Non-empty string is skipped. - ParentPath: "already-set", + CreateDashboardRequest: &dashboards.CreateDashboardRequest{ + ParentPath: "already-set", + }, }, "d3": { // No parent path set. diff --git a/bundle/config/mutator/initialize_urls_test.go b/bundle/config/mutator/initialize_urls_test.go index bfc0b56490..61103de80e 100644 --- a/bundle/config/mutator/initialize_urls_test.go +++ b/bundle/config/mutator/initialize_urls_test.go @@ -8,6 +8,7 @@ import ( "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/databricks-sdk-go/service/compute" + "github.com/databricks/databricks-sdk-go/service/dashboards" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/databricks/databricks-sdk-go/service/ml" "github.com/databricks/databricks-sdk-go/service/pipelines" @@ -87,8 +88,10 @@ func TestInitializeURLs(t *testing.T) { }, Dashboards: map[string]*resources.Dashboard{ "dashboard1": { - ID: "01ef8d56871e1d50ae30ce7375e42478", - DisplayName: "My special dashboard", + ID: "01ef8d56871e1d50ae30ce7375e42478", + CreateDashboardRequest: &dashboards.CreateDashboardRequest{ + DisplayName: "My special dashboard", + }, }, }, }, diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go index 3b2aff00ed..4346e88fe9 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -14,6 +14,7 @@ import ( sdkconfig "github.com/databricks/databricks-sdk-go/config" "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/databricks-sdk-go/service/compute" + "github.com/databricks/databricks-sdk-go/service/dashboards" "github.com/databricks/databricks-sdk-go/service/iam" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/databricks/databricks-sdk-go/service/ml" @@ -124,7 +125,11 @@ func mockBundle(mode config.Mode) *bundle.Bundle { "cluster1": {ClusterSpec: &compute.ClusterSpec{ClusterName: "cluster1", SparkVersion: "13.2.x", NumWorkers: 1}}, }, Dashboards: map[string]*resources.Dashboard{ - "dashboard1": {DisplayName: "dashboard1"}, + "dashboard1": { + CreateDashboardRequest: &dashboards.CreateDashboardRequest{ + DisplayName: "dashboard1", + }, + }, }, }, }, diff --git a/bundle/config/resources/dashboard.go b/bundle/config/resources/dashboard.go index 0eb104c6a0..462dbc5641 100644 --- a/bundle/config/resources/dashboard.go +++ b/bundle/config/resources/dashboard.go @@ -17,27 +17,18 @@ type Dashboard struct { ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` URL string `json:"url,omitempty" bundle:"internal"` - // =========================== - // === BEGIN OF API FIELDS === - // =========================== + *dashboards.CreateDashboardRequest - // DisplayName is the display name of the dashboard (both as title and as basename in the workspace). - DisplayName string `json:"display_name"` - - // WarehouseID is the ID of the SQL Warehouse used to run the dashboard's queries. - WarehouseID string `json:"warehouse_id"` + // ========================= + // === Additional fields === + // ========================= // SerializedDashboard holds the contents of the dashboard in serialized JSON form. - // Note: its type is any and not string such that it can be inlined as YAML. - // If it is not a string, its contents will be marshalled as JSON. + // We override the field's type from the SDK struct here to allow for inlining as YAML. + // If the value is a string, it is used as is. + // If it is not a string, its contents is marshalled as JSON. SerializedDashboard any `json:"serialized_dashboard,omitempty"` - // ParentPath is the workspace path of the folder containing the dashboard. - // Includes leading slash and no trailing slash. - // - // Defaults to ${workspace.resource_path} if not set. - ParentPath string `json:"parent_path,omitempty"` - // EmbedCredentials is a flag to indicate if the publisher's credentials should // be embedded in the published dashboard. These embedded credentials will be used // to execute the published dashboard's queries. @@ -45,10 +36,6 @@ type Dashboard struct { // Defaults to false if not set. EmbedCredentials bool `json:"embed_credentials,omitempty"` - // =========================== - // ==== END OF API FIELDS ==== - // =========================== - // FilePath points to the local `.lvdash.json` file containing the dashboard definition. FilePath string `json:"file_path,omitempty"` } diff --git a/bundle/deploy/terraform/check_modified_dashboards_test.go b/bundle/deploy/terraform/check_modified_dashboards_test.go index ff6f19b83c..c5e4c40d38 100644 --- a/bundle/deploy/terraform/check_modified_dashboards_test.go +++ b/bundle/deploy/terraform/check_modified_dashboards_test.go @@ -29,7 +29,9 @@ func mockDashboardBundle(t *testing.T) *bundle.Bundle { Resources: config.Resources{ Dashboards: map[string]*resources.Dashboard{ "dash1": { - DisplayName: "My Special Dashboard", + CreateDashboardRequest: &dashboards.CreateDashboardRequest{ + DisplayName: "My Special Dashboard", + }, }, }, }, diff --git a/bundle/deploy/terraform/convert_test.go b/bundle/deploy/terraform/convert_test.go index dab8890b37..3f69bbed4b 100644 --- a/bundle/deploy/terraform/convert_test.go +++ b/bundle/deploy/terraform/convert_test.go @@ -12,6 +12,7 @@ import ( "github.com/databricks/cli/libs/dyn/convert" "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/databricks-sdk-go/service/compute" + "github.com/databricks/databricks-sdk-go/service/dashboards" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/databricks/databricks-sdk-go/service/ml" "github.com/databricks/databricks-sdk-go/service/pipelines" @@ -791,7 +792,9 @@ func TestTerraformToBundleEmptyRemoteResources(t *testing.T) { }, Dashboards: map[string]*resources.Dashboard{ "test_dashboard": { - DisplayName: "test_dashboard", + CreateDashboardRequest: &dashboards.CreateDashboardRequest{ + DisplayName: "test_dashboard", + }, }, }, }, @@ -948,10 +951,14 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { }, Dashboards: map[string]*resources.Dashboard{ "test_dashboard": { - DisplayName: "test_dashboard", + CreateDashboardRequest: &dashboards.CreateDashboardRequest{ + DisplayName: "test_dashboard", + }, }, "test_dashboard_new": { - DisplayName: "test_dashboard_new", + CreateDashboardRequest: &dashboards.CreateDashboardRequest{ + DisplayName: "test_dashboard_new", + }, }, }, }, diff --git a/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go b/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go index dfb2ffa44c..0c4866524b 100644 --- a/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go +++ b/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go @@ -8,15 +8,19 @@ import ( "github.com/databricks/cli/bundle/internal/tf/schema" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/dyn/convert" + "github.com/databricks/databricks-sdk-go/service/dashboards" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestConvertDashboard(t *testing.T) { var src = resources.Dashboard{ - DisplayName: "my dashboard", - WarehouseID: "f00dcafe", - ParentPath: "/some/path", + CreateDashboardRequest: &dashboards.CreateDashboardRequest{ + DisplayName: "my dashboard", + WarehouseId: "f00dcafe", + ParentPath: "/some/path", + }, + EmbedCredentials: true, Permissions: []resources.Permission{ From c4df41c3d62af477fb5733e17cd091269f67cefb Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 24 Oct 2024 16:29:52 +0200 Subject: [PATCH 32/39] Rename remote modification check --- ... => check_dashboards_modified_remotely.go} | 30 +++++++------------ ...heck_dashboards_modified_remotely_test.go} | 20 ++++++------- bundle/phases/deploy.go | 2 +- 3 files changed, 21 insertions(+), 31 deletions(-) rename bundle/deploy/terraform/{check_modified_dashboards.go => check_dashboards_modified_remotely.go} (80%) rename bundle/deploy/terraform/{check_modified_dashboards_test.go => check_dashboards_modified_remotely_test.go} (85%) diff --git a/bundle/deploy/terraform/check_modified_dashboards.go b/bundle/deploy/terraform/check_dashboards_modified_remotely.go similarity index 80% rename from bundle/deploy/terraform/check_modified_dashboards.go rename to bundle/deploy/terraform/check_dashboards_modified_remotely.go index 1a0bbcafca..c884bcb9b4 100644 --- a/bundle/deploy/terraform/check_modified_dashboards.go +++ b/bundle/deploy/terraform/check_dashboards_modified_remotely.go @@ -16,7 +16,7 @@ type dashboardState struct { ETag string } -func collectDashboards(ctx context.Context, b *bundle.Bundle) ([]dashboardState, error) { +func collectDashboardsFromState(ctx context.Context, b *bundle.Bundle) ([]dashboardState, error) { state, err := ParseResourcesState(ctx, b) if err != nil && state == nil { return nil, err @@ -28,22 +28,12 @@ func collectDashboards(ctx context.Context, b *bundle.Bundle) ([]dashboardState, continue } for _, instance := range resource.Instances { - id := instance.Attributes.ID - if id == "" { - continue - } - switch resource.Type { case "databricks_dashboard": - etag := instance.Attributes.ETag - if etag == "" { - continue - } - dashboards = append(dashboards, dashboardState{ Name: resource.Name, - ID: id, - ETag: etag, + ID: instance.Attributes.ID, + ETag: instance.Attributes.ETag, }) } } @@ -52,14 +42,14 @@ func collectDashboards(ctx context.Context, b *bundle.Bundle) ([]dashboardState, return dashboards, nil } -type checkModifiedDashboards struct { +type checkDashboardsModifiedRemotely struct { } -func (l *checkModifiedDashboards) Name() string { - return "CheckModifiedDashboards" +func (l *checkDashboardsModifiedRemotely) Name() string { + return "CheckDashboardsModifiedRemotely" } -func (l *checkModifiedDashboards) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { +func (l *checkDashboardsModifiedRemotely) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { // This mutator is relevant only if the bundle includes dashboards. if len(b.Config.Resources.Dashboards) == 0 { return nil @@ -70,7 +60,7 @@ func (l *checkModifiedDashboards) Apply(ctx context.Context, b *bundle.Bundle) d return nil } - dashboards, err := collectDashboards(ctx, b) + dashboards, err := collectDashboardsFromState(ctx, b) if err != nil { return diag.FromErr(err) } @@ -122,6 +112,6 @@ func (l *checkModifiedDashboards) Apply(ctx context.Context, b *bundle.Bundle) d return diags } -func CheckModifiedDashboards() *checkModifiedDashboards { - return &checkModifiedDashboards{} +func CheckDashboardsModifiedRemotely() *checkDashboardsModifiedRemotely { + return &checkDashboardsModifiedRemotely{} } diff --git a/bundle/deploy/terraform/check_modified_dashboards_test.go b/bundle/deploy/terraform/check_dashboards_modified_remotely_test.go similarity index 85% rename from bundle/deploy/terraform/check_modified_dashboards_test.go rename to bundle/deploy/terraform/check_dashboards_modified_remotely_test.go index c5e4c40d38..c13f800f7e 100644 --- a/bundle/deploy/terraform/check_modified_dashboards_test.go +++ b/bundle/deploy/terraform/check_dashboards_modified_remotely_test.go @@ -40,7 +40,7 @@ func mockDashboardBundle(t *testing.T) *bundle.Bundle { return b } -func TestCheckModifiedDashboards_NoDashboards(t *testing.T) { +func TestCheckDashboardsModifiedRemotely_NoDashboards(t *testing.T) { dir := t.TempDir() b := &bundle.Bundle{ BundleRootPath: dir, @@ -52,17 +52,17 @@ func TestCheckModifiedDashboards_NoDashboards(t *testing.T) { }, } - diags := bundle.Apply(context.Background(), b, CheckModifiedDashboards()) + diags := bundle.Apply(context.Background(), b, CheckDashboardsModifiedRemotely()) assert.Empty(t, diags) } -func TestCheckModifiedDashboards_FirstDeployment(t *testing.T) { +func TestCheckDashboardsModifiedRemotely_FirstDeployment(t *testing.T) { b := mockDashboardBundle(t) - diags := bundle.Apply(context.Background(), b, CheckModifiedDashboards()) + diags := bundle.Apply(context.Background(), b, CheckDashboardsModifiedRemotely()) assert.Empty(t, diags) } -func TestCheckModifiedDashboards_ExistingStateNoChange(t *testing.T) { +func TestCheckDashboardsModifiedRemotely_ExistingStateNoChange(t *testing.T) { ctx := context.Background() b := mockDashboardBundle(t) @@ -81,11 +81,11 @@ func TestCheckModifiedDashboards_ExistingStateNoChange(t *testing.T) { b.SetWorkpaceClient(m.WorkspaceClient) // No changes, so no diags. - diags := bundle.Apply(ctx, b, CheckModifiedDashboards()) + diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely()) assert.Empty(t, diags) } -func TestCheckModifiedDashboards_ExistingStateChange(t *testing.T) { +func TestCheckDashboardsModifiedRemotely_ExistingStateChange(t *testing.T) { ctx := context.Background() b := mockDashboardBundle(t) @@ -104,14 +104,14 @@ func TestCheckModifiedDashboards_ExistingStateChange(t *testing.T) { b.SetWorkpaceClient(m.WorkspaceClient) // The dashboard has changed, so expect an error. - diags := bundle.Apply(ctx, b, CheckModifiedDashboards()) + diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely()) if assert.Len(t, diags, 1) { assert.Equal(t, diag.Error, diags[0].Severity) assert.Equal(t, `dashboard "dash1" has been modified remotely`, diags[0].Summary) } } -func TestCheckModifiedDashboards_ExistingStateFailureToGet(t *testing.T) { +func TestCheckDashboardsModifiedRemotely_ExistingStateFailureToGet(t *testing.T) { ctx := context.Background() b := mockDashboardBundle(t) @@ -127,7 +127,7 @@ func TestCheckModifiedDashboards_ExistingStateFailureToGet(t *testing.T) { b.SetWorkpaceClient(m.WorkspaceClient) // Unable to get the dashboard, so expect an error. - diags := bundle.Apply(ctx, b, CheckModifiedDashboards()) + diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely()) if assert.Len(t, diags, 1) { assert.Equal(t, diag.Error, diags[0].Severity) assert.Equal(t, `failed to get dashboard "dash1"`, diags[0].Summary) diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index 514a02ee79..e623c364f9 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -152,7 +152,7 @@ func Deploy(outputHandler sync.OutputHandler) bundle.Mutator { bundle.Defer( bundle.Seq( terraform.StatePull(), - terraform.CheckModifiedDashboards(), + terraform.CheckDashboardsModifiedRemotely(), deploy.StatePull(), mutator.ValidateGitDetails(), artifacts.CleanUp(), From a3e32c0adb0594b043d5a42987c270e3129f8bd8 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 24 Oct 2024 17:10:13 +0200 Subject: [PATCH 33/39] Marshal contents of 'serialized_dashboard' if not a string --- .../terraform/tfdyn/convert_dashboard.go | 41 +++++++++++++++-- .../terraform/tfdyn/convert_dashboard_test.go | 45 +++++++++++++++++++ 2 files changed, 83 insertions(+), 3 deletions(-) diff --git a/bundle/deploy/terraform/tfdyn/convert_dashboard.go b/bundle/deploy/terraform/tfdyn/convert_dashboard.go index f905ba9996..5d3a5d14f7 100644 --- a/bundle/deploy/terraform/tfdyn/convert_dashboard.go +++ b/bundle/deploy/terraform/tfdyn/convert_dashboard.go @@ -2,6 +2,7 @@ package tfdyn import ( "context" + "encoding/json" "fmt" "github.com/databricks/cli/bundle/internal/tf/schema" @@ -10,6 +11,34 @@ import ( "github.com/databricks/cli/libs/log" ) +const ( + filePathFieldName = "file_path" + serializedDashboardFieldName = "serialized_dashboard" +) + +// Marshal "serialized_dashboard" as JSON if it is set in the input but not in the output. +func marshalSerializedDashboard(vin dyn.Value, vout dyn.Value) (dyn.Value, error) { + // Skip if the "serialized_dashboard" field is already set. + if v := vout.Get(serializedDashboardFieldName); v.IsValid() { + return vout, nil + } + + // Skip if the "serialized_dashboard" field on the input is not set. + v := vin.Get(serializedDashboardFieldName) + if !v.IsValid() { + return vout, nil + } + + // Marshal the "serialized_dashboard" field as JSON. + data, err := json.Marshal(v.AsAny()) + if err != nil { + return dyn.InvalidValue, fmt.Errorf("failed to marshal serialized_dashboard: %w", err) + } + + // Set the "serialized_dashboard" field on the output. + return dyn.Set(vout, serializedDashboardFieldName, dyn.V(string(data))) +} + func convertDashboardResource(ctx context.Context, vin dyn.Value) (dyn.Value, error) { var err error @@ -22,8 +51,8 @@ func convertDashboardResource(ctx context.Context, vin dyn.Value) (dyn.Value, er // Include "serialized_dashboard" field if "file_path" is set. // Note: the Terraform resource supports "file_path" natively, but its // change detection mechanism doesn't work as expected at the time of writing (Sep 30). - if path, ok := vout.Get("file_path").AsString(); ok { - vout, err = dyn.Set(vout, "serialized_dashboard", dyn.V(fmt.Sprintf("${file(\"%s\")}", path))) + if path, ok := vout.Get(filePathFieldName).AsString(); ok { + vout, err = dyn.Set(vout, serializedDashboardFieldName, dyn.V(fmt.Sprintf("${file(\"%s\")}", path))) if err != nil { return dyn.InvalidValue, fmt.Errorf("failed to set serialized_dashboard: %w", err) } @@ -33,7 +62,7 @@ func convertDashboardResource(ctx context.Context, vin dyn.Value) (dyn.Value, er case 0: return v, nil case 1: - if p[0] == dyn.Key("file_path") { + if p[0] == dyn.Key(filePathFieldName) { return v, dyn.ErrDrop } } @@ -46,6 +75,12 @@ func convertDashboardResource(ctx context.Context, vin dyn.Value) (dyn.Value, er } } + // Marshal "serialized_dashboard" as JSON if it is set in the input but not in the output. + vout, err = marshalSerializedDashboard(vin, vout) + if err != nil { + return dyn.InvalidValue, err + } + return vout, nil } diff --git a/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go b/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go index 0c4866524b..0794ce15c4 100644 --- a/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go +++ b/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go @@ -82,3 +82,48 @@ func TestConvertDashboardFilePath(t *testing.T) { "file_path": "some/path", }) } + +func TestConvertDashboardSerializedDashboardString(t *testing.T) { + var src = resources.Dashboard{ + SerializedDashboard: `{ "json": true }`, + } + + vin, err := convert.FromTyped(src, dyn.NilValue) + require.NoError(t, err) + + ctx := context.Background() + out := schema.NewResources() + err = dashboardConverter{}.Convert(ctx, "my_dashboard", vin, out) + require.NoError(t, err) + + // Assert that the "serialized_dashboard" is included. + assert.Subset(t, out.Dashboard["my_dashboard"], map[string]any{ + "serialized_dashboard": `{ "json": true }`, + }) +} + +func TestConvertDashboardSerializedDashboardAny(t *testing.T) { + var src = resources.Dashboard{ + SerializedDashboard: map[string]any{ + "pages": []map[string]any{ + { + "displayName": "New Page", + "layout": []map[string]any{}, + }, + }, + }, + } + + vin, err := convert.FromTyped(src, dyn.NilValue) + require.NoError(t, err) + + ctx := context.Background() + out := schema.NewResources() + err = dashboardConverter{}.Convert(ctx, "my_dashboard", vin, out) + require.NoError(t, err) + + // Assert that the "serialized_dashboard" is included. + assert.Subset(t, out.Dashboard["my_dashboard"], map[string]any{ + "serialized_dashboard": `{"pages":[{"displayName":"New Page","layout":[]}]}`, + }) +} From 72078bb09ca07df8df54b5769b566ac0352fabab Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 25 Oct 2024 12:16:16 +0200 Subject: [PATCH 34/39] Add integration test --- .../databricks_template_schema.json | 12 ++++ .../dashboards/template/dashboard.lvdash.json | 34 ++++++++++ .../dashboards/template/databricks.yml.tmpl | 12 ++++ internal/bundle/dashboards_test.go | 63 +++++++++++++++++++ internal/bundle/helpers.go | 22 +++++++ 5 files changed, 143 insertions(+) create mode 100644 internal/bundle/bundles/dashboards/databricks_template_schema.json create mode 100644 internal/bundle/bundles/dashboards/template/dashboard.lvdash.json create mode 100644 internal/bundle/bundles/dashboards/template/databricks.yml.tmpl create mode 100644 internal/bundle/dashboards_test.go diff --git a/internal/bundle/bundles/dashboards/databricks_template_schema.json b/internal/bundle/bundles/dashboards/databricks_template_schema.json new file mode 100644 index 0000000000..1aa5728fc1 --- /dev/null +++ b/internal/bundle/bundles/dashboards/databricks_template_schema.json @@ -0,0 +1,12 @@ +{ + "properties": { + "unique_id": { + "type": "string", + "description": "Unique ID for job name" + }, + "warehouse_id": { + "type": "string", + "description": "The SQL warehouse ID to use for the dashboard" + } + } +} diff --git a/internal/bundle/bundles/dashboards/template/dashboard.lvdash.json b/internal/bundle/bundles/dashboards/template/dashboard.lvdash.json new file mode 100644 index 0000000000..397a9a1259 --- /dev/null +++ b/internal/bundle/bundles/dashboards/template/dashboard.lvdash.json @@ -0,0 +1,34 @@ +{ + "pages": [ + { + "displayName": "New Page", + "layout": [ + { + "position": { + "height": 2, + "width": 6, + "x": 0, + "y": 0 + }, + "widget": { + "name": "82eb9107", + "textbox_spec": "# I'm a title" + } + }, + { + "position": { + "height": 2, + "width": 6, + "x": 0, + "y": 2 + }, + "widget": { + "name": "ffa6de4f", + "textbox_spec": "Text" + } + } + ], + "name": "fdd21a3c" + } + ] +} diff --git a/internal/bundle/bundles/dashboards/template/databricks.yml.tmpl b/internal/bundle/bundles/dashboards/template/databricks.yml.tmpl new file mode 100644 index 0000000000..e777123818 --- /dev/null +++ b/internal/bundle/bundles/dashboards/template/databricks.yml.tmpl @@ -0,0 +1,12 @@ +bundle: + name: dashboards + +workspace: + root_path: "~/.bundle/{{.unique_id}}" + +resources: + dashboards: + file_reference: + display_name: test-dashboard-{{.unique_id}} + file_path: ./dashboard.lvdash.json + warehouse_id: {{.warehouse_id}} diff --git a/internal/bundle/dashboards_test.go b/internal/bundle/dashboards_test.go new file mode 100644 index 0000000000..b12cc040c7 --- /dev/null +++ b/internal/bundle/dashboards_test.go @@ -0,0 +1,63 @@ +package bundle + +import ( + "fmt" + "testing" + + "github.com/databricks/cli/internal/acc" + "github.com/databricks/databricks-sdk-go/service/dashboards" + "github.com/databricks/databricks-sdk-go/service/workspace" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestAccDashboards(t *testing.T) { + ctx, wt := acc.WorkspaceTest(t) + + warehouseID := acc.GetEnvOrSkipTest(t, "TEST_DEFAULT_WAREHOUSE_ID") + uniqueID := uuid.New().String() + root, err := initTestTemplate(t, ctx, "dashboards", map[string]any{ + "unique_id": uniqueID, + "warehouse_id": warehouseID, + }) + require.NoError(t, err) + + t.Cleanup(func() { + err = destroyBundle(t, ctx, root) + require.NoError(t, err) + }) + + err = deployBundle(t, ctx, root) + require.NoError(t, err) + + // Load bundle configuration by running the validate command. + b := unmarshalConfig(t, mustValidateBundle(t, ctx, root)) + + // Assert that the dashboard exists at the expected path and is, indeed, a dashboard. + oi, err := wt.W.Workspace.GetStatusByPath(ctx, fmt.Sprintf("%s/test-dashboard-%s.lvdash.json", b.Config.Workspace.ResourcePath, uniqueID)) + require.NoError(t, err) + assert.EqualValues(t, workspace.ObjectTypeDashboard, oi.ObjectType) + + // Load the dashboard by its ID and confirm its display name. + dashboard, err := wt.W.Lakeview.GetByDashboardId(ctx, oi.ResourceId) + require.NoError(t, err) + assert.Equal(t, fmt.Sprintf("test-dashboard-%s", uniqueID), dashboard.DisplayName) + + // Make an out of band modification to the dashboard and confirm that it is detected. + _, err = wt.W.Lakeview.Update(ctx, dashboards.UpdateDashboardRequest{ + DashboardId: oi.ResourceId, + SerializedDashboard: dashboard.SerializedDashboard, + }) + require.NoError(t, err) + + // Try to redeploy the bundle and confirm that the out of band modification is detected. + stdout, _, err := deployBundleWithArgs(t, ctx, root) + require.Error(t, err) + assert.Contains(t, stdout, `Error: dashboard "file_reference" has been modified remotely`+"\n") + + // Redeploy the bundle with the --force flag and confirm that the out of band modification is ignored. + _, stderr, err := deployBundleWithArgs(t, ctx, root, "--force") + require.NoError(t, err) + assert.Contains(t, stderr, `Deployment complete!`+"\n") +} diff --git a/internal/bundle/helpers.go b/internal/bundle/helpers.go index b8c81a8d2c..8f1a866f65 100644 --- a/internal/bundle/helpers.go +++ b/internal/bundle/helpers.go @@ -11,6 +11,7 @@ import ( "strings" "testing" + "github.com/databricks/cli/bundle" "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/internal" "github.com/databricks/cli/libs/cmdio" @@ -66,6 +67,19 @@ func validateBundle(t *testing.T, ctx context.Context, path string) ([]byte, err return stdout.Bytes(), err } +func mustValidateBundle(t *testing.T, ctx context.Context, path string) []byte { + data, err := validateBundle(t, ctx, path) + require.NoError(t, err) + return data +} + +func unmarshalConfig(t *testing.T, data []byte) *bundle.Bundle { + bundle := &bundle.Bundle{} + err := json.Unmarshal(data, &bundle.Config) + require.NoError(t, err) + return bundle +} + func deployBundle(t *testing.T, ctx context.Context, path string) error { ctx = env.Set(ctx, "BUNDLE_ROOT", path) c := internal.NewCobraTestRunnerWithContext(t, ctx, "bundle", "deploy", "--force-lock", "--auto-approve") @@ -73,6 +87,14 @@ func deployBundle(t *testing.T, ctx context.Context, path string) error { return err } +func deployBundleWithArgs(t *testing.T, ctx context.Context, path string, args ...string) (string, string, error) { + ctx = env.Set(ctx, "BUNDLE_ROOT", path) + args = append([]string{"bundle", "deploy"}, args...) + c := internal.NewCobraTestRunnerWithContext(t, ctx, args...) + stdout, stderr, err := c.Run() + return stdout.String(), stderr.String(), err +} + func deployBundleWithFlags(t *testing.T, ctx context.Context, path string, flags []string) error { ctx = env.Set(ctx, "BUNDLE_ROOT", path) args := []string{"bundle", "deploy", "--force-lock"} From 21dabe87fd27369026b7eacd28da8fbf059a9868 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 25 Oct 2024 12:58:18 +0200 Subject: [PATCH 35/39] Quote path when passing to TF --- .../terraform/tfdyn/convert_dashboard.go | 2 +- .../terraform/tfdyn/convert_dashboard_test.go | 24 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/bundle/deploy/terraform/tfdyn/convert_dashboard.go b/bundle/deploy/terraform/tfdyn/convert_dashboard.go index 5d3a5d14f7..3ba7e19a2d 100644 --- a/bundle/deploy/terraform/tfdyn/convert_dashboard.go +++ b/bundle/deploy/terraform/tfdyn/convert_dashboard.go @@ -52,7 +52,7 @@ func convertDashboardResource(ctx context.Context, vin dyn.Value) (dyn.Value, er // Note: the Terraform resource supports "file_path" natively, but its // change detection mechanism doesn't work as expected at the time of writing (Sep 30). if path, ok := vout.Get(filePathFieldName).AsString(); ok { - vout, err = dyn.Set(vout, serializedDashboardFieldName, dyn.V(fmt.Sprintf("${file(\"%s\")}", path))) + vout, err = dyn.Set(vout, serializedDashboardFieldName, dyn.V(fmt.Sprintf("${file(%q)}", path))) if err != nil { return dyn.InvalidValue, fmt.Errorf("failed to set serialized_dashboard: %w", err) } diff --git a/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go b/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go index 0794ce15c4..9cefbc10e0 100644 --- a/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go +++ b/bundle/deploy/terraform/tfdyn/convert_dashboard_test.go @@ -83,6 +83,30 @@ func TestConvertDashboardFilePath(t *testing.T) { }) } +func TestConvertDashboardFilePathQuoted(t *testing.T) { + var src = resources.Dashboard{ + FilePath: `C:\foo\bar\baz\dashboard.lvdash.json`, + } + + vin, err := convert.FromTyped(src, dyn.NilValue) + require.NoError(t, err) + + ctx := context.Background() + out := schema.NewResources() + err = dashboardConverter{}.Convert(ctx, "my_dashboard", vin, out) + require.NoError(t, err) + + // Assert that the "serialized_dashboard" is included. + assert.Subset(t, out.Dashboard["my_dashboard"], map[string]any{ + "serialized_dashboard": `${file("C:\\foo\\bar\\baz\\dashboard.lvdash.json")}`, + }) + + // Assert that the "file_path" doesn't carry over. + assert.NotSubset(t, out.Dashboard["my_dashboard"], map[string]any{ + "file_path": `C:\foo\bar\baz\dashboard.lvdash.json`, + }) +} + func TestConvertDashboardSerializedDashboardString(t *testing.T) { var src = resources.Dashboard{ SerializedDashboard: `{ "json": true }`, From 2348e852acd20747baf31a78234a36516b7f45c3 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 25 Oct 2024 15:28:20 +0200 Subject: [PATCH 36/39] Update schema --- bundle/schema/jsonschema.json | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/bundle/schema/jsonschema.json b/bundle/schema/jsonschema.json index 788b0e1f06..62e5fe6d8c 100644 --- a/bundle/schema/jsonschema.json +++ b/bundle/schema/jsonschema.json @@ -186,6 +186,7 @@ "type": "object", "properties": { "display_name": { + "description": "The display name of the dashboard.", "$ref": "#/$defs/string" }, "embed_credentials": { @@ -195,22 +196,24 @@ "$ref": "#/$defs/string" }, "parent_path": { + "description": "The workspace path of the folder containing the dashboard. Includes leading slash and no\ntrailing slash.\nThis field is excluded in List Dashboards responses.", "$ref": "#/$defs/string" }, "permissions": { "$ref": "#/$defs/slice/github.com/databricks/cli/bundle/config/resources.Permission" }, "serialized_dashboard": { + "description": "The contents of the dashboard in serialized string form.\nThis field is excluded in List Dashboards responses.\nUse the [get dashboard API](https://docs.databricks.com/api/workspace/lakeview/get)\nto retrieve an example response, which includes the `serialized_dashboard` field.\nThis field provides the structure of the JSON string that represents the dashboard's\nlayout and components.", "$ref": "#/$defs/interface" }, "warehouse_id": { + "description": "The warehouse ID used to run the dashboard.", "$ref": "#/$defs/string" } }, "additionalProperties": false, "required": [ - "display_name", - "warehouse_id" + "display_name" ] }, { From bfa9c3a2c35414e02206dcad2de6f3297df36f0c Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 28 Oct 2024 17:15:31 +0100 Subject: [PATCH 37/39] Completion for the resource flag --- cmd/bundle/generate/dashboard.go | 39 ++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/cmd/bundle/generate/dashboard.go b/cmd/bundle/generate/dashboard.go index e139c4b1c7..4d75eca568 100644 --- a/cmd/bundle/generate/dashboard.go +++ b/cmd/bundle/generate/dashboard.go @@ -1,6 +1,7 @@ package generate import ( + "bytes" "context" "encoding/json" "errors" @@ -16,6 +17,7 @@ import ( "github.com/databricks/cli/bundle/deploy/terraform" "github.com/databricks/cli/bundle/phases" "github.com/databricks/cli/bundle/render" + "github.com/databricks/cli/bundle/resources" "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" @@ -25,6 +27,7 @@ import ( "github.com/databricks/databricks-sdk-go/service/dashboards" "github.com/databricks/databricks-sdk-go/service/workspace" "github.com/spf13/cobra" + "golang.org/x/exp/maps" "gopkg.in/yaml.v3" ) @@ -129,11 +132,20 @@ func remarshalJSON(data []byte) ([]byte, error) { if err != nil { return nil, err } - out, err := json.MarshalIndent(tmp, "", " ") + + // Remarshal the data to ensure its formatting is stable. + // The result will have alphabetically sorted keys and be indented. + // HTML escaping is disabled to retain characters such as &, <, and >. + var buf bytes.Buffer + enc := json.NewEncoder(&buf) + enc.SetIndent("", " ") + enc.SetEscapeHTML(false) + err = enc.Encode(tmp) if err != nil { return nil, err } - return out, nil + + return buf.Bytes(), nil } func (d *dashboard) saveSerializedDashboard(_ context.Context, b *bundle.Bundle, dashboard *dashboards.Dashboard, filename string) error { @@ -378,6 +390,26 @@ func (d *dashboard) RunE(cmd *cobra.Command, args []string) error { return nil } +// filterDashboards returns a filter that only includes dashboards. +func filterDashboards(ref resources.Reference) bool { + return ref.Description.SingularName == "dashboard" +} + +// dashboardResourceCompletion executes to autocomplete the argument to the resource flag. +func dashboardResourceCompletion(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + b, diags := root.MustConfigureBundle(cmd) + if err := diags.Error(); err != nil { + cobra.CompErrorln(err.Error()) + return nil, cobra.ShellCompDirectiveError + } + + if b == nil { + return nil, cobra.ShellCompDirectiveNoFileComp + } + + return maps.Keys(resources.Completions(b, filterDashboards)), cobra.ShellCompDirectiveNoFileComp +} + func NewGenerateDashboardCommand() *cobra.Command { cmd := &cobra.Command{ Use: "dashboard", @@ -409,6 +441,9 @@ func NewGenerateDashboardCommand() *cobra.Command { cmd.MarkFlagsMutuallyExclusive("watch", "existing-path") cmd.MarkFlagsMutuallyExclusive("watch", "existing-id") + // Completion for the resource flag. + cmd.RegisterFlagCompletionFunc("resource", dashboardResourceCompletion) + cmd.RunE = d.RunE return cmd } From 1f26c68c04f0b58c3d1acf68dc26f3a833bdd598 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 29 Oct 2024 09:16:11 +0100 Subject: [PATCH 38/39] Comments --- cmd/bundle/generate/dashboard.go | 90 ++++++++++++++++----------- cmd/bundle/generate/dashboard_test.go | 2 +- 2 files changed, 55 insertions(+), 37 deletions(-) diff --git a/cmd/bundle/generate/dashboard.go b/cmd/bundle/generate/dashboard.go index 4d75eca568..4a538a2937 100644 --- a/cmd/bundle/generate/dashboard.go +++ b/cmd/bundle/generate/dashboard.go @@ -23,6 +23,7 @@ import ( "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/dyn/yamlsaver" "github.com/databricks/cli/libs/textutil" + "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/apierr" "github.com/databricks/databricks-sdk-go/service/dashboards" "github.com/databricks/databricks-sdk-go/service/workspace" @@ -33,8 +34,8 @@ import ( type dashboard struct { // Lookup flags for one-time generate. - dashboardPath string - dashboardId string + existingPath string + existingID string // Lookup flag for existing bundle resource. resource string @@ -55,9 +56,9 @@ type dashboard struct { func (d *dashboard) resolveID(ctx context.Context, b *bundle.Bundle) (string, diag.Diagnostics) { switch { - case d.dashboardPath != "": + case d.existingPath != "": return d.resolveFromPath(ctx, b) - case d.dashboardId != "": + case d.existingID != "": return d.resolveFromID(ctx, b) } @@ -66,10 +67,10 @@ func (d *dashboard) resolveID(ctx context.Context, b *bundle.Bundle) (string, di func (d *dashboard) resolveFromPath(ctx context.Context, b *bundle.Bundle) (string, diag.Diagnostics) { w := b.WorkspaceClient() - obj, err := w.Workspace.GetStatusByPath(ctx, d.dashboardPath) + obj, err := w.Workspace.GetStatusByPath(ctx, d.existingPath) if err != nil { if apierr.IsMissing(err) { - return "", diag.Errorf("dashboard %q not found", path.Base(d.dashboardPath)) + return "", diag.Errorf("dashboard %q not found", path.Base(d.existingPath)) } // Emit a more descriptive error message for legacy dashboards. @@ -77,7 +78,7 @@ func (d *dashboard) resolveFromPath(ctx context.Context, b *bundle.Bundle) (stri return "", diag.Diagnostics{ { Severity: diag.Error, - Summary: fmt.Sprintf("dashboard %q is a legacy dashboard", path.Base(d.dashboardPath)), + Summary: fmt.Sprintf("dashboard %q is a legacy dashboard", path.Base(d.existingPath)), Detail: "" + "Databricks Asset Bundles work exclusively with AI/BI dashboards.\n" + "\n" + @@ -114,10 +115,10 @@ func (d *dashboard) resolveFromPath(ctx context.Context, b *bundle.Bundle) (stri func (d *dashboard) resolveFromID(ctx context.Context, b *bundle.Bundle) (string, diag.Diagnostics) { w := b.WorkspaceClient() - obj, err := w.Lakeview.GetByDashboardId(ctx, d.dashboardId) + obj, err := w.Lakeview.GetByDashboardId(ctx, d.existingID) if err != nil { if apierr.IsMissing(err) { - return "", diag.Errorf("dashboard with ID %s not found", d.dashboardId) + return "", diag.Errorf("dashboard with ID %s not found", d.existingID) } return "", diag.FromErr(err) } @@ -234,7 +235,32 @@ func (d *dashboard) saveConfiguration(ctx context.Context, b *bundle.Bundle, das return nil } -func (d *dashboard) generateForResource(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { +func waitForChanges(ctx context.Context, w *databricks.WorkspaceClient, dashboard *dashboards.Dashboard) diag.Diagnostics { + // Compute [time.Time] for the most recent update. + tref, err := time.Parse(time.RFC3339, dashboard.UpdateTime) + if err != nil { + return diag.FromErr(err) + } + + for { + obj, err := w.Workspace.GetStatusByPath(ctx, dashboard.Path) + if err != nil { + return diag.FromErr(err) + } + + // Compute [time.Time] from timestamp in millis since epoch. + tcur := time.Unix(0, obj.ModifiedAt*int64(time.Millisecond)) + if tcur.After(tref) { + break + } + + time.Sleep(1 * time.Second) + } + + return nil +} + +func (d *dashboard) updateDashboardForResource(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) @@ -250,10 +276,11 @@ func (d *dashboard) generateForResource(ctx context.Context, b *bundle.Bundle) d // Overwrite the dashboard at the path referenced from the resource. dashboardPath := resource.FilePath + w := b.WorkspaceClient() + // Start polling the underlying dashboard for changes. var etag string for { - w := b.WorkspaceClient() dashboard, err := w.Lakeview.GetByDashboardId(ctx, dashboardID) if err != nil { return diag.FromErr(err) @@ -274,28 +301,11 @@ func (d *dashboard) generateForResource(ctx context.Context, b *bundle.Bundle) d // Update the etag for the next iteration. etag = dashboard.Etag - // Compute [time.Time] for the most recent update. - tref, err := time.Parse(time.RFC3339, dashboard.UpdateTime) - if err != nil { - return diag.FromErr(err) - } - // Now poll the workspace API for changes. - // This is much more efficient than polling the dashboard API. - for { - obj, err := w.Workspace.GetStatusByPath(ctx, dashboard.Path) - if err != nil { - return diag.FromErr(err) - } - - // Compute [time.Time] from timestamp in millis since epoch. - tcur := time.Unix(0, obj.ModifiedAt*int64(time.Millisecond)) - if tcur.After(tref) { - break - } - - time.Sleep(1 * time.Second) - } + // This is much more efficient than polling the dashboard API because it + // includes the entire serialized dashboard whereas we're only interested + // in the last modified time of the dashboard here. + waitForChanges(ctx, w, dashboard) } } @@ -346,7 +356,7 @@ func (d *dashboard) runForResource(ctx context.Context, b *bundle.Bundle) diag.D return diags } - return d.generateForResource(ctx, b) + return d.updateDashboardForResource(ctx, b) } func (d *dashboard) runForExisting(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { @@ -419,22 +429,30 @@ func NewGenerateDashboardCommand() *cobra.Command { d := &dashboard{} // 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.existingPath, "existing-path", "", `workspace path of the dashboard to generate configuration for`) + cmd.Flags().StringVar(&d.existingID, "existing-id", "", `ID of the dashboard to generate configuration for`) cmd.Flags().StringVar(&d.resource, "resource", "", `resource key of dashboard to watch for changes`) + // Alias lookup flags that include the resource type name. + // Included for symmetry with the other generate commands, but we prefer the shorter flags. + cmd.Flags().StringVar(&d.existingPath, "existing-dashboard-path", "", `workspace path of the dashboard to generate configuration for`) + cmd.Flags().StringVar(&d.existingID, "existing-dashboard-id", "", `ID of the dashboard to generate configuration for`) + cmd.Flags().MarkHidden("existing-dashboard-path") + cmd.Flags().MarkHidden("existing-dashboard-id") + // Output flags. cmd.Flags().StringVarP(&d.resourceDir, "resource-dir", "d", "./resources", `directory to write the configuration to`) cmd.Flags().StringVarP(&d.dashboardDir, "dashboard-dir", "s", "./src", `directory to write the dashboard representation to`) cmd.Flags().BoolVarP(&d.force, "force", "f", false, `force overwrite existing files in the output directory`) + // Exactly one of the lookup flags must be provided. cmd.MarkFlagsOneRequired( "existing-path", "existing-id", "resource", ) - // Watch flags. + // Watch flag. This is relevant only in combination with the resource flag. 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. diff --git a/cmd/bundle/generate/dashboard_test.go b/cmd/bundle/generate/dashboard_test.go index 8df6650795..517ca19c15 100644 --- a/cmd/bundle/generate/dashboard_test.go +++ b/cmd/bundle/generate/dashboard_test.go @@ -22,7 +22,7 @@ func TestDashboard_ErrorOnLegacyDashboard(t *testing.T) { // < } d := dashboard{ - dashboardPath: "/path/to/legacy dashboard", + existingPath: "/path/to/legacy dashboard", } m := mocks.NewMockWorkspaceClient(t) From d1cc35f4fddf70b4eb4ce43a6e107198bae421e1 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 29 Oct 2024 11:24:02 +0100 Subject: [PATCH 39/39] Test coverage --- cmd/bundle/generate/dashboard_test.go | 139 ++++++++++++++++++++++++++ 1 file changed, 139 insertions(+) diff --git a/cmd/bundle/generate/dashboard_test.go b/cmd/bundle/generate/dashboard_test.go index 517ca19c15..6741e6a39e 100644 --- a/cmd/bundle/generate/dashboard_test.go +++ b/cmd/bundle/generate/dashboard_test.go @@ -2,11 +2,16 @@ package generate import ( "context" + "os" + "path/filepath" "testing" "github.com/databricks/cli/bundle" "github.com/databricks/databricks-sdk-go/apierr" "github.com/databricks/databricks-sdk-go/experimental/mocks" + "github.com/databricks/databricks-sdk-go/service/dashboards" + "github.com/databricks/databricks-sdk-go/service/workspace" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -41,3 +46,137 @@ func TestDashboard_ErrorOnLegacyDashboard(t *testing.T) { require.Len(t, diags, 1) assert.Equal(t, diags[0].Summary, "dashboard \"legacy dashboard\" is a legacy dashboard") } + +func TestDashboard_ExistingID_Nominal(t *testing.T) { + root := t.TempDir() + b := &bundle.Bundle{ + BundleRootPath: root, + } + + m := mocks.NewMockWorkspaceClient(t) + b.SetWorkpaceClient(m.WorkspaceClient) + + dashboardsAPI := m.GetMockLakeviewAPI() + dashboardsAPI.EXPECT().GetByDashboardId(mock.Anything, "f00dcafe").Return(&dashboards.Dashboard{ + DashboardId: "f00dcafe", + DisplayName: "This is a test dashboard", + SerializedDashboard: `{"pages":[{"displayName":"New Page","layout":[],"name":"12345678"}]}`, + WarehouseId: "w4r3h0us3", + }, nil) + + ctx := bundle.Context(context.Background(), b) + cmd := NewGenerateDashboardCommand() + cmd.SetContext(ctx) + cmd.Flag("existing-id").Value.Set("f00dcafe") + + err := cmd.RunE(cmd, []string{}) + require.NoError(t, err) + + // Assert the contents of the generated configuration + data, err := os.ReadFile(filepath.Join(root, "resources", "this_is_a_test_dashboard.dashboard.yml")) + require.NoError(t, err) + assert.Equal(t, `resources: + dashboards: + this_is_a_test_dashboard: + display_name: "This is a test dashboard" + warehouse_id: w4r3h0us3 + file_path: ../src/this_is_a_test_dashboard.lvdash.json +`, string(data)) + + data, err = os.ReadFile(filepath.Join(root, "src", "this_is_a_test_dashboard.lvdash.json")) + require.NoError(t, err) + assert.JSONEq(t, `{"pages":[{"displayName":"New Page","layout":[],"name":"12345678"}]}`, string(data)) +} + +func TestDashboard_ExistingID_NotFound(t *testing.T) { + root := t.TempDir() + b := &bundle.Bundle{ + BundleRootPath: root, + } + + m := mocks.NewMockWorkspaceClient(t) + b.SetWorkpaceClient(m.WorkspaceClient) + + dashboardsAPI := m.GetMockLakeviewAPI() + dashboardsAPI.EXPECT().GetByDashboardId(mock.Anything, "f00dcafe").Return(nil, &apierr.APIError{ + StatusCode: 404, + }) + + ctx := bundle.Context(context.Background(), b) + cmd := NewGenerateDashboardCommand() + cmd.SetContext(ctx) + cmd.Flag("existing-id").Value.Set("f00dcafe") + + err := cmd.RunE(cmd, []string{}) + require.Error(t, err) +} + +func TestDashboard_ExistingPath_Nominal(t *testing.T) { + root := t.TempDir() + b := &bundle.Bundle{ + BundleRootPath: root, + } + + m := mocks.NewMockWorkspaceClient(t) + b.SetWorkpaceClient(m.WorkspaceClient) + + workspaceAPI := m.GetMockWorkspaceAPI() + workspaceAPI.EXPECT().GetStatusByPath(mock.Anything, "/path/to/dashboard").Return(&workspace.ObjectInfo{ + ObjectType: workspace.ObjectTypeDashboard, + ResourceId: "f00dcafe", + }, nil) + + dashboardsAPI := m.GetMockLakeviewAPI() + dashboardsAPI.EXPECT().GetByDashboardId(mock.Anything, "f00dcafe").Return(&dashboards.Dashboard{ + DashboardId: "f00dcafe", + DisplayName: "This is a test dashboard", + SerializedDashboard: `{"pages":[{"displayName":"New Page","layout":[],"name":"12345678"}]}`, + WarehouseId: "w4r3h0us3", + }, nil) + + ctx := bundle.Context(context.Background(), b) + cmd := NewGenerateDashboardCommand() + cmd.SetContext(ctx) + cmd.Flag("existing-path").Value.Set("/path/to/dashboard") + + err := cmd.RunE(cmd, []string{}) + require.NoError(t, err) + + // Assert the contents of the generated configuration + data, err := os.ReadFile(filepath.Join(root, "resources", "this_is_a_test_dashboard.dashboard.yml")) + require.NoError(t, err) + assert.Equal(t, `resources: + dashboards: + this_is_a_test_dashboard: + display_name: "This is a test dashboard" + warehouse_id: w4r3h0us3 + file_path: ../src/this_is_a_test_dashboard.lvdash.json +`, string(data)) + + data, err = os.ReadFile(filepath.Join(root, "src", "this_is_a_test_dashboard.lvdash.json")) + require.NoError(t, err) + assert.JSONEq(t, `{"pages":[{"displayName":"New Page","layout":[],"name":"12345678"}]}`, string(data)) +} + +func TestDashboard_ExistingPath_NotFound(t *testing.T) { + root := t.TempDir() + b := &bundle.Bundle{ + BundleRootPath: root, + } + + m := mocks.NewMockWorkspaceClient(t) + b.SetWorkpaceClient(m.WorkspaceClient) + + workspaceAPI := m.GetMockWorkspaceAPI() + workspaceAPI.EXPECT().GetStatusByPath(mock.Anything, "/path/to/dashboard").Return(nil, &apierr.APIError{ + StatusCode: 404, + }) + + ctx := bundle.Context(context.Background(), b) + cmd := NewGenerateDashboardCommand() + cmd.SetContext(ctx) + cmd.Flag("existing-path").Value.Set("/path/to/dashboard") + + err := cmd.RunE(cmd, []string{}) + require.Error(t, err) +}