From 869637dc54161be696d5d56fb60995e0696c681a Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Sat, 24 Aug 2024 20:48:36 +0200 Subject: [PATCH 01/15] Add a textual bundle summary command --- bundle/config/mutator/initialize_urls.go | 62 +++++++++ bundle/config/mutator/initialize_urls_test.go | 124 ++++++++++++++++++ bundle/config/resources.go | 63 +++++++++ bundle/config/resources/job.go | 16 +++ bundle/config/resources/mlflow_experiment.go | 16 +++ bundle/config/resources/mlflow_model.go | 16 +++ .../resources/model_serving_endpoint.go | 16 +++ bundle/config/resources/pipeline.go | 16 +++ bundle/config/resources/quality_monitor.go | 17 +++ bundle/config/resources/registered_model.go | 17 +++ bundle/config/resources/schema.go | 29 ++++ bundle/config/resources_test.go | 20 +++ bundle/render/render_text_output.go | 118 +++++++++++++---- bundle/render/render_text_output_test.go | 103 ++++++++++++++- cmd/bundle/deploy.go | 2 +- cmd/bundle/summary.go | 12 +- cmd/bundle/validate.go | 2 +- 17 files changed, 612 insertions(+), 37 deletions(-) create mode 100644 bundle/config/mutator/initialize_urls.go create mode 100644 bundle/config/mutator/initialize_urls_test.go diff --git a/bundle/config/mutator/initialize_urls.go b/bundle/config/mutator/initialize_urls.go new file mode 100644 index 0000000000..e403d30d67 --- /dev/null +++ b/bundle/config/mutator/initialize_urls.go @@ -0,0 +1,62 @@ +package mutator + +import ( + "context" + "fmt" + "strconv" + "strings" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" +) + +type initializeUrls struct { + name string +} + +// InitializeURLs makes sure the URL field of each resource is configured. +// NOTE: since this depends on an extra API call, this mutator adds some extra +// latency. As such, it should only be used when needed. +// This URL field is used for the output of the 'bundle summary' CLI command. +func InitializeURLs() bundle.Mutator { + return &initializeUrls{} +} + +func (m *initializeUrls) Name() string { + return fmt.Sprintf("ConfigureURLs(%s)", m.name) +} + +func (m *initializeUrls) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + workspaceId, err := b.WorkspaceClient().CurrentWorkspaceID(ctx) + orgId := strconv.FormatInt(workspaceId, 10) + if err != nil { + return diag.FromErr(err) + } + configureForOrgId(b, orgId) + return nil +} + +func configureForOrgId(b *bundle.Bundle, orgId string) { + urlPrefix := b.Config.Workspace.Host + if !strings.HasSuffix(urlPrefix, "/") { + urlPrefix += "/" + } + urlSuffix := "" + + // Add ?o= only if wasn't in the subdomain already. + // The ?o= is needed when vanity URLs / legacy workspace URLs are used. + // If it's not needed we prefer to leave it out since these URLs are rather + // long for most terminals. + // + // See https://docs.databricks.com/en/workspace/workspace-details.html for + // further reading about the '?o=' suffix. + if !strings.Contains(urlPrefix, orgId) { + urlSuffix = "?o=" + orgId + } + + for _, rs := range b.Config.Resources.AllResources() { + for _, r := range rs { + r.InitializeURL(urlPrefix, urlSuffix) + } + } +} diff --git a/bundle/config/mutator/initialize_urls_test.go b/bundle/config/mutator/initialize_urls_test.go new file mode 100644 index 0000000000..5ef13119ca --- /dev/null +++ b/bundle/config/mutator/initialize_urls_test.go @@ -0,0 +1,124 @@ +package mutator + +import ( + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/databricks-sdk-go/service/catalog" + "github.com/databricks/databricks-sdk-go/service/jobs" + "github.com/databricks/databricks-sdk-go/service/ml" + "github.com/databricks/databricks-sdk-go/service/pipelines" + "github.com/databricks/databricks-sdk-go/service/serving" + "github.com/stretchr/testify/require" +) + +func TestIntitializeURLs(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + Host: "https://mycompany.databricks.com/", + }, + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job1": { + ID: "1", + JobSettings: &jobs.JobSettings{Name: "job1"}, + }, + }, + Pipelines: map[string]*resources.Pipeline{ + "pipeline1": { + ID: "3", + PipelineSpec: &pipelines.PipelineSpec{Name: "pipeline1"}, + }, + }, + Experiments: map[string]*resources.MlflowExperiment{ + "experiment1": { + ID: "4", + Experiment: &ml.Experiment{Name: "experiment1"}, + }, + }, + Models: map[string]*resources.MlflowModel{ + "model1": { + ID: "6", + Model: &ml.Model{Name: "model1"}, + }, + }, + ModelServingEndpoints: map[string]*resources.ModelServingEndpoint{ + "servingendpoint1": { + ID: "7", + CreateServingEndpoint: &serving.CreateServingEndpoint{ + Name: "my_serving_endpoint", + }, + }, + }, + RegisteredModels: map[string]*resources.RegisteredModel{ + "registeredmodel1": { + ID: "8", + CreateRegisteredModelRequest: &catalog.CreateRegisteredModelRequest{ + Name: "my_registered_model", + }, + }, + }, + QualityMonitors: map[string]*resources.QualityMonitor{ + "qualityMonitor1": { + CreateMonitor: &catalog.CreateMonitor{ + TableName: "catalog.schema.qualityMonitor1", + }, + }, + }, + Schemas: map[string]*resources.Schema{ + "schema1": { + ID: "catalog.schema", + CreateSchema: &catalog.CreateSchema{ + Name: "schema", + }, + }, + }, + }, + }, + } + + expectedURLs := map[string]string{ + "job1": "https://mycompany.databricks.com/jobs/1?o=123456", + "pipeline1": "https://mycompany.databricks.com/pipelines/3?o=123456", + "experiment1": "https://mycompany.databricks.com/ml/experiments/4?o=123456", + "model1": "https://mycompany.databricks.com/ml/models/model1?o=123456", + "servingendpoint1": "https://mycompany.databricks.com/ml/endpoints/my_serving_endpoint?o=123456", + "registeredmodel1": "https://mycompany.databricks.com/explore/data/models/8?o=123456", + "qualityMonitor1": "https://mycompany.databricks.com/explore/data/catalog/schema/qualityMonitor1?o=123456", + "schema1": "https://mycompany.databricks.com/explore/data/catalog/schema?o=123456", + } + + configureForOrgId(b, "123456") + + for _, rs := range b.Config.Resources.AllResources() { + for key, r := range rs { + require.Equal(t, expectedURLs[key], r.GetURL(), "Unexpected URL for "+key) + } + } +} + +func TestIntitializeURLs_NoOrgId(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + // Hostname with org id in URL and no trailing / + Host: "https://adb-123456.azuredatabricks.net", + }, + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job1": { + ID: "1", + JobSettings: &jobs.JobSettings{Name: "job1"}, + }, + }, + }, + }, + } + + configureForOrgId(b, "123456") + + require.Equal(t, "https://adb-123456.azuredatabricks.net/jobs/1", b.Config.Resources.Jobs["job1"].URL) +} diff --git a/bundle/config/resources.go b/bundle/config/resources.go index 22d69ffb53..3cee9abe38 100644 --- a/bundle/config/resources.go +++ b/bundle/config/resources.go @@ -29,6 +29,69 @@ type ConfigResource interface { // Terraform equivalent name of the resource. For example "databricks_job" // for jobs and "databricks_pipeline" for pipelines. TerraformResourceName() string + + // GetName returns the in-product name of the resource. + GetName() string + + // GetURL returns the URL of the resource. + GetURL() string + + // InitializeURL initializes the URL field of the resource. + InitializeURL(urlPrefix string, urlSuffix string) +} + +func (r *Resources) AllResources() map[string]map[string]ConfigResource { + result := make(map[string]map[string]ConfigResource) + + jobResources := make(map[string]ConfigResource) + for key, job := range r.Jobs { + jobResources[key] = job + } + result["jobs"] = jobResources + + pipelineResources := make(map[string]ConfigResource) + for key, pipeline := range r.Pipelines { + pipelineResources[key] = pipeline + } + result["pipelines"] = pipelineResources + + modelResources := make(map[string]ConfigResource) + for key, model := range r.Models { + modelResources[key] = model + } + result["models"] = modelResources + + experimentResources := make(map[string]ConfigResource) + for key, experiment := range r.Experiments { + experimentResources[key] = experiment + } + result["experiments"] = experimentResources + + modelServingEndpointResources := make(map[string]ConfigResource) + for key, endpoint := range r.ModelServingEndpoints { + modelServingEndpointResources[key] = endpoint + } + result["model_serving_endpoints"] = modelServingEndpointResources + + registeredModelResources := make(map[string]ConfigResource) + for key, registeredModel := range r.RegisteredModels { + registeredModelResources[key] = registeredModel + } + result["registered_models"] = registeredModelResources + + qualityMonitorResources := make(map[string]ConfigResource) + for key, qualityMonitor := range r.QualityMonitors { + qualityMonitorResources[key] = qualityMonitor + } + result["quality_monitors"] = qualityMonitorResources + + schemaResources := make(map[string]ConfigResource) + for key, schema := range r.Schemas { + schemaResources[key] = schema + } + result["schemas"] = schemaResources + + return result } func (r *Resources) FindResourceByConfigKey(key string) (ConfigResource, error) { diff --git a/bundle/config/resources/job.go b/bundle/config/resources/job.go index d8f97a2db6..6c119c3eb9 100644 --- a/bundle/config/resources/job.go +++ b/bundle/config/resources/job.go @@ -14,6 +14,7 @@ type Job 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"` *jobs.JobSettings } @@ -44,3 +45,18 @@ func (j *Job) Exists(ctx context.Context, w *databricks.WorkspaceClient, id stri func (j *Job) TerraformResourceName() string { return "databricks_job" } + +func (j *Job) InitializeURL(urlPrefix string, urlSuffix string) { + if j.ID == "" { + return + } + j.URL = urlPrefix + "jobs/" + j.ID + urlSuffix +} + +func (j *Job) GetName() string { + return j.Name +} + +func (j *Job) GetURL() string { + return j.URL +} diff --git a/bundle/config/resources/mlflow_experiment.go b/bundle/config/resources/mlflow_experiment.go index 0ab4864360..e9cb2c0bb1 100644 --- a/bundle/config/resources/mlflow_experiment.go +++ b/bundle/config/resources/mlflow_experiment.go @@ -13,6 +13,7 @@ type MlflowExperiment 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"` *ml.Experiment } @@ -39,3 +40,18 @@ func (s *MlflowExperiment) Exists(ctx context.Context, w *databricks.WorkspaceCl func (s *MlflowExperiment) TerraformResourceName() string { return "databricks_mlflow_experiment" } + +func (s *MlflowExperiment) InitializeURL(urlPrefix string, urlSuffix string) { + if s.ID == "" { + return + } + s.URL = urlPrefix + "ml/experiments/" + s.ID + urlSuffix +} + +func (s *MlflowExperiment) GetName() string { + return s.Name +} + +func (s *MlflowExperiment) GetURL() string { + return s.URL +} diff --git a/bundle/config/resources/mlflow_model.go b/bundle/config/resources/mlflow_model.go index 300474e359..1fd681a1b9 100644 --- a/bundle/config/resources/mlflow_model.go +++ b/bundle/config/resources/mlflow_model.go @@ -13,6 +13,7 @@ type MlflowModel 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"` *ml.Model } @@ -39,3 +40,18 @@ func (s *MlflowModel) Exists(ctx context.Context, w *databricks.WorkspaceClient, func (s *MlflowModel) TerraformResourceName() string { return "databricks_mlflow_model" } + +func (s *MlflowModel) InitializeURL(urlPrefix string, urlSuffix string) { + if s.ID == "" { + return + } + s.URL = urlPrefix + "ml/models/" + s.Name + urlSuffix +} + +func (s *MlflowModel) GetName() string { + return s.Name +} + +func (s *MlflowModel) GetURL() string { + return s.URL +} diff --git a/bundle/config/resources/model_serving_endpoint.go b/bundle/config/resources/model_serving_endpoint.go index 5efb7ea267..f8981707be 100644 --- a/bundle/config/resources/model_serving_endpoint.go +++ b/bundle/config/resources/model_serving_endpoint.go @@ -23,6 +23,7 @@ type ModelServingEndpoint struct { Permissions []Permission `json:"permissions,omitempty"` ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` + URL string `json:"url,omitempty" bundle:"internal"` } func (s *ModelServingEndpoint) UnmarshalJSON(b []byte) error { @@ -47,3 +48,18 @@ func (s *ModelServingEndpoint) Exists(ctx context.Context, w *databricks.Workspa func (s *ModelServingEndpoint) TerraformResourceName() string { return "databricks_model_serving" } + +func (s *ModelServingEndpoint) InitializeURL(urlPrefix string, urlSuffix string) { + if s.ID == "" { + return + } + s.URL = urlPrefix + "ml/endpoints/" + s.Name + urlSuffix +} + +func (s *ModelServingEndpoint) GetName() string { + return s.Name +} + +func (s *ModelServingEndpoint) GetURL() string { + return s.URL +} diff --git a/bundle/config/resources/pipeline.go b/bundle/config/resources/pipeline.go index 55270be654..63518001f7 100644 --- a/bundle/config/resources/pipeline.go +++ b/bundle/config/resources/pipeline.go @@ -13,6 +13,7 @@ type Pipeline 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"` *pipelines.PipelineSpec } @@ -39,3 +40,18 @@ func (p *Pipeline) Exists(ctx context.Context, w *databricks.WorkspaceClient, id func (p *Pipeline) TerraformResourceName() string { return "databricks_pipeline" } + +func (p *Pipeline) InitializeURL(urlPrefix string, urlSuffix string) { + if p.ID == "" { + return + } + p.URL = urlPrefix + "pipelines/" + p.ID + urlSuffix +} + +func (p *Pipeline) GetName() string { + return p.Name +} + +func (s *Pipeline) GetURL() string { + return s.URL +} diff --git a/bundle/config/resources/quality_monitor.go b/bundle/config/resources/quality_monitor.go index 9160782cd0..6cf234a947 100644 --- a/bundle/config/resources/quality_monitor.go +++ b/bundle/config/resources/quality_monitor.go @@ -2,6 +2,7 @@ package resources import ( "context" + "strings" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go" @@ -20,6 +21,7 @@ type QualityMonitor struct { ID string `json:"id,omitempty" bundle:"readonly"` ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` + URL string `json:"url,omitempty" bundle:"internal"` } func (s *QualityMonitor) UnmarshalJSON(b []byte) error { @@ -44,3 +46,18 @@ func (s *QualityMonitor) Exists(ctx context.Context, w *databricks.WorkspaceClie func (s *QualityMonitor) TerraformResourceName() string { return "databricks_quality_monitor" } + +func (s *QualityMonitor) InitializeURL(urlPrefix string, urlSuffix string) { + if s.TableName == "" { + return + } + s.URL = urlPrefix + "explore/data/" + strings.ReplaceAll(s.TableName, ".", "/") + urlSuffix +} + +func (s *QualityMonitor) GetName() string { + return s.TableName +} + +func (s *QualityMonitor) GetURL() string { + return s.URL +} diff --git a/bundle/config/resources/registered_model.go b/bundle/config/resources/registered_model.go index 6033ffdf2b..9e4357bc2a 100644 --- a/bundle/config/resources/registered_model.go +++ b/bundle/config/resources/registered_model.go @@ -2,6 +2,7 @@ package resources import ( "context" + "strings" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go" @@ -24,6 +25,7 @@ type RegisteredModel struct { *catalog.CreateRegisteredModelRequest ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` + URL string `json:"url,omitempty" bundle:"internal"` } func (s *RegisteredModel) UnmarshalJSON(b []byte) error { @@ -48,3 +50,18 @@ func (s *RegisteredModel) Exists(ctx context.Context, w *databricks.WorkspaceCli func (s *RegisteredModel) TerraformResourceName() string { return "databricks_registered_model" } + +func (s *RegisteredModel) InitializeURL(urlPrefix string, urlSuffix string) { + if s.ID == "" { + return + } + s.URL = urlPrefix + "explore/data/models/" + strings.ReplaceAll(s.ID, ".", "/") + urlSuffix +} + +func (s *RegisteredModel) GetName() string { + return s.Name +} + +func (s *RegisteredModel) GetURL() string { + return s.URL +} diff --git a/bundle/config/resources/schema.go b/bundle/config/resources/schema.go index 7ab00495a8..3e8b9f4841 100644 --- a/bundle/config/resources/schema.go +++ b/bundle/config/resources/schema.go @@ -1,6 +1,11 @@ package resources import ( + "context" + "fmt" + "strings" + + "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/catalog" ) @@ -16,6 +21,30 @@ type Schema struct { *catalog.CreateSchema ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` + URL string `json:"url,omitempty" bundle:"internal"` +} + +func (s *Schema) Exists(ctx context.Context, w *databricks.WorkspaceClient, id string) (bool, error) { + return false, fmt.Errorf("schema.Exists() is not supported") +} + +func (s *Schema) TerraformResourceName() string { + return "databricks_schema" +} + +func (s *Schema) InitializeURL(urlPrefix string, urlSuffix string) { + if s.ID == "" { + return + } + s.URL = urlPrefix + "explore/data/" + strings.ReplaceAll(s.ID, ".", "/") + urlSuffix +} + +func (s *Schema) GetURL() string { + return s.URL +} + +func (s *Schema) GetName() string { + return s.Name } func (s *Schema) UnmarshalJSON(b []byte) error { diff --git a/bundle/config/resources_test.go b/bundle/config/resources_test.go index 6860d73daa..8a10fc5a04 100644 --- a/bundle/config/resources_test.go +++ b/bundle/config/resources_test.go @@ -3,6 +3,7 @@ package config import ( "encoding/json" "reflect" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -61,3 +62,22 @@ func TestCustomMarshallerIsImplemented(t *testing.T) { }, "Resource %s does not have a custom unmarshaller", field.Name) } } + +func TestResourcesAllResourcesCompleteness(t *testing.T) { + r := Resources{} + rt := reflect.TypeOf(r) + + result := r.AllResources() + + for i := 0; i < rt.NumField(); i++ { + field := rt.Field(i) + jsonTag := field.Tag.Get("json") + + if idx := strings.Index(jsonTag, ","); idx != -1 { + jsonTag = jsonTag[:idx] + } + + _, exists := result[jsonTag] + assert.True(t, exists, "Field %s is missing in AllResources map", field.Name) + } +} diff --git a/bundle/render/render_text_output.go b/bundle/render/render_text_output.go index ea0b9a944f..7cca86894d 100644 --- a/bundle/render/render_text_output.go +++ b/bundle/render/render_text_output.go @@ -1,9 +1,11 @@ package render import ( + "context" "fmt" "io" "path/filepath" + "sort" "strings" "text/template" @@ -56,7 +58,7 @@ const warningTemplate = `{{ "Warning" | yellow }}: {{ .Summary }} ` -const summaryTemplate = `{{- if .Name -}} +const summaryHeaderTemplate = `{{- if .Name -}} Name: {{ .Name | bold }} {{- if .Target }} Target: {{ .Target | bold }} @@ -73,12 +75,30 @@ Workspace: Path: {{ .Path | bold }} {{- end }} {{- end }} +{{ end -}}` -{{ end -}} - -{{ .Trailer }} +const resourcesTemplate = `Resources: +{{- range . }} + {{ .GroupName }}: + {{- range .Resources }} + {{ .Key | bold }}: + Name: {{ .Name }} + URL: {{ if .URL }}{{ .URL | cyan }}{{ else }}{{ "(not deployed)" | cyan }}{{ end }} + {{- end }} +{{- end }} ` +type ResourceGroup struct { + GroupName string + Resources []ResourceInfo +} + +type ResourceInfo struct { + Key string + Name string + URL string +} + func pluralize(n int, singular, plural string) string { if n == 1 { return fmt.Sprintf("%d %s", n, singular) @@ -95,15 +115,15 @@ func buildTrailer(diags diag.Diagnostics) string { parts = append(parts, color.YellowString(pluralize(warnings, "warning", "warnings"))) } if len(parts) > 0 { - return fmt.Sprintf("Found %s", strings.Join(parts, " and ")) + return fmt.Sprintf("Found %s\n", strings.Join(parts, " and ")) } else { - return color.GreenString("Validation OK!") + return color.GreenString("Validation OK!\n") } } -func renderSummaryTemplate(out io.Writer, b *bundle.Bundle, diags diag.Diagnostics) error { +func renderSummaryHeaderTemplate(out io.Writer, b *bundle.Bundle) error { if b == nil { - return renderSummaryTemplate(out, &bundle.Bundle{}, diags) + return renderSummaryHeaderTemplate(out, &bundle.Bundle{}) } var currentUser = &iam.User{} @@ -114,20 +134,19 @@ func renderSummaryTemplate(out io.Writer, b *bundle.Bundle, diags diag.Diagnosti } } - t := template.Must(template.New("summary").Funcs(renderFuncMap).Parse(summaryTemplate)) + t := template.Must(template.New("summary").Funcs(renderFuncMap).Parse(summaryHeaderTemplate)) err := t.Execute(out, map[string]any{ - "Name": b.Config.Bundle.Name, - "Target": b.Config.Bundle.Target, - "User": currentUser.UserName, - "Path": b.Config.Workspace.RootPath, - "Host": b.Config.Workspace.Host, - "Trailer": buildTrailer(diags), + "Name": b.Config.Bundle.Name, + "Target": b.Config.Bundle.Target, + "User": currentUser.UserName, + "Path": b.Config.Workspace.RootPath, + "Host": b.Config.Workspace.Host, }) return err } -func renderDiagnostics(out io.Writer, b *bundle.Bundle, diags diag.Diagnostics) error { +func renderDiagnosticsOnly(out io.Writer, b *bundle.Bundle, diags diag.Diagnostics) error { errorT := template.Must(template.New("error").Funcs(renderFuncMap).Parse(errorTemplate)) warningT := template.Must(template.New("warning").Funcs(renderFuncMap).Parse(warningTemplate)) @@ -173,19 +192,74 @@ type RenderOptions struct { RenderSummaryTable bool } -// RenderTextOutput renders the diagnostics in a human-readable format. -func RenderTextOutput(out io.Writer, b *bundle.Bundle, diags diag.Diagnostics, opts RenderOptions) error { - err := renderDiagnostics(out, b, diags) +// RenderDiagnostics renders the diagnostics in a human-readable format. +func RenderDiagnostics(out io.Writer, b *bundle.Bundle, diags diag.Diagnostics, opts RenderOptions) error { + err := renderDiagnosticsOnly(out, b, diags) if err != nil { return fmt.Errorf("failed to render diagnostics: %w", err) } if opts.RenderSummaryTable { - err = renderSummaryTemplate(out, b, diags) - if err != nil { - return fmt.Errorf("failed to render summary: %w", err) + if b != nil { + err = renderSummaryHeaderTemplate(out, b) + if err != nil { + return fmt.Errorf("failed to render summary: %w", err) + } + io.WriteString(out, "\n") } + trailer := buildTrailer(diags) + io.WriteString(out, trailer) } return nil } + +func RenderSummary(ctx context.Context, out io.Writer, b *bundle.Bundle) error { + if err := renderSummaryHeaderTemplate(out, b); err != nil { + return err + } + + var resourceGroups []ResourceGroup + + for group, r := range b.Config.Resources.AllResources() { + resources := make([]ResourceInfo, 0, len(r)) + for key, resource := range r { + resources = append(resources, ResourceInfo{ + Key: key, + Name: resource.GetName(), + URL: resource.GetURL(), + }) + } + + if len(resources) > 0 { + capitalizedGroup := strings.ToUpper(group[:1]) + group[1:] + resourceGroups = append(resourceGroups, ResourceGroup{ + GroupName: capitalizedGroup, + Resources: resources, + }) + } + } + + if err := renderResourcesTemplate(out, resourceGroups); err != nil { + return fmt.Errorf("failed to render resources template: %w", err) + } + + return nil +} + +// Helper function to sort and render resource groups using the template +func renderResourcesTemplate(out io.Writer, resourceGroups []ResourceGroup) error { + // Sort everything to ensure consistent output + sort.Slice(resourceGroups, func(i, j int) bool { + return resourceGroups[i].GroupName < resourceGroups[j].GroupName + }) + for _, group := range resourceGroups { + sort.Slice(group.Resources, func(i, j int) bool { + return group.Resources[i].Key < group.Resources[j].Key + }) + } + + t := template.Must(template.New("resources").Funcs(renderFuncMap).Parse(resourcesTemplate)) + + return t.Execute(out, resourceGroups) +} diff --git a/bundle/render/render_text_output_test.go b/bundle/render/render_text_output_test.go index 976f86e79c..91e13ffb29 100644 --- a/bundle/render/render_text_output_test.go +++ b/bundle/render/render_text_output_test.go @@ -2,14 +2,19 @@ package render import ( "bytes" + "context" + "io" "testing" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" - assert "github.com/databricks/cli/libs/dyn/dynassert" + "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/databricks-sdk-go/service/iam" + "github.com/databricks/databricks-sdk-go/service/jobs" + "github.com/databricks/databricks-sdk-go/service/pipelines" "github.com/stretchr/testify/require" ) @@ -192,10 +197,10 @@ func TestRenderTextOutput(t *testing.T) { t.Run(tc.name, func(t *testing.T) { writer := &bytes.Buffer{} - err := RenderTextOutput(writer, tc.bundle, tc.diags, tc.opts) + err := RenderDiagnostics(writer, tc.bundle, tc.diags, tc.opts) require.NoError(t, err) - assert.Equal(t, tc.expected, writer.String()) + require.Equal(t, tc.expected, writer.String()) }) } } @@ -310,10 +315,10 @@ func TestRenderDiagnostics(t *testing.T) { t.Run(tc.name, func(t *testing.T) { writer := &bytes.Buffer{} - err := renderDiagnostics(writer, bundle, tc.diags) + err := renderDiagnosticsOnly(writer, bundle, tc.diags) require.NoError(t, err) - assert.Equal(t, tc.expected, writer.String()) + require.Equal(t, tc.expected, writer.String()) }) } } @@ -321,8 +326,92 @@ func TestRenderDiagnostics(t *testing.T) { func TestRenderSummaryTemplate_nilBundle(t *testing.T) { writer := &bytes.Buffer{} - err := renderSummaryTemplate(writer, nil, nil) + err := renderSummaryHeaderTemplate(writer, nil) require.NoError(t, err) - assert.Equal(t, "Validation OK!\n", writer.String()) + io.WriteString(writer, buildTrailer(nil)) + + require.Equal(t, "Validation OK!\n", writer.String()) +} + +func TestRenderSummary(t *testing.T) { + ctx := context.Background() + + // Create a mock bundle with various resources + b := &bundle.Bundle{ + Config: config.Root{ + Bundle: config.Bundle{ + Name: "test-bundle", + Target: "test-target", + }, + Workspace: config.Workspace{ + Host: "https://mycompany.databricks.com/", + }, + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job1": { + ID: "1", + URL: "https://url1", + JobSettings: &jobs.JobSettings{Name: "job1-name"}, + }, + "job2": { + ID: "2", + URL: "https://url2", + JobSettings: &jobs.JobSettings{Name: "job2-name"}, + }, + }, + Pipelines: map[string]*resources.Pipeline{ + "pipeline2": { + ID: "4", + // no URL + PipelineSpec: &pipelines.PipelineSpec{Name: "pipeline2-name"}, + }, + "pipeline1": { + ID: "3", + URL: "https://url3", + PipelineSpec: &pipelines.PipelineSpec{Name: "pipeline1-name"}, + }, + }, + Schemas: map[string]*resources.Schema{ + "schema1": { + ID: "catalog.schema", + CreateSchema: &catalog.CreateSchema{ + Name: "schema", + }, + // no URL + }, + }, + }, + }, + } + + writer := &bytes.Buffer{} + err := RenderSummary(ctx, writer, b) + require.NoError(t, err) + + expectedSummary := `Name: test-bundle +Target: test-target +Workspace: + Host: https://mycompany.databricks.com/ +Resources: + Jobs: + job1: + Name: job1-name + URL: https://url1 + job2: + Name: job2-name + URL: https://url2 + Pipelines: + pipeline1: + Name: pipeline1-name + URL: https://url3 + pipeline2: + Name: pipeline2-name + URL: (not deployed) + Schemas: + schema1: + Name: schema + URL: (not deployed) +` + require.Equal(t, expectedSummary, writer.String()) } diff --git a/cmd/bundle/deploy.go b/cmd/bundle/deploy.go index 1166875ab3..a0ef99c5fc 100644 --- a/cmd/bundle/deploy.go +++ b/cmd/bundle/deploy.go @@ -61,7 +61,7 @@ func newDeployCommand() *cobra.Command { } renderOpts := render.RenderOptions{RenderSummaryTable: false} - err := render.RenderTextOutput(cmd.OutOrStdout(), b, diags, renderOpts) + err := render.RenderDiagnostics(cmd.OutOrStdout(), b, diags, renderOpts) if err != nil { return fmt.Errorf("failed to render output: %w", err) } diff --git a/cmd/bundle/summary.go b/cmd/bundle/summary.go index 5a64b46c0a..8c34dd6122 100644 --- a/cmd/bundle/summary.go +++ b/cmd/bundle/summary.go @@ -8,8 +8,10 @@ import ( "path/filepath" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/mutator" "github.com/databricks/cli/bundle/deploy/terraform" "github.com/databricks/cli/bundle/phases" + "github.com/databricks/cli/bundle/render" "github.com/databricks/cli/cmd/bundle/utils" "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/libs/flags" @@ -19,11 +21,8 @@ import ( func newSummaryCommand() *cobra.Command { cmd := &cobra.Command{ Use: "summary", - Short: "Describe the bundle resources and their deployment states", + Short: "Summarize resources deployed by this bundle", Args: root.NoArgs, - - // This command is currently intended for the Databricks VSCode extension only - Hidden: true, } var forcePull bool @@ -60,14 +59,15 @@ func newSummaryCommand() *cobra.Command { } } - diags = bundle.Apply(ctx, b, terraform.Load()) + diags = bundle.Apply(ctx, b, + bundle.Seq(terraform.Load(), mutator.InitializeURLs())) if err := diags.Error(); err != nil { return err } switch root.OutputType(cmd) { case flags.OutputText: - return fmt.Errorf("%w, only json output is supported", errors.ErrUnsupported) + return render.RenderSummary(ctx, cmd.OutOrStdout(), b) case flags.OutputJSON: buf, err := json.MarshalIndent(b.Config, "", " ") if err != nil { diff --git a/cmd/bundle/validate.go b/cmd/bundle/validate.go index 496d5d2b50..5331e7e7b1 100644 --- a/cmd/bundle/validate.go +++ b/cmd/bundle/validate.go @@ -54,7 +54,7 @@ func newValidateCommand() *cobra.Command { switch root.OutputType(cmd) { case flags.OutputText: renderOpts := render.RenderOptions{RenderSummaryTable: true} - err := render.RenderTextOutput(cmd.OutOrStdout(), b, diags, renderOpts) + err := render.RenderDiagnostics(cmd.OutOrStdout(), b, diags, renderOpts) if err != nil { return fmt.Errorf("failed to render output: %w", err) } From 1325fd867acd33f7dbc0d0dad4836064cf6c7117 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Thu, 29 Aug 2024 16:45:07 +0200 Subject: [PATCH 02/15] Fix typo --- bundle/config/mutator/initialize_urls_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bundle/config/mutator/initialize_urls_test.go b/bundle/config/mutator/initialize_urls_test.go index 5ef13119ca..84fb79a1a7 100644 --- a/bundle/config/mutator/initialize_urls_test.go +++ b/bundle/config/mutator/initialize_urls_test.go @@ -14,7 +14,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestIntitializeURLs(t *testing.T) { +func TestInitializeURLs(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ Workspace: config.Workspace{ @@ -100,7 +100,7 @@ func TestIntitializeURLs(t *testing.T) { } } -func TestIntitializeURLs_NoOrgId(t *testing.T) { +func URLs_NoOrgId(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ Workspace: config.Workspace{ From 189d40b1fa63fac7cb3ca2e21f437b228b8dd682 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Fri, 4 Oct 2024 20:43:00 -0700 Subject: [PATCH 03/15] Address reviewer comments --- bundle/config/mutator/initialize_urls.go | 22 ++++++++----------- bundle/config/mutator/initialize_urls_test.go | 8 ++----- 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/bundle/config/mutator/initialize_urls.go b/bundle/config/mutator/initialize_urls.go index e403d30d67..d4e09f46ed 100644 --- a/bundle/config/mutator/initialize_urls.go +++ b/bundle/config/mutator/initialize_urls.go @@ -10,7 +10,7 @@ import ( "github.com/databricks/cli/libs/diag" ) -type initializeUrls struct { +type initializeURLs struct { name string } @@ -19,30 +19,25 @@ type initializeUrls struct { // latency. As such, it should only be used when needed. // This URL field is used for the output of the 'bundle summary' CLI command. func InitializeURLs() bundle.Mutator { - return &initializeUrls{} + return &initializeURLs{} } -func (m *initializeUrls) Name() string { - return fmt.Sprintf("ConfigureURLs(%s)", m.name) +func (m *initializeURLs) Name() string { + return fmt.Sprintf("InitializeURLs(%s)", m.name) } -func (m *initializeUrls) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { +func (m *initializeURLs) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { workspaceId, err := b.WorkspaceClient().CurrentWorkspaceID(ctx) orgId := strconv.FormatInt(workspaceId, 10) if err != nil { return diag.FromErr(err) } - configureForOrgId(b, orgId) + urlPrefix := b.WorkspaceClient().Config.CanonicalHostName() + "/" + initializeForWorkspace(b, orgId, urlPrefix) return nil } -func configureForOrgId(b *bundle.Bundle, orgId string) { - urlPrefix := b.Config.Workspace.Host - if !strings.HasSuffix(urlPrefix, "/") { - urlPrefix += "/" - } - urlSuffix := "" - +func initializeForWorkspace(b *bundle.Bundle, orgId string, urlPrefix string) { // Add ?o= only if wasn't in the subdomain already. // The ?o= is needed when vanity URLs / legacy workspace URLs are used. // If it's not needed we prefer to leave it out since these URLs are rather @@ -50,6 +45,7 @@ func configureForOrgId(b *bundle.Bundle, orgId string) { // // See https://docs.databricks.com/en/workspace/workspace-details.html for // further reading about the '?o=' suffix. + urlSuffix := "" if !strings.Contains(urlPrefix, orgId) { urlSuffix = "?o=" + orgId } diff --git a/bundle/config/mutator/initialize_urls_test.go b/bundle/config/mutator/initialize_urls_test.go index 84fb79a1a7..35edc98c9d 100644 --- a/bundle/config/mutator/initialize_urls_test.go +++ b/bundle/config/mutator/initialize_urls_test.go @@ -91,7 +91,7 @@ func TestInitializeURLs(t *testing.T) { "schema1": "https://mycompany.databricks.com/explore/data/catalog/schema?o=123456", } - configureForOrgId(b, "123456") + initializeForWorkspace(b, "123456", "https://mycompany.databricks.com/") for _, rs := range b.Config.Resources.AllResources() { for key, r := range rs { @@ -103,10 +103,6 @@ func TestInitializeURLs(t *testing.T) { func URLs_NoOrgId(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ - Workspace: config.Workspace{ - // Hostname with org id in URL and no trailing / - Host: "https://adb-123456.azuredatabricks.net", - }, Resources: config.Resources{ Jobs: map[string]*resources.Job{ "job1": { @@ -118,7 +114,7 @@ func URLs_NoOrgId(t *testing.T) { }, } - configureForOrgId(b, "123456") + initializeForWorkspace(b, "123456", "https://adb-123456.azuredatabricks.net/") require.Equal(t, "https://adb-123456.azuredatabricks.net/jobs/1", b.Config.Resources.Jobs["job1"].URL) } From 65bad56e10e4db1b8747b7908b47418f8ef9684c Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Fri, 4 Oct 2024 21:03:05 -0700 Subject: [PATCH 04/15] Fix test failure based on code from main --- bundle/config/resources.go | 6 ++++++ bundle/config/resources/clusters.go | 16 ++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/bundle/config/resources.go b/bundle/config/resources.go index e932dab0e6..b76f080e2c 100644 --- a/bundle/config/resources.go +++ b/bundle/config/resources.go @@ -92,6 +92,12 @@ func (r *Resources) AllResources() map[string]map[string]ConfigResource { } result["schemas"] = schemaResources + clusterResources := make(map[string]ConfigResource) + for key, schema := range r.Clusters { + clusterResources[key] = schema + } + result["clusters"] = clusterResources + return result } diff --git a/bundle/config/resources/clusters.go b/bundle/config/resources/clusters.go index 6323456669..c5f61d0ec2 100644 --- a/bundle/config/resources/clusters.go +++ b/bundle/config/resources/clusters.go @@ -13,6 +13,7 @@ type Cluster 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"` *compute.ClusterSpec } @@ -37,3 +38,18 @@ func (s *Cluster) Exists(ctx context.Context, w *databricks.WorkspaceClient, id func (s *Cluster) TerraformResourceName() string { return "databricks_cluster" } + +func (s *Cluster) InitializeURL(urlPrefix string, urlSuffix string) { + if s.ID == "" { + return + } + s.URL = urlPrefix + "compute/clusters/" + s.ID + urlSuffix +} + +func (s *Cluster) GetName() string { + return s.ClusterName +} + +func (s *Cluster) GetURL() string { + return s.URL +} From d54f641640613518aa8bf5028313adf54db612b2 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Fri, 11 Oct 2024 14:43:36 +0200 Subject: [PATCH 05/15] Fix test name --- bundle/config/mutator/initialize_urls_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/config/mutator/initialize_urls_test.go b/bundle/config/mutator/initialize_urls_test.go index 35edc98c9d..b28795c775 100644 --- a/bundle/config/mutator/initialize_urls_test.go +++ b/bundle/config/mutator/initialize_urls_test.go @@ -100,7 +100,7 @@ func TestInitializeURLs(t *testing.T) { } } -func URLs_NoOrgId(t *testing.T) { +func TestInitializeURLsWithoutOrgId(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ Resources: config.Resources{ From aea4a6e63409deb5f10b9e792aba4a782ea0c585 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Thu, 17 Oct 2024 10:22:16 +0200 Subject: [PATCH 06/15] Styling fix --- bundle/config/mutator/initialize_urls.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/config/mutator/initialize_urls.go b/bundle/config/mutator/initialize_urls.go index d4e09f46ed..fcaa79e8f5 100644 --- a/bundle/config/mutator/initialize_urls.go +++ b/bundle/config/mutator/initialize_urls.go @@ -28,10 +28,10 @@ func (m *initializeURLs) Name() string { func (m *initializeURLs) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { workspaceId, err := b.WorkspaceClient().CurrentWorkspaceID(ctx) - orgId := strconv.FormatInt(workspaceId, 10) if err != nil { return diag.FromErr(err) } + orgId := strconv.FormatInt(workspaceId, 10) urlPrefix := b.WorkspaceClient().Config.CanonicalHostName() + "/" initializeForWorkspace(b, orgId, urlPrefix) return nil From 13049a555b80cb070519979d300c68dcf11285a9 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 17 Oct 2024 15:41:24 +0200 Subject: [PATCH 07/15] Remove unused name field --- bundle/config/mutator/initialize_urls.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bundle/config/mutator/initialize_urls.go b/bundle/config/mutator/initialize_urls.go index fcaa79e8f5..2d332f2294 100644 --- a/bundle/config/mutator/initialize_urls.go +++ b/bundle/config/mutator/initialize_urls.go @@ -2,7 +2,6 @@ package mutator import ( "context" - "fmt" "strconv" "strings" @@ -11,7 +10,6 @@ import ( ) type initializeURLs struct { - name string } // InitializeURLs makes sure the URL field of each resource is configured. @@ -23,7 +21,7 @@ func InitializeURLs() bundle.Mutator { } func (m *initializeURLs) Name() string { - return fmt.Sprintf("InitializeURLs(%s)", m.name) + return "InitializeURLs" } func (m *initializeURLs) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { From 2c8bb75bc469da006bf229f3be170733430b884a Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 17 Oct 2024 15:58:04 +0200 Subject: [PATCH 08/15] Add singular/plural titles to config.SupportedResources() --- bundle/config/resources.go | 69 ++++++++++++++++++++++++++++----- bundle/config/resources_test.go | 12 +++--- 2 files changed, 65 insertions(+), 16 deletions(-) diff --git a/bundle/config/resources.go b/bundle/config/resources.go index 0456291692..9f4d7ff855 100644 --- a/bundle/config/resources.go +++ b/bundle/config/resources.go @@ -130,20 +130,71 @@ func (r *Resources) FindResourceByConfigKey(key string) (ConfigResource, error) } type ResourceDescription struct { + // Singular and plural name when used to refer to the configuration. SingularName string + PluralName string + + // Singular and plural title when used in summaries / terminal UI. + SingularTitle string + PluralTitle string } // The keys of the map corresponds to the resource key in the bundle configuration. func SupportedResources() map[string]ResourceDescription { return map[string]ResourceDescription{ - "jobs": {SingularName: "job"}, - "pipelines": {SingularName: "pipeline"}, - "models": {SingularName: "model"}, - "experiments": {SingularName: "experiment"}, - "model_serving_endpoints": {SingularName: "model_serving_endpoint"}, - "registered_models": {SingularName: "registered_model"}, - "quality_monitors": {SingularName: "quality_monitor"}, - "schemas": {SingularName: "schema"}, - "clusters": {SingularName: "cluster"}, + "jobs": { + SingularName: "job", + PluralName: "jobs", + SingularTitle: "Job", + PluralTitle: "Jobs", + }, + "pipelines": { + SingularName: "pipeline", + PluralName: "pipelines", + SingularTitle: "Pipeline", + PluralTitle: "Pipelines", + }, + "models": { + SingularName: "model", + PluralName: "models", + SingularTitle: "Model", + PluralTitle: "Models", + }, + "experiments": { + SingularName: "experiment", + PluralName: "experiments", + SingularTitle: "Experiment", + PluralTitle: "Experiments", + }, + "model_serving_endpoints": { + SingularName: "model_serving_endpoint", + PluralName: "model_serving_endpoints", + SingularTitle: "Model Serving Endpoint", + PluralTitle: "Model Serving Endpoints", + }, + "registered_models": { + SingularName: "registered_model", + PluralName: "registered_models", + SingularTitle: "Registered Model", + PluralTitle: "Registered Models", + }, + "quality_monitors": { + SingularName: "quality_monitor", + PluralName: "quality_monitors", + SingularTitle: "Quality Monitor", + PluralTitle: "Quality Monitors", + }, + "schemas": { + SingularName: "schema", + PluralName: "schemas", + SingularTitle: "Schema", + PluralTitle: "Schemas", + }, + "clusters": { + SingularName: "cluster", + PluralName: "clusters", + SingularTitle: "Cluster", + PluralTitle: "Clusters", + }, } } diff --git a/bundle/config/resources_test.go b/bundle/config/resources_test.go index 36b54b32f1..668cfac38f 100644 --- a/bundle/config/resources_test.go +++ b/bundle/config/resources_test.go @@ -83,16 +83,14 @@ func TestResourcesAllResourcesCompleteness(t *testing.T) { } func TestSupportedResources(t *testing.T) { - expected := map[string]ResourceDescription{} + // Please add your resource to the SupportedResources() function in resources.go if you add a new resource. + actual := SupportedResources() + typ := reflect.TypeOf(Resources{}) for i := 0; i < typ.NumField(); i++ { field := typ.Field(i) jsonTags := strings.Split(field.Tag.Get("json"), ",") - singularName := strings.TrimSuffix(jsonTags[0], "s") - expected[jsonTags[0]] = ResourceDescription{SingularName: singularName} + pluralName := jsonTags[0] + assert.Equal(t, actual[pluralName].PluralName, pluralName) } - - // Please add your resource to the SupportedResources() function in resources.go - // if you are adding a new resource. - assert.Equal(t, expected, SupportedResources()) } From 85bc79f7a7fbfe2d5fa123802bd7c121de1eb495 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 17 Oct 2024 16:39:17 +0200 Subject: [PATCH 09/15] Use plural title resource header --- bundle/config/mutator/initialize_urls.go | 4 +- bundle/config/mutator/initialize_urls_test.go | 4 +- bundle/config/resources.go | 84 +++++++------------ bundle/config/resources_test.go | 9 +- bundle/render/render_text_output.go | 9 +- bundle/render/render_text_output_test.go | 14 ++++ 6 files changed, 59 insertions(+), 65 deletions(-) diff --git a/bundle/config/mutator/initialize_urls.go b/bundle/config/mutator/initialize_urls.go index 2d332f2294..cb85c5515e 100644 --- a/bundle/config/mutator/initialize_urls.go +++ b/bundle/config/mutator/initialize_urls.go @@ -48,8 +48,8 @@ func initializeForWorkspace(b *bundle.Bundle, orgId string, urlPrefix string) { urlSuffix = "?o=" + orgId } - for _, rs := range b.Config.Resources.AllResources() { - for _, r := range rs { + for _, group := range b.Config.Resources.AllResources() { + for _, r := range group.Resources { r.InitializeURL(urlPrefix, urlSuffix) } } diff --git a/bundle/config/mutator/initialize_urls_test.go b/bundle/config/mutator/initialize_urls_test.go index b28795c775..94778ec5a2 100644 --- a/bundle/config/mutator/initialize_urls_test.go +++ b/bundle/config/mutator/initialize_urls_test.go @@ -93,8 +93,8 @@ func TestInitializeURLs(t *testing.T) { initializeForWorkspace(b, "123456", "https://mycompany.databricks.com/") - for _, rs := range b.Config.Resources.AllResources() { - for key, r := range rs { + for _, group := range b.Config.Resources.AllResources() { + for key, r := range group.Resources { require.Equal(t, expectedURLs[key], r.GetURL(), "Unexpected URL for "+key) } } diff --git a/bundle/config/resources.go b/bundle/config/resources.go index 9f4d7ff855..63c16d0a60 100644 --- a/bundle/config/resources.go +++ b/bundle/config/resources.go @@ -41,64 +41,42 @@ type ConfigResource interface { InitializeURL(urlPrefix string, urlSuffix string) } -func (r *Resources) AllResources() map[string]map[string]ConfigResource { - result := make(map[string]map[string]ConfigResource) - - jobResources := make(map[string]ConfigResource) - for key, job := range r.Jobs { - jobResources[key] = job - } - result["jobs"] = jobResources - - pipelineResources := make(map[string]ConfigResource) - for key, pipeline := range r.Pipelines { - pipelineResources[key] = pipeline - } - result["pipelines"] = pipelineResources - - modelResources := make(map[string]ConfigResource) - for key, model := range r.Models { - modelResources[key] = model - } - result["models"] = modelResources - - experimentResources := make(map[string]ConfigResource) - for key, experiment := range r.Experiments { - experimentResources[key] = experiment - } - result["experiments"] = experimentResources - - modelServingEndpointResources := make(map[string]ConfigResource) - for key, endpoint := range r.ModelServingEndpoints { - modelServingEndpointResources[key] = endpoint - } - result["model_serving_endpoints"] = modelServingEndpointResources - - registeredModelResources := make(map[string]ConfigResource) - for key, registeredModel := range r.RegisteredModels { - registeredModelResources[key] = registeredModel - } - result["registered_models"] = registeredModelResources +// ResourceGroup represents a group of resources of the same type. +// It includes a description of the resource type and a map of resources. +type ResourceGroup struct { + Description ResourceDescription + Resources map[string]ConfigResource +} - qualityMonitorResources := make(map[string]ConfigResource) - for key, qualityMonitor := range r.QualityMonitors { - qualityMonitorResources[key] = qualityMonitor +// collectResourceMap collects resources of a specific type into a ResourceGroup. +func collectResourceMap[T ConfigResource]( + description ResourceDescription, + input map[string]T, +) ResourceGroup { + resources := make(map[string]ConfigResource) + for key, resource := range input { + resources[key] = resource } - result["quality_monitors"] = qualityMonitorResources - - schemaResources := make(map[string]ConfigResource) - for key, schema := range r.Schemas { - schemaResources[key] = schema + return ResourceGroup{ + Description: description, + Resources: resources, } - result["schemas"] = schemaResources +} - clusterResources := make(map[string]ConfigResource) - for key, schema := range r.Clusters { - clusterResources[key] = schema +// AllResources returns all resources in the bundle grouped by their resource type. +func (r *Resources) AllResources() []ResourceGroup { + descriptions := SupportedResources() + return []ResourceGroup{ + collectResourceMap(descriptions["jobs"], r.Jobs), + collectResourceMap(descriptions["pipelines"], r.Pipelines), + collectResourceMap(descriptions["models"], r.Models), + collectResourceMap(descriptions["experiments"], r.Experiments), + collectResourceMap(descriptions["model_serving_endpoints"], r.ModelServingEndpoints), + collectResourceMap(descriptions["registered_models"], r.RegisteredModels), + collectResourceMap(descriptions["quality_monitors"], r.QualityMonitors), + collectResourceMap(descriptions["schemas"], r.Schemas), + collectResourceMap(descriptions["clusters"], r.Clusters), } - result["clusters"] = clusterResources - - return result } func (r *Resources) FindResourceByConfigKey(key string) (ConfigResource, error) { diff --git a/bundle/config/resources_test.go b/bundle/config/resources_test.go index 668cfac38f..9ae73b22a0 100644 --- a/bundle/config/resources_test.go +++ b/bundle/config/resources_test.go @@ -67,7 +67,11 @@ func TestResourcesAllResourcesCompleteness(t *testing.T) { r := Resources{} rt := reflect.TypeOf(r) - result := r.AllResources() + // Collect set of includes resource types + var types []string + for _, group := range r.AllResources() { + types = append(types, group.Description.PluralName) + } for i := 0; i < rt.NumField(); i++ { field := rt.Field(i) @@ -77,8 +81,7 @@ func TestResourcesAllResourcesCompleteness(t *testing.T) { jsonTag = jsonTag[:idx] } - _, exists := result[jsonTag] - assert.True(t, exists, "Field %s is missing in AllResources map", field.Name) + assert.Contains(t, types, jsonTag, "Field %s is missing in AllResources", field.Name) } } diff --git a/bundle/render/render_text_output.go b/bundle/render/render_text_output.go index 229c484655..2f7affbf3b 100644 --- a/bundle/render/render_text_output.go +++ b/bundle/render/render_text_output.go @@ -187,9 +187,9 @@ func RenderSummary(ctx context.Context, out io.Writer, b *bundle.Bundle) error { var resourceGroups []ResourceGroup - for group, r := range b.Config.Resources.AllResources() { - resources := make([]ResourceInfo, 0, len(r)) - for key, resource := range r { + for _, group := range b.Config.Resources.AllResources() { + resources := make([]ResourceInfo, 0, len(group.Resources)) + for key, resource := range group.Resources { resources = append(resources, ResourceInfo{ Key: key, Name: resource.GetName(), @@ -198,9 +198,8 @@ func RenderSummary(ctx context.Context, out io.Writer, b *bundle.Bundle) error { } if len(resources) > 0 { - capitalizedGroup := strings.ToUpper(group[:1]) + group[1:] resourceGroups = append(resourceGroups, ResourceGroup{ - GroupName: capitalizedGroup, + GroupName: group.Description.PluralTitle, Resources: resources, }) } diff --git a/bundle/render/render_text_output_test.go b/bundle/render/render_text_output_test.go index 48c174937d..ef80d87f5d 100644 --- a/bundle/render/render_text_output_test.go +++ b/bundle/render/render_text_output_test.go @@ -15,6 +15,7 @@ import ( "github.com/databricks/databricks-sdk-go/service/iam" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/databricks/databricks-sdk-go/service/pipelines" + "github.com/databricks/databricks-sdk-go/service/serving" "github.com/stretchr/testify/require" ) @@ -539,6 +540,15 @@ func TestRenderSummary(t *testing.T) { // no URL }, }, + ModelServingEndpoints: map[string]*resources.ModelServingEndpoint{ + "endpoint1": { + ID: "7", + CreateServingEndpoint: &serving.CreateServingEndpoint{ + Name: "my_serving_endpoint", + }, + URL: "https://url4", + }, + }, }, }, } @@ -559,6 +569,10 @@ Resources: job2: Name: job2-name URL: https://url2 + Model Serving Endpoints: + endpoint1: + Name: my_serving_endpoint + URL: https://url4 Pipelines: pipeline1: Name: pipeline1-name From dccd70bdb7a6efe6f9fb2b1cd1bfd4348147ad43 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 17 Oct 2024 16:40:46 +0200 Subject: [PATCH 10/15] require -> assert --- bundle/render/render_text_output_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/bundle/render/render_text_output_test.go b/bundle/render/render_text_output_test.go index ef80d87f5d..cd9e7723be 100644 --- a/bundle/render/render_text_output_test.go +++ b/bundle/render/render_text_output_test.go @@ -16,6 +16,7 @@ import ( "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/databricks/databricks-sdk-go/service/pipelines" "github.com/databricks/databricks-sdk-go/service/serving" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -335,7 +336,7 @@ func TestRenderTextOutput(t *testing.T) { err := RenderDiagnostics(writer, tc.bundle, tc.diags, tc.opts) require.NoError(t, err) - require.Equal(t, tc.expected, writer.String()) + assert.Equal(t, tc.expected, writer.String()) }) } } @@ -477,7 +478,7 @@ func TestRenderDiagnostics(t *testing.T) { err := renderDiagnosticsOnly(writer, bundle, tc.diags) require.NoError(t, err) - require.Equal(t, tc.expected, writer.String()) + assert.Equal(t, tc.expected, writer.String()) }) } } @@ -490,7 +491,7 @@ func TestRenderSummaryTemplate_nilBundle(t *testing.T) { io.WriteString(writer, buildTrailer(nil)) - require.Equal(t, "Validation OK!\n", writer.String()) + assert.Equal(t, "Validation OK!\n", writer.String()) } func TestRenderSummary(t *testing.T) { @@ -585,5 +586,5 @@ Resources: Name: schema URL: (not deployed) ` - require.Equal(t, expectedSummary, writer.String()) + assert.Equal(t, expectedSummary, writer.String()) } From b1dd60c410f80f2638de4e4a7aa25feb84f03081 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 17 Oct 2024 16:45:18 +0200 Subject: [PATCH 11/15] Include test case for clusters --- bundle/config/mutator/initialize_urls_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/bundle/config/mutator/initialize_urls_test.go b/bundle/config/mutator/initialize_urls_test.go index 94778ec5a2..4af7f4a8aa 100644 --- a/bundle/config/mutator/initialize_urls_test.go +++ b/bundle/config/mutator/initialize_urls_test.go @@ -7,6 +7,7 @@ import ( "github.com/databricks/cli/bundle/config" "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/jobs" "github.com/databricks/databricks-sdk-go/service/ml" "github.com/databricks/databricks-sdk-go/service/pipelines" @@ -76,6 +77,14 @@ func TestInitializeURLs(t *testing.T) { }, }, }, + Clusters: map[string]*resources.Cluster{ + "cluster1": { + ID: "1017-103929-vlr7jzcf", + ClusterSpec: &compute.ClusterSpec{ + ClusterName: "cluster1", + }, + }, + }, }, }, } @@ -89,6 +98,7 @@ func TestInitializeURLs(t *testing.T) { "registeredmodel1": "https://mycompany.databricks.com/explore/data/models/8?o=123456", "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", } initializeForWorkspace(b, "123456", "https://mycompany.databricks.com/") From 7bf2ec378cc50d34bc3219997378bea838b2ceb6 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 17 Oct 2024 17:17:17 +0200 Subject: [PATCH 12/15] Always use ID to synthesize URLs --- bundle/config/mutator/initialize_urls_test.go | 6 +++--- bundle/config/resources/mlflow_model.go | 2 +- bundle/config/resources/model_serving_endpoint.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bundle/config/mutator/initialize_urls_test.go b/bundle/config/mutator/initialize_urls_test.go index 4af7f4a8aa..5847dcc2a3 100644 --- a/bundle/config/mutator/initialize_urls_test.go +++ b/bundle/config/mutator/initialize_urls_test.go @@ -42,13 +42,13 @@ func TestInitializeURLs(t *testing.T) { }, Models: map[string]*resources.MlflowModel{ "model1": { - ID: "6", + ID: "a model uses its name for identifier", Model: &ml.Model{Name: "model1"}, }, }, ModelServingEndpoints: map[string]*resources.ModelServingEndpoint{ "servingendpoint1": { - ID: "7", + ID: "my_serving_endpoint", CreateServingEndpoint: &serving.CreateServingEndpoint{ Name: "my_serving_endpoint", }, @@ -93,7 +93,7 @@ func TestInitializeURLs(t *testing.T) { "job1": "https://mycompany.databricks.com/jobs/1?o=123456", "pipeline1": "https://mycompany.databricks.com/pipelines/3?o=123456", "experiment1": "https://mycompany.databricks.com/ml/experiments/4?o=123456", - "model1": "https://mycompany.databricks.com/ml/models/model1?o=123456", + "model1": "https://mycompany.databricks.com/ml/models/a model uses its name for identifier?o=123456", "servingendpoint1": "https://mycompany.databricks.com/ml/endpoints/my_serving_endpoint?o=123456", "registeredmodel1": "https://mycompany.databricks.com/explore/data/models/8?o=123456", "qualityMonitor1": "https://mycompany.databricks.com/explore/data/catalog/schema/qualityMonitor1?o=123456", diff --git a/bundle/config/resources/mlflow_model.go b/bundle/config/resources/mlflow_model.go index 1fd681a1b9..201ba49f16 100644 --- a/bundle/config/resources/mlflow_model.go +++ b/bundle/config/resources/mlflow_model.go @@ -45,7 +45,7 @@ func (s *MlflowModel) InitializeURL(urlPrefix string, urlSuffix string) { if s.ID == "" { return } - s.URL = urlPrefix + "ml/models/" + s.Name + urlSuffix + s.URL = urlPrefix + "ml/models/" + s.ID + urlSuffix } func (s *MlflowModel) GetName() string { diff --git a/bundle/config/resources/model_serving_endpoint.go b/bundle/config/resources/model_serving_endpoint.go index f8981707be..4ec6fb3cec 100644 --- a/bundle/config/resources/model_serving_endpoint.go +++ b/bundle/config/resources/model_serving_endpoint.go @@ -53,7 +53,7 @@ func (s *ModelServingEndpoint) InitializeURL(urlPrefix string, urlSuffix string) if s.ID == "" { return } - s.URL = urlPrefix + "ml/endpoints/" + s.Name + urlSuffix + s.URL = urlPrefix + "ml/endpoints/" + s.ID + urlSuffix } func (s *ModelServingEndpoint) GetName() string { From 7658dbdff2872e8b7b503e407ca5b127873e60c8 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 17 Oct 2024 17:30:48 +0200 Subject: [PATCH 13/15] Use net/url to build resource URLs such that proper escaping is done --- bundle/config/mutator/initialize_urls.go | 23 +++++++++++++------ bundle/config/mutator/initialize_urls_test.go | 2 +- bundle/config/resources.go | 3 ++- bundle/config/resources/clusters.go | 7 ++++-- bundle/config/resources/job.go | 7 ++++-- bundle/config/resources/mlflow_experiment.go | 7 ++++-- bundle/config/resources/mlflow_model.go | 7 ++++-- .../resources/model_serving_endpoint.go | 7 ++++-- bundle/config/resources/pipeline.go | 7 ++++-- bundle/config/resources/quality_monitor.go | 7 ++++-- bundle/config/resources/registered_model.go | 7 ++++-- bundle/config/resources/schema.go | 6 +++-- 12 files changed, 63 insertions(+), 27 deletions(-) diff --git a/bundle/config/mutator/initialize_urls.go b/bundle/config/mutator/initialize_urls.go index cb85c5515e..3193059121 100644 --- a/bundle/config/mutator/initialize_urls.go +++ b/bundle/config/mutator/initialize_urls.go @@ -2,6 +2,7 @@ package mutator import ( "context" + "net/url" "strconv" "strings" @@ -30,12 +31,17 @@ func (m *initializeURLs) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagn return diag.FromErr(err) } orgId := strconv.FormatInt(workspaceId, 10) - urlPrefix := b.WorkspaceClient().Config.CanonicalHostName() + "/" - initializeForWorkspace(b, orgId, urlPrefix) + host := b.WorkspaceClient().Config.CanonicalHostName() + initializeForWorkspace(b, orgId, host) return nil } -func initializeForWorkspace(b *bundle.Bundle, orgId string, urlPrefix string) { +func initializeForWorkspace(b *bundle.Bundle, orgId string, host string) error { + baseURL, err := url.Parse(host) + if err != nil { + return err + } + // Add ?o= only if wasn't in the subdomain already. // The ?o= is needed when vanity URLs / legacy workspace URLs are used. // If it's not needed we prefer to leave it out since these URLs are rather @@ -43,14 +49,17 @@ func initializeForWorkspace(b *bundle.Bundle, orgId string, urlPrefix string) { // // See https://docs.databricks.com/en/workspace/workspace-details.html for // further reading about the '?o=' suffix. - urlSuffix := "" - if !strings.Contains(urlPrefix, orgId) { - urlSuffix = "?o=" + orgId + if !strings.Contains(baseURL.Hostname(), orgId) { + values := baseURL.Query() + values.Add("o", orgId) + baseURL.RawQuery = values.Encode() } for _, group := range b.Config.Resources.AllResources() { for _, r := range group.Resources { - r.InitializeURL(urlPrefix, urlSuffix) + r.InitializeURL(*baseURL) } } + + return nil } diff --git a/bundle/config/mutator/initialize_urls_test.go b/bundle/config/mutator/initialize_urls_test.go index 5847dcc2a3..8927b43134 100644 --- a/bundle/config/mutator/initialize_urls_test.go +++ b/bundle/config/mutator/initialize_urls_test.go @@ -93,7 +93,7 @@ func TestInitializeURLs(t *testing.T) { "job1": "https://mycompany.databricks.com/jobs/1?o=123456", "pipeline1": "https://mycompany.databricks.com/pipelines/3?o=123456", "experiment1": "https://mycompany.databricks.com/ml/experiments/4?o=123456", - "model1": "https://mycompany.databricks.com/ml/models/a model uses its name for identifier?o=123456", + "model1": "https://mycompany.databricks.com/ml/models/a%20model%20uses%20its%20name%20for%20identifier?o=123456", "servingendpoint1": "https://mycompany.databricks.com/ml/endpoints/my_serving_endpoint?o=123456", "registeredmodel1": "https://mycompany.databricks.com/explore/data/models/8?o=123456", "qualityMonitor1": "https://mycompany.databricks.com/explore/data/catalog/schema/qualityMonitor1?o=123456", diff --git a/bundle/config/resources.go b/bundle/config/resources.go index 63c16d0a60..ee72d9fbda 100644 --- a/bundle/config/resources.go +++ b/bundle/config/resources.go @@ -3,6 +3,7 @@ package config import ( "context" "fmt" + "net/url" "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/databricks-sdk-go" @@ -38,7 +39,7 @@ type ConfigResource interface { GetURL() string // InitializeURL initializes the URL field of the resource. - InitializeURL(urlPrefix string, urlSuffix string) + InitializeURL(url.URL) } // ResourceGroup represents a group of resources of the same type. diff --git a/bundle/config/resources/clusters.go b/bundle/config/resources/clusters.go index c5f61d0ec2..eb0247c6e9 100644 --- a/bundle/config/resources/clusters.go +++ b/bundle/config/resources/clusters.go @@ -2,6 +2,8 @@ package resources import ( "context" + "fmt" + "net/url" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go" @@ -39,11 +41,12 @@ func (s *Cluster) TerraformResourceName() string { return "databricks_cluster" } -func (s *Cluster) InitializeURL(urlPrefix string, urlSuffix string) { +func (s *Cluster) InitializeURL(baseURL url.URL) { if s.ID == "" { return } - s.URL = urlPrefix + "compute/clusters/" + s.ID + urlSuffix + baseURL.Path = fmt.Sprintf("compute/clusters/%s", s.ID) + s.URL = baseURL.String() } func (s *Cluster) GetName() string { diff --git a/bundle/config/resources/job.go b/bundle/config/resources/job.go index 6c119c3eb9..98db1ec5d9 100644 --- a/bundle/config/resources/job.go +++ b/bundle/config/resources/job.go @@ -2,6 +2,8 @@ package resources import ( "context" + "fmt" + "net/url" "strconv" "github.com/databricks/cli/libs/log" @@ -46,11 +48,12 @@ func (j *Job) TerraformResourceName() string { return "databricks_job" } -func (j *Job) InitializeURL(urlPrefix string, urlSuffix string) { +func (j *Job) InitializeURL(baseURL url.URL) { if j.ID == "" { return } - j.URL = urlPrefix + "jobs/" + j.ID + urlSuffix + baseURL.Path = fmt.Sprintf("jobs/%s", j.ID) + j.URL = baseURL.String() } func (j *Job) GetName() string { diff --git a/bundle/config/resources/mlflow_experiment.go b/bundle/config/resources/mlflow_experiment.go index e9cb2c0bb1..a5871468fd 100644 --- a/bundle/config/resources/mlflow_experiment.go +++ b/bundle/config/resources/mlflow_experiment.go @@ -2,6 +2,8 @@ package resources import ( "context" + "fmt" + "net/url" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go" @@ -41,11 +43,12 @@ func (s *MlflowExperiment) TerraformResourceName() string { return "databricks_mlflow_experiment" } -func (s *MlflowExperiment) InitializeURL(urlPrefix string, urlSuffix string) { +func (s *MlflowExperiment) InitializeURL(baseURL url.URL) { if s.ID == "" { return } - s.URL = urlPrefix + "ml/experiments/" + s.ID + urlSuffix + baseURL.Path = fmt.Sprintf("ml/experiments/%s", s.ID) + s.URL = baseURL.String() } func (s *MlflowExperiment) GetName() string { diff --git a/bundle/config/resources/mlflow_model.go b/bundle/config/resources/mlflow_model.go index 201ba49f16..9ead254d88 100644 --- a/bundle/config/resources/mlflow_model.go +++ b/bundle/config/resources/mlflow_model.go @@ -2,6 +2,8 @@ package resources import ( "context" + "fmt" + "net/url" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go" @@ -41,11 +43,12 @@ func (s *MlflowModel) TerraformResourceName() string { return "databricks_mlflow_model" } -func (s *MlflowModel) InitializeURL(urlPrefix string, urlSuffix string) { +func (s *MlflowModel) InitializeURL(baseURL url.URL) { if s.ID == "" { return } - s.URL = urlPrefix + "ml/models/" + s.ID + urlSuffix + baseURL.Path = fmt.Sprintf("ml/models/%s", s.ID) + s.URL = baseURL.String() } func (s *MlflowModel) GetName() string { diff --git a/bundle/config/resources/model_serving_endpoint.go b/bundle/config/resources/model_serving_endpoint.go index 4ec6fb3cec..7f3ae00c86 100644 --- a/bundle/config/resources/model_serving_endpoint.go +++ b/bundle/config/resources/model_serving_endpoint.go @@ -2,6 +2,8 @@ package resources import ( "context" + "fmt" + "net/url" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go" @@ -49,11 +51,12 @@ func (s *ModelServingEndpoint) TerraformResourceName() string { return "databricks_model_serving" } -func (s *ModelServingEndpoint) InitializeURL(urlPrefix string, urlSuffix string) { +func (s *ModelServingEndpoint) InitializeURL(baseURL url.URL) { if s.ID == "" { return } - s.URL = urlPrefix + "ml/endpoints/" + s.ID + urlSuffix + baseURL.Path = fmt.Sprintf("ml/endpoints/%s", s.ID) + s.URL = baseURL.String() } func (s *ModelServingEndpoint) GetName() string { diff --git a/bundle/config/resources/pipeline.go b/bundle/config/resources/pipeline.go index 63518001f7..b3311d8e21 100644 --- a/bundle/config/resources/pipeline.go +++ b/bundle/config/resources/pipeline.go @@ -2,6 +2,8 @@ package resources import ( "context" + "fmt" + "net/url" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go" @@ -41,11 +43,12 @@ func (p *Pipeline) TerraformResourceName() string { return "databricks_pipeline" } -func (p *Pipeline) InitializeURL(urlPrefix string, urlSuffix string) { +func (p *Pipeline) InitializeURL(baseURL url.URL) { if p.ID == "" { return } - p.URL = urlPrefix + "pipelines/" + p.ID + urlSuffix + baseURL.Path = fmt.Sprintf("pipelines/%s", p.ID) + p.URL = baseURL.String() } func (p *Pipeline) GetName() string { diff --git a/bundle/config/resources/quality_monitor.go b/bundle/config/resources/quality_monitor.go index 6cf234a947..3c823e625b 100644 --- a/bundle/config/resources/quality_monitor.go +++ b/bundle/config/resources/quality_monitor.go @@ -2,6 +2,8 @@ package resources import ( "context" + "fmt" + "net/url" "strings" "github.com/databricks/cli/libs/log" @@ -47,11 +49,12 @@ func (s *QualityMonitor) TerraformResourceName() string { return "databricks_quality_monitor" } -func (s *QualityMonitor) InitializeURL(urlPrefix string, urlSuffix string) { +func (s *QualityMonitor) InitializeURL(baseURL url.URL) { if s.TableName == "" { return } - s.URL = urlPrefix + "explore/data/" + strings.ReplaceAll(s.TableName, ".", "/") + urlSuffix + baseURL.Path = fmt.Sprintf("explore/data/%s", strings.ReplaceAll(s.TableName, ".", "/")) + s.URL = baseURL.String() } func (s *QualityMonitor) GetName() string { diff --git a/bundle/config/resources/registered_model.go b/bundle/config/resources/registered_model.go index 9e4357bc2a..c44526d091 100644 --- a/bundle/config/resources/registered_model.go +++ b/bundle/config/resources/registered_model.go @@ -2,6 +2,8 @@ package resources import ( "context" + "fmt" + "net/url" "strings" "github.com/databricks/cli/libs/log" @@ -51,11 +53,12 @@ func (s *RegisteredModel) TerraformResourceName() string { return "databricks_registered_model" } -func (s *RegisteredModel) InitializeURL(urlPrefix string, urlSuffix string) { +func (s *RegisteredModel) InitializeURL(baseURL url.URL) { if s.ID == "" { return } - s.URL = urlPrefix + "explore/data/models/" + strings.ReplaceAll(s.ID, ".", "/") + urlSuffix + baseURL.Path = fmt.Sprintf("explore/data/models/%s", strings.ReplaceAll(s.ID, ".", "/")) + s.URL = baseURL.String() } func (s *RegisteredModel) GetName() string { diff --git a/bundle/config/resources/schema.go b/bundle/config/resources/schema.go index 3e8b9f4841..a9f905cf18 100644 --- a/bundle/config/resources/schema.go +++ b/bundle/config/resources/schema.go @@ -3,6 +3,7 @@ package resources import ( "context" "fmt" + "net/url" "strings" "github.com/databricks/databricks-sdk-go" @@ -32,11 +33,12 @@ func (s *Schema) TerraformResourceName() string { return "databricks_schema" } -func (s *Schema) InitializeURL(urlPrefix string, urlSuffix string) { +func (s *Schema) InitializeURL(baseURL url.URL) { if s.ID == "" { return } - s.URL = urlPrefix + "explore/data/" + strings.ReplaceAll(s.ID, ".", "/") + urlSuffix + baseURL.Path = fmt.Sprintf("explore/data/%s", strings.ReplaceAll(s.ID, ".", "/")) + s.URL = baseURL.String() } func (s *Schema) GetURL() string { From 0cd3fcdda13b8b00ec73160e614cbc37d209d07d Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 18 Oct 2024 08:36:34 +0200 Subject: [PATCH 14/15] Update bundle/config/resources.go Co-authored-by: Lennart Kats (databricks) --- bundle/config/resources.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/config/resources.go b/bundle/config/resources.go index ee72d9fbda..9513369e4d 100644 --- a/bundle/config/resources.go +++ b/bundle/config/resources.go @@ -39,7 +39,7 @@ type ConfigResource interface { GetURL() string // InitializeURL initializes the URL field of the resource. - InitializeURL(url.URL) + InitializeURL(baseURL url.URL) } // ResourceGroup represents a group of resources of the same type. From c45b058f7cc9946fe6f12bbaaa28cc6beb44ed24 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 18 Oct 2024 08:36:55 +0200 Subject: [PATCH 15/15] Update bundle/config/mutator/initialize_urls_test.go Co-authored-by: Lennart Kats (databricks) --- bundle/config/mutator/initialize_urls_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/config/mutator/initialize_urls_test.go b/bundle/config/mutator/initialize_urls_test.go index 8927b43134..71cc153ab7 100644 --- a/bundle/config/mutator/initialize_urls_test.go +++ b/bundle/config/mutator/initialize_urls_test.go @@ -43,7 +43,7 @@ func TestInitializeURLs(t *testing.T) { Models: map[string]*resources.MlflowModel{ "model1": { ID: "a model uses its name for identifier", - Model: &ml.Model{Name: "model1"}, + Model: &ml.Model{Name: "a model uses its name for identifier"}, }, }, ModelServingEndpoints: map[string]*resources.ModelServingEndpoint{