From b99570818d03997cba3bd2604c685de1545ce30c Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 25 Apr 2025 12:39:39 +0200 Subject: [PATCH] Make apply_presets.go more resilient and consistent --- .../empty_resources/empty_dict/output.txt | 44 +++++----- .../empty_resources/with_grants/output.txt | 44 +++++----- .../with_permissions/output.txt | 27 +++---- .../mutator/resourcemutator/apply_presets.go | 80 ++++++++++++------- 4 files changed, 115 insertions(+), 80 deletions(-) diff --git a/acceptance/bundle/validate/empty_resources/empty_dict/output.txt b/acceptance/bundle/validate/empty_resources/empty_dict/output.txt index b650b9502f..8bdfb1cb9e 100644 --- a/acceptance/bundle/validate/empty_resources/empty_dict/output.txt +++ b/acceptance/bundle/validate/empty_resources/empty_dict/output.txt @@ -1,43 +1,51 @@ === resources.jobs.rname === -Error: job rname is not defined - { "jobs": { - "rname": {} + "rname": { + "deployment": { + "kind": "BUNDLE", + "metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/BUNDLE/default/state/metadata.json" + }, + "edit_mode": "UI_LOCKED", + "format": "MULTI_TASK", + "permissions": [], + "queue": { + "enabled": true + } + } } } === resources.pipelines.rname === -Error: pipeline rname is not defined - { "pipelines": { - "rname": {} + "rname": { + "permissions": [] + } } } === resources.models.rname === -Error: model rname is not defined - { "models": { - "rname": {} + "rname": { + "permissions": [] + } } } === resources.experiments.rname === -Error: experiment rname is not defined - { "experiments": { - "rname": {} + "rname": { + "name": ".", + "permissions": [] + } } } === resources.registered_models.rname === -Error: registered model rname is not defined - { "registered_models": { "rname": {} @@ -52,8 +60,6 @@ Error: registered model rname is not defined } === resources.schemas.rname === -Error: schema rname is not defined - { "schemas": { "rname": {} @@ -70,11 +76,11 @@ Error: schema rname is not defined } === resources.clusters.rname === -Error: cluster rname is not defined - { "clusters": { - "rname": {} + "rname": { + "custom_tags": {} + } } } diff --git a/acceptance/bundle/validate/empty_resources/with_grants/output.txt b/acceptance/bundle/validate/empty_resources/with_grants/output.txt index e91661e29d..6e7ac17c2b 100644 --- a/acceptance/bundle/validate/empty_resources/with_grants/output.txt +++ b/acceptance/bundle/validate/empty_resources/with_grants/output.txt @@ -4,11 +4,20 @@ Warning: unknown field: grants at resources.jobs.rname in databricks.yml:7:7 -Error: job rname is not defined - { "jobs": { - "rname": {} + "rname": { + "deployment": { + "kind": "BUNDLE", + "metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/BUNDLE/default/state/metadata.json" + }, + "edit_mode": "UI_LOCKED", + "format": "MULTI_TASK", + "permissions": [], + "queue": { + "enabled": true + } + } } } @@ -17,11 +26,11 @@ Warning: unknown field: grants at resources.pipelines.rname in databricks.yml:7:7 -Error: pipeline rname is not defined - { "pipelines": { - "rname": {} + "rname": { + "permissions": [] + } } } @@ -30,11 +39,11 @@ Warning: unknown field: grants at resources.models.rname in databricks.yml:7:7 -Error: model rname is not defined - { "models": { - "rname": {} + "rname": { + "permissions": [] + } } } @@ -43,17 +52,16 @@ Warning: unknown field: grants at resources.experiments.rname in databricks.yml:7:7 -Error: experiment rname is not defined - { "experiments": { - "rname": {} + "rname": { + "name": ".", + "permissions": [] + } } } === resources.registered_models.rname === -Error: registered model rname is not defined - { "registered_models": { "rname": { @@ -74,8 +82,6 @@ Warning: unknown field: grants } === resources.schemas.rname === -Error: schema rname is not defined - { "schemas": { "rname": { @@ -99,11 +105,11 @@ Warning: unknown field: grants at resources.clusters.rname in databricks.yml:7:7 -Error: cluster rname is not defined - { "clusters": { - "rname": {} + "rname": { + "custom_tags": {} + } } } diff --git a/acceptance/bundle/validate/empty_resources/with_permissions/output.txt b/acceptance/bundle/validate/empty_resources/with_permissions/output.txt index 718ede2ba3..c4a3537266 100644 --- a/acceptance/bundle/validate/empty_resources/with_permissions/output.txt +++ b/acceptance/bundle/validate/empty_resources/with_permissions/output.txt @@ -1,18 +1,23 @@ === resources.jobs.rname === -Error: job rname is not defined - { "jobs": { "rname": { - "permissions": [] + "deployment": { + "kind": "BUNDLE", + "metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/BUNDLE/default/state/metadata.json" + }, + "edit_mode": "UI_LOCKED", + "format": "MULTI_TASK", + "permissions": [], + "queue": { + "enabled": true + } } } } === resources.pipelines.rname === -Error: pipeline rname is not defined - { "pipelines": { "rname": { @@ -22,8 +27,6 @@ Error: pipeline rname is not defined } === resources.models.rname === -Error: model rname is not defined - { "models": { "rname": { @@ -33,11 +36,10 @@ Error: model rname is not defined } === resources.experiments.rname === -Error: experiment rname is not defined - { "experiments": { "rname": { + "name": ".", "permissions": [] } } @@ -48,8 +50,6 @@ Warning: unknown field: permissions at resources.registered_models.rname in databricks.yml:7:7 -Error: registered model rname is not defined - { "registered_models": { "rname": {} @@ -72,8 +72,6 @@ Warning: unknown field: permissions at resources.schemas.rname in databricks.yml:7:7 -Error: schema rname is not defined - { "schemas": { "rname": {} @@ -94,11 +92,10 @@ Warning: unknown field: permissions } === resources.clusters.rname === -Error: cluster rname is not defined - { "clusters": { "rname": { + "custom_tags": {}, "permissions": [] } } diff --git a/bundle/config/mutator/resourcemutator/apply_presets.go b/bundle/config/mutator/resourcemutator/apply_presets.go index 62bebae6fe..31c162f5ec 100644 --- a/bundle/config/mutator/resourcemutator/apply_presets.go +++ b/bundle/config/mutator/resourcemutator/apply_presets.go @@ -13,8 +13,12 @@ import ( "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/textutil" "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" + "github.com/databricks/databricks-sdk-go/service/serving" ) type applyPresets struct{} @@ -47,11 +51,17 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos tags := toTagArray(t.Tags) // Jobs presets: Prefix, Tags, JobsMaxConcurrentRuns, TriggerPauseStatus - for key, j := range r.Jobs { - if j.JobSettings == nil { - diags = diags.Extend(diag.Errorf("job %s is not defined", key)) + for _, j := range r.Jobs { + // Here and for other resources we follow the same approach for nil checks: + // If resource itself is nil, ignore it (It is currently allowed, but should have generic check with location-enriched diagnostics that disallows nils anywhere). + // If embedded pointer is nil, initialize it with empty struct. The fact that we embed pointer struct is implementation detail. + // It will remain nil if there are no fields in it that are provided by users, but that maybe ok, users could be relying on presets or defaults. + if j == nil { continue } + if j.JobSettings == nil { + j.JobSettings = &jobs.JobSettings{} + } j.Name = prefix + j.Name if len(tags) > 0 { if j.Tags == nil { @@ -86,11 +96,13 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos } // Pipelines presets: Prefix, PipelinesDevelopment - for key, p := range r.Pipelines { - if p.CreatePipeline == nil { - diags = diags.Extend(diag.Errorf("pipeline %s is not defined", key)) + for _, p := range r.Pipelines { + if p == nil { continue } + if p.CreatePipeline == nil { + p.CreatePipeline = &pipelines.CreatePipeline{} + } p.Name = prefix + p.Name if config.IsExplicitlyEnabled(t.PipelinesDevelopment) { p.Development = true @@ -102,11 +114,13 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos } // Models presets: Prefix, Tags - for key, m := range r.Models { - if m.Model == nil { - diags = diags.Extend(diag.Errorf("model %s is not defined", key)) + for _, m := range r.Models { + if m == nil { continue } + if m.Model == nil { + m.Model = &ml.Model{} + } m.Name = prefix + m.Name for _, t := range tags { exists := slices.ContainsFunc(m.Tags, func(modelTag ml.ModelTag) bool { @@ -120,11 +134,13 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos } // Experiments presets: Prefix, Tags - for key, e := range r.Experiments { - if e.Experiment == nil { - diags = diags.Extend(diag.Errorf("experiment %s is not defined", key)) + for _, e := range r.Experiments { + if e == nil { continue } + if e.Experiment == nil { + e.Experiment = &ml.Experiment{} + } filepath := e.Name dir := path.Dir(filepath) base := path.Base(filepath) @@ -148,22 +164,26 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos } // Model serving endpoint presets: Prefix - for key, e := range r.ModelServingEndpoints { - if e.CreateServingEndpoint == nil { - diags = diags.Extend(diag.Errorf("model serving endpoint %s is not defined", key)) + for _, e := range r.ModelServingEndpoints { + if e == nil { continue } + if e.CreateServingEndpoint == nil { + e.CreateServingEndpoint = &serving.CreateServingEndpoint{} + } e.Name = normalizePrefix(prefix) + e.Name // As of 2024-06, model serving endpoints don't yet support tags } // Registered models presets: Prefix - for key, m := range r.RegisteredModels { - if m.CreateRegisteredModelRequest == nil { - diags = diags.Extend(diag.Errorf("registered model %s is not defined", key)) + for _, m := range r.RegisteredModels { + if m == nil { continue } + if m.CreateRegisteredModelRequest == nil { + m.CreateRegisteredModelRequest = &catalog.CreateRegisteredModelRequest{} + } m.Name = normalizePrefix(prefix) + m.Name // As of 2024-06, registered models don't yet support tags @@ -186,22 +206,26 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos } // Schemas: Prefix - for key, s := range r.Schemas { - if s.CreateSchema == nil { - diags = diags.Extend(diag.Errorf("schema %s is not defined", key)) + for _, s := range r.Schemas { + if s == nil { continue } + if s.CreateSchema == nil { + s.CreateSchema = &catalog.CreateSchema{} + } s.Name = normalizePrefix(prefix) + s.Name // HTTP API for schemas doesn't yet support tags. It's only supported in // the Databricks UI and via the SQL API. } // Clusters: Prefix, Tags - for key, c := range r.Clusters { - if c.ClusterSpec == nil { - diags = diags.Extend(diag.Errorf("cluster %s is not defined", key)) + for _, c := range r.Clusters { + if c == nil { continue } + if c.ClusterSpec == nil { + c.ClusterSpec = &compute.ClusterSpec{} + } c.ClusterName = prefix + c.ClusterName if c.CustomTags == nil { c.CustomTags = make(map[string]string) @@ -216,11 +240,13 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos } // Dashboards: Prefix - for key, dashboard := range r.Dashboards { - if dashboard == nil || dashboard.Dashboard == nil { - diags = diags.Extend(diag.Errorf("dashboard %s s is not defined", key)) + for _, dashboard := range r.Dashboards { + if dashboard == nil { continue } + if dashboard.Dashboard == nil { + dashboard.Dashboard = &dashboards.Dashboard{} + } dashboard.DisplayName = prefix + dashboard.DisplayName }