From ba7e37f9388f57e3510b34cf5b7591ed8f7dce7b Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 15 Jan 2025 12:35:18 +0100 Subject: [PATCH 1/4] Migrate TestResolveComplexVariable to acceptance test --- .../variables/complex-simple/databricks.yml | 28 +++++++++ .../variables/complex-simple/output.txt | 14 +++++ .../bundle/variables/complex-simple/script | 1 + .../resolve_variable_references_test.go | 58 ------------------- 4 files changed, 43 insertions(+), 58 deletions(-) create mode 100644 acceptance/bundle/variables/complex-simple/databricks.yml create mode 100644 acceptance/bundle/variables/complex-simple/output.txt create mode 100644 acceptance/bundle/variables/complex-simple/script diff --git a/acceptance/bundle/variables/complex-simple/databricks.yml b/acceptance/bundle/variables/complex-simple/databricks.yml new file mode 100644 index 0000000000..65e32e2983 --- /dev/null +++ b/acceptance/bundle/variables/complex-simple/databricks.yml @@ -0,0 +1,28 @@ +# This example works and properly merges resources.jobs.job1.job_clusters.new_cluster and ${var.cluster}. +# retaining num_workers, spark_version and overriding node_type_id. +# However, it prints a warning "expected map, found string" which is unnecessary. +bundle: + name: TestResolveComplexVariable + +variables: + type: "complex" + cluster: + value: + node_type_id: "Standard_DS3_v2" + num_workers: 2 + +resources: + jobs: + job1: + job_clusters: + - new_cluster: + node_type_id: "random" + spark_version: 13.3.x-scala2.12 + +targets: + dev: + resources: + jobs: + job1: + job_clusters: + - new_cluster: ${var.cluster} diff --git a/acceptance/bundle/variables/complex-simple/output.txt b/acceptance/bundle/variables/complex-simple/output.txt new file mode 100644 index 0000000000..4bd962c266 --- /dev/null +++ b/acceptance/bundle/variables/complex-simple/output.txt @@ -0,0 +1,14 @@ +Warning: expected map, found string + at variables.type + in databricks.yml:8:9 + +[ + { + "job_cluster_key": "", + "new_cluster": { + "node_type_id": "Standard_DS3_v2", + "num_workers": 2, + "spark_version": "13.3.x-scala2.12" + } + } +] diff --git a/acceptance/bundle/variables/complex-simple/script b/acceptance/bundle/variables/complex-simple/script new file mode 100644 index 0000000000..1c31d0b406 --- /dev/null +++ b/acceptance/bundle/variables/complex-simple/script @@ -0,0 +1 @@ +$CLI bundle validate -o json | jq .resources.jobs.job1.job_clusters diff --git a/bundle/config/mutator/resolve_variable_references_test.go b/bundle/config/mutator/resolve_variable_references_test.go index 6bf84ae6da..2ef2738498 100644 --- a/bundle/config/mutator/resolve_variable_references_test.go +++ b/bundle/config/mutator/resolve_variable_references_test.go @@ -16,64 +16,6 @@ import ( "github.com/stretchr/testify/require" ) -func TestResolveComplexVariable(t *testing.T) { - b := &bundle.Bundle{ - Config: config.Root{ - Bundle: config.Bundle{ - Name: "example", - }, - Variables: map[string]*variable.Variable{ - "cluster": { - Value: map[string]any{ - "node_type_id": "Standard_DS3_v2", - "num_workers": 2, - }, - Type: variable.VariableTypeComplex, - }, - }, - - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "job1": { - JobSettings: &jobs.JobSettings{ - JobClusters: []jobs.JobCluster{ - { - NewCluster: compute.ClusterSpec{ - NodeTypeId: "random", - }, - }, - }, - }, - }, - }, - }, - }, - } - - ctx := context.Background() - - // Assign the variables to the dynamic configuration. - diags := bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { - var p dyn.Path - var err error - - p = dyn.MustPathFromString("resources.jobs.job1.job_clusters[0]") - v, err = dyn.SetByPath(v, p.Append(dyn.Key("new_cluster")), dyn.V("${var.cluster}")) - require.NoError(t, err) - - return v, nil - }) - return diag.FromErr(err) - }) - require.NoError(t, diags.Error()) - - diags = bundle.Apply(ctx, b, ResolveVariableReferences("bundle", "workspace", "variables")) - require.NoError(t, diags.Error()) - require.Equal(t, "Standard_DS3_v2", b.Config.Resources.Jobs["job1"].JobSettings.JobClusters[0].NewCluster.NodeTypeId) - require.Equal(t, 2, b.Config.Resources.Jobs["job1"].JobSettings.JobClusters[0].NewCluster.NumWorkers) -} - func TestResolveComplexVariableReferencesWithComplexVariablesError(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ From 3c3b76ea078aa07a9fe0c00577cd9f55272cb55d Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 15 Jan 2025 12:52:48 +0100 Subject: [PATCH 2/4] Migrate TestResolveComplexVariableReferencesWithComplexVariablesError --- .../complex-within-complex/databricks.yml | 34 ++++++++++ .../complex-within-complex/output.txt | 18 ++++++ .../variables/complex-within-complex/script | 1 + .../resolve_variable_references_test.go | 64 ------------------- 4 files changed, 53 insertions(+), 64 deletions(-) create mode 100644 acceptance/bundle/variables/complex-within-complex/databricks.yml create mode 100644 acceptance/bundle/variables/complex-within-complex/output.txt create mode 100644 acceptance/bundle/variables/complex-within-complex/script diff --git a/acceptance/bundle/variables/complex-within-complex/databricks.yml b/acceptance/bundle/variables/complex-within-complex/databricks.yml new file mode 100644 index 0000000000..f1d77289ef --- /dev/null +++ b/acceptance/bundle/variables/complex-within-complex/databricks.yml @@ -0,0 +1,34 @@ +# Does not work currently, explicitly disabled, even though it works if you remove 'type: "complex"' lines +# Also fails to merge clusters. +bundle: + name: TestResolveComplexVariableReferencesWithComplexVariablesError + +variables: + cluster: + type: "complex" + value: + node_type_id: "Standard_DS3_v2" + num_workers: 2 + spark_conf: "${var.spark_conf}" + spark_conf: + type: "complex" + value: + spark.executor.memory: "4g" + spark.executor.cores: "2" + +resources: + jobs: + job1: + job_clusters: + - job_cluster_key: my_cluster + new_cluster: + node_type_id: "random" + +targets: + dev: + resources: + jobs: + job1: + job_clusters: + - job_cluster_key: my_cluster + new_cluster: ${var.cluster} diff --git a/acceptance/bundle/variables/complex-within-complex/output.txt b/acceptance/bundle/variables/complex-within-complex/output.txt new file mode 100644 index 0000000000..6f0b88a9c7 --- /dev/null +++ b/acceptance/bundle/variables/complex-within-complex/output.txt @@ -0,0 +1,18 @@ +Warning: unknown field: node_type_id + at resources.jobs.job1.job_clusters[0] + in databricks.yml:23:11 + +Error: complex variables cannot contain references to another complex variables + +[ + { + "job_cluster_key": "my_cluster", + "new_cluster": null + }, + { + "job_cluster_key": "my_cluster", + "new_cluster": "${var.cluster}" + } +] + +Exit code: 1 diff --git a/acceptance/bundle/variables/complex-within-complex/script b/acceptance/bundle/variables/complex-within-complex/script new file mode 100644 index 0000000000..1c31d0b406 --- /dev/null +++ b/acceptance/bundle/variables/complex-within-complex/script @@ -0,0 +1 @@ +$CLI bundle validate -o json | jq .resources.jobs.job1.job_clusters diff --git a/bundle/config/mutator/resolve_variable_references_test.go b/bundle/config/mutator/resolve_variable_references_test.go index 2ef2738498..4bae7867a0 100644 --- a/bundle/config/mutator/resolve_variable_references_test.go +++ b/bundle/config/mutator/resolve_variable_references_test.go @@ -16,70 +16,6 @@ import ( "github.com/stretchr/testify/require" ) -func TestResolveComplexVariableReferencesWithComplexVariablesError(t *testing.T) { - b := &bundle.Bundle{ - Config: config.Root{ - Bundle: config.Bundle{ - Name: "example", - }, - Variables: map[string]*variable.Variable{ - "cluster": { - Value: map[string]any{ - "node_type_id": "Standard_DS3_v2", - "num_workers": 2, - "spark_conf": "${var.spark_conf}", - }, - Type: variable.VariableTypeComplex, - }, - "spark_conf": { - Value: map[string]any{ - "spark.executor.memory": "4g", - "spark.executor.cores": "2", - }, - Type: variable.VariableTypeComplex, - }, - }, - - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "job1": { - JobSettings: &jobs.JobSettings{ - JobClusters: []jobs.JobCluster{ - { - NewCluster: compute.ClusterSpec{ - NodeTypeId: "random", - }, - }, - }, - }, - }, - }, - }, - }, - } - - ctx := context.Background() - - // Assign the variables to the dynamic configuration. - diags := bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { - var p dyn.Path - var err error - - p = dyn.MustPathFromString("resources.jobs.job1.job_clusters[0]") - v, err = dyn.SetByPath(v, p.Append(dyn.Key("new_cluster")), dyn.V("${var.cluster}")) - require.NoError(t, err) - - return v, nil - }) - return diag.FromErr(err) - }) - require.NoError(t, diags.Error()) - - diags = bundle.Apply(ctx, b, bundle.Seq(ResolveVariableReferencesInComplexVariables(), ResolveVariableReferences("bundle", "workspace", "variables"))) - require.ErrorContains(t, diags.Error(), "complex variables cannot contain references to another complex variables") -} - func TestResolveComplexVariableWithVarReference(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ From e1e3796327e04a5d65bb8077e07d0e6ed8e2fa88 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 15 Jan 2025 13:03:35 +0100 Subject: [PATCH 3/4] update --- acceptance/bundle/variables/complex-within-complex/output.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acceptance/bundle/variables/complex-within-complex/output.txt b/acceptance/bundle/variables/complex-within-complex/output.txt index 6f0b88a9c7..124bc33630 100644 --- a/acceptance/bundle/variables/complex-within-complex/output.txt +++ b/acceptance/bundle/variables/complex-within-complex/output.txt @@ -1,6 +1,6 @@ Warning: unknown field: node_type_id at resources.jobs.job1.job_clusters[0] - in databricks.yml:23:11 + in databricks.yml:25:11 Error: complex variables cannot contain references to another complex variables From 402858403d7ea0184c32517231a802cd3854c856 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 15 Jan 2025 15:41:40 +0100 Subject: [PATCH 4/4] move "type: complex" to proper level --- acceptance/bundle/variables/complex-simple/databricks.yml | 3 +-- acceptance/bundle/variables/complex-simple/output.txt | 4 ---- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/acceptance/bundle/variables/complex-simple/databricks.yml b/acceptance/bundle/variables/complex-simple/databricks.yml index 65e32e2983..135ff86cf6 100644 --- a/acceptance/bundle/variables/complex-simple/databricks.yml +++ b/acceptance/bundle/variables/complex-simple/databricks.yml @@ -1,12 +1,11 @@ # This example works and properly merges resources.jobs.job1.job_clusters.new_cluster and ${var.cluster}. # retaining num_workers, spark_version and overriding node_type_id. -# However, it prints a warning "expected map, found string" which is unnecessary. bundle: name: TestResolveComplexVariable variables: - type: "complex" cluster: + type: "complex" value: node_type_id: "Standard_DS3_v2" num_workers: 2 diff --git a/acceptance/bundle/variables/complex-simple/output.txt b/acceptance/bundle/variables/complex-simple/output.txt index 4bd962c266..16b0ec80f9 100644 --- a/acceptance/bundle/variables/complex-simple/output.txt +++ b/acceptance/bundle/variables/complex-simple/output.txt @@ -1,7 +1,3 @@ -Warning: expected map, found string - at variables.type - in databricks.yml:8:9 - [ { "job_cluster_key": "",