From 3461c66dc947615476f259ce5f61b4cd2f0ccb6d Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 3 Sep 2024 10:35:41 +0200 Subject: [PATCH 01/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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 5b6a7b2e7c19106e520e7fc135f667e54959ac6e Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 22 Oct 2024 14:56:48 +0200 Subject: [PATCH 25/32] 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 866bfc5be7f9b87e562872c7473ac7e054ef5cd4 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 24 Oct 2024 13:35:22 +0200 Subject: [PATCH 26/32] 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 27/32] 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 28/32] 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 29/32] 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 30/32] 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 31/32] 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 32/32] 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" ] }, {