From f667e127df14f482bd86a7a808ea772b2d8d4123 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 20 Aug 2024 16:00:30 +0200 Subject: [PATCH] deprecate compute_id in favor of cluster_id --- bundle/config/bundle.go | 7 +- bundle/config/mutator/compute_id_compat.go | 68 +++++++++++++++++++ .../config/mutator/compute_id_compate_test.go | 31 +++++++++ bundle/config/mutator/mutator.go | 1 + bundle/config/mutator/override_compute.go | 8 +-- .../config/mutator/override_compute_test.go | 4 +- bundle/config/root.go | 6 +- bundle/config/target.go | 7 +- cmd/bundle/deploy.go | 11 ++- .../databricks_template_schema.json | 16 ----- .../template/databricks.yml.tmpl | 18 ----- .../clusters copy/template/hello_world.py | 1 - 12 files changed, 127 insertions(+), 51 deletions(-) create mode 100644 bundle/config/mutator/compute_id_compat.go create mode 100644 bundle/config/mutator/compute_id_compate_test.go delete mode 100644 internal/bundle/bundles/clusters copy/databricks_template_schema.json delete mode 100644 internal/bundle/bundles/clusters copy/template/databricks.yml.tmpl delete mode 100644 internal/bundle/bundles/clusters copy/template/hello_world.py diff --git a/bundle/config/bundle.go b/bundle/config/bundle.go index 78648dfd7..f533c4d18 100644 --- a/bundle/config/bundle.go +++ b/bundle/config/bundle.go @@ -38,8 +38,11 @@ type Bundle struct { // Annotated readonly as this should be set at the target level. Mode Mode `json:"mode,omitempty" bundle:"readonly"` - // Overrides the compute used for jobs and other supported assets. - ComputeID string `json:"compute_id,omitempty"` + // DEPRECATED: Overrides the compute used for jobs and other supported assets. + ComputeId string `json:"compute_id,omitempty"` + + // Overrides the cluster used for jobs and other supported assets. + ClusterId string `json:"cluster_id,omitempty"` // Deployment section specifies deployment related configuration for bundle Deployment Deployment `json:"deployment,omitempty"` diff --git a/bundle/config/mutator/compute_id_compat.go b/bundle/config/mutator/compute_id_compat.go new file mode 100644 index 000000000..eabfc9aa7 --- /dev/null +++ b/bundle/config/mutator/compute_id_compat.go @@ -0,0 +1,68 @@ +package mutator + +import ( + "context" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" +) + +type computeIdToClusterId struct{} + +func ComputeIdToClusterId() bundle.Mutator { + return &computeIdToClusterId{} +} + +func (m *computeIdToClusterId) Name() string { + return "ComputeIdToClusterId" +} + +func (m *computeIdToClusterId) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + // If the "compute_id" key is not set, just skip + if b.Config.Bundle.ComputeId == "" { + return nil + } + + var diags diag.Diagnostics + + // The "compute_id" key is set; rewrite it to "cluster_id". + err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + computeId, err := dyn.Get(v, "bundle.compute_id") + if err != nil { + return v, err + } + + if computeId.Kind() != dyn.KindInvalid { + p := dyn.NewPath(dyn.Key("bundle"), dyn.Key("compute_id")) + diags = diags.Append(diag.Diagnostic{ + Severity: diag.Warning, + Summary: "compute_id is deprecated, please use cluster_id instead", + Locations: computeId.Locations(), + Paths: []dyn.Path{p}, + }) + + nv, err := dyn.Set(v, "bundle.cluster_id", computeId) + if err != nil { + return dyn.InvalidValue, err + } + // Drop the "compute_id" key. + return dyn.Walk(nv, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + switch len(p) { + case 0, 1: + return v, nil + case 2: + if p[1] == dyn.Key("compute_id") { + return v, dyn.ErrDrop + } + } + return v, dyn.ErrSkip + }) + } + + return v, nil + }) + + diags = diags.Extend(diag.FromErr(err)) + return diags +} diff --git a/bundle/config/mutator/compute_id_compate_test.go b/bundle/config/mutator/compute_id_compate_test.go new file mode 100644 index 000000000..f6a8b7eb0 --- /dev/null +++ b/bundle/config/mutator/compute_id_compate_test.go @@ -0,0 +1,31 @@ +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/libs/diag" + "github.com/stretchr/testify/assert" +) + +func TestComputeIdToClusterId(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Bundle: config.Bundle{ + ComputeId: "compute-id", + }, + }, + } + + diags := bundle.Apply(context.Background(), b, mutator.ComputeIdToClusterId()) + assert.NoError(t, diags.Error()) + assert.Equal(t, "compute-id", b.Config.Bundle.ClusterId) + assert.Empty(t, b.Config.Bundle.ComputeId) + + assert.Len(t, diags, 1) + assert.Equal(t, "compute_id is deprecated, please use cluster_id instead", diags[0].Summary) + assert.Equal(t, diag.Warning, diags[0].Severity) +} diff --git a/bundle/config/mutator/mutator.go b/bundle/config/mutator/mutator.go index 0458beff4..faf50ae6e 100644 --- a/bundle/config/mutator/mutator.go +++ b/bundle/config/mutator/mutator.go @@ -23,6 +23,7 @@ func DefaultMutators() []bundle.Mutator { VerifyCliVersion(), EnvironmentsToTargets(), + ComputeIdToClusterId(), InitializeVariables(), DefineDefaultTarget(), LoadGitDetails(), diff --git a/bundle/config/mutator/override_compute.go b/bundle/config/mutator/override_compute.go index 73fbad364..5700cdf26 100644 --- a/bundle/config/mutator/override_compute.go +++ b/bundle/config/mutator/override_compute.go @@ -39,22 +39,22 @@ func overrideJobCompute(j *resources.Job, compute string) { func (m *overrideCompute) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { if b.Config.Bundle.Mode != config.Development { - if b.Config.Bundle.ComputeID != "" { + if b.Config.Bundle.ClusterId != "" { return diag.Errorf("cannot override compute for an target that does not use 'mode: development'") } return nil } if v := env.Get(ctx, "DATABRICKS_CLUSTER_ID"); v != "" { - b.Config.Bundle.ComputeID = v + b.Config.Bundle.ClusterId = v } - if b.Config.Bundle.ComputeID == "" { + if b.Config.Bundle.ClusterId == "" { return nil } r := b.Config.Resources for i := range r.Jobs { - overrideJobCompute(r.Jobs[i], b.Config.Bundle.ComputeID) + overrideJobCompute(r.Jobs[i], b.Config.Bundle.ClusterId) } return nil diff --git a/bundle/config/mutator/override_compute_test.go b/bundle/config/mutator/override_compute_test.go index 152ee543e..369447d7e 100644 --- a/bundle/config/mutator/override_compute_test.go +++ b/bundle/config/mutator/override_compute_test.go @@ -20,7 +20,7 @@ func TestOverrideDevelopment(t *testing.T) { Config: config.Root{ Bundle: config.Bundle{ Mode: config.Development, - ComputeID: "newClusterID", + ClusterId: "newClusterID", }, Resources: config.Resources{ Jobs: map[string]*resources.Job{ @@ -144,7 +144,7 @@ func TestOverrideProduction(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ Bundle: config.Bundle{ - ComputeID: "newClusterID", + ClusterId: "newClusterID", }, Resources: config.Resources{ Jobs: map[string]*resources.Job{ diff --git a/bundle/config/root.go b/bundle/config/root.go index 86dc33921..91ab46779 100644 --- a/bundle/config/root.go +++ b/bundle/config/root.go @@ -366,9 +366,9 @@ func (r *Root) MergeTargetOverrides(name string) error { } } - // Merge `compute_id`. This field must be overwritten if set, not merged. - if v := target.Get("compute_id"); v.Kind() != dyn.KindInvalid { - root, err = dyn.SetByPath(root, dyn.NewPath(dyn.Key("bundle"), dyn.Key("compute_id")), v) + // Merge `cluster_id`. This field must be overwritten if set, not merged. + if v := target.Get("cluster_id"); v.Kind() != dyn.KindInvalid { + root, err = dyn.SetByPath(root, dyn.NewPath(dyn.Key("bundle"), dyn.Key("cluster_id")), v) if err != nil { return err } diff --git a/bundle/config/target.go b/bundle/config/target.go index a2ef4d735..cfc81c731 100644 --- a/bundle/config/target.go +++ b/bundle/config/target.go @@ -24,8 +24,11 @@ type Target struct { // name prefix of deployed resources. Presets Presets `json:"presets,omitempty"` - // Overrides the compute used for jobs and other supported assets. - ComputeID string `json:"compute_id,omitempty"` + // DEPRECATED: Overrides the compute used for jobs and other supported assets. + ComputeId string `json:"compute_id,omitempty"` + + // Overrides the cluster used for jobs and other supported assets. + ClusterId string `json:"cluster_id,omitempty"` Bundle *Bundle `json:"bundle,omitempty"` diff --git a/cmd/bundle/deploy.go b/cmd/bundle/deploy.go index 1166875ab..0c9eb623c 100644 --- a/cmd/bundle/deploy.go +++ b/cmd/bundle/deploy.go @@ -23,13 +23,15 @@ func newDeployCommand() *cobra.Command { var force bool var forceLock bool var failOnActiveRuns bool - var computeID string + var clusterId string var autoApprove bool cmd.Flags().BoolVar(&force, "force", false, "Force-override Git branch validation.") cmd.Flags().BoolVar(&forceLock, "force-lock", false, "Force acquisition of deployment lock.") cmd.Flags().BoolVar(&failOnActiveRuns, "fail-on-active-runs", false, "Fail if there are running jobs or pipelines in the deployment.") - cmd.Flags().StringVarP(&computeID, "compute-id", "c", "", "Override compute in the deployment with the given compute ID.") + cmd.Flags().StringVar(&clusterId, "compute-id", "", "Override cluster in the deployment with the given compute ID.") + cmd.Flags().StringVarP(&clusterId, "cluster-id", "c", "", "Override cluster in the deployment with the given cluster ID.") cmd.Flags().BoolVar(&autoApprove, "auto-approve", false, "Skip interactive approvals that might be required for deployment.") + cmd.Flags().MarkDeprecated("compute-id", "use --cluster-id instead") cmd.RunE = func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() @@ -42,7 +44,10 @@ func newDeployCommand() *cobra.Command { b.AutoApprove = autoApprove if cmd.Flag("compute-id").Changed { - b.Config.Bundle.ComputeID = computeID + b.Config.Bundle.ClusterId = clusterId + } + if cmd.Flag("cluster-id").Changed { + b.Config.Bundle.ClusterId = clusterId } if cmd.Flag("fail-on-active-runs").Changed { b.Config.Bundle.Deployment.FailOnActiveRuns = failOnActiveRuns diff --git a/internal/bundle/bundles/clusters copy/databricks_template_schema.json b/internal/bundle/bundles/clusters copy/databricks_template_schema.json deleted file mode 100644 index c1c5cf12e..000000000 --- a/internal/bundle/bundles/clusters copy/databricks_template_schema.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "properties": { - "unique_id": { - "type": "string", - "description": "Unique ID for job name" - }, - "spark_version": { - "type": "string", - "description": "Spark version used for job cluster" - }, - "node_type_id": { - "type": "string", - "description": "Node type id for job cluster" - } - } -} diff --git a/internal/bundle/bundles/clusters copy/template/databricks.yml.tmpl b/internal/bundle/bundles/clusters copy/template/databricks.yml.tmpl deleted file mode 100644 index a88cbd30e..000000000 --- a/internal/bundle/bundles/clusters copy/template/databricks.yml.tmpl +++ /dev/null @@ -1,18 +0,0 @@ -bundle: - name: basic - -workspace: - root_path: "~/.bundle/{{.unique_id}}" - -resources: - jobs: - foo: - name: test-job-basic-{{.unique_id}} - tasks: - - task_key: my_notebook_task - new_cluster: - num_workers: 1 - spark_version: "{{.spark_version}}" - node_type_id: "{{.node_type_id}}" - spark_python_task: - python_file: ./hello_world.py diff --git a/internal/bundle/bundles/clusters copy/template/hello_world.py b/internal/bundle/bundles/clusters copy/template/hello_world.py deleted file mode 100644 index f301245e2..000000000 --- a/internal/bundle/bundles/clusters copy/template/hello_world.py +++ /dev/null @@ -1 +0,0 @@ -print("Hello World!")