Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate more variable tests to acceptance #2154

Merged
merged 4 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions acceptance/bundle/variables/complex-simple/databricks.yml
Original file line number Diff line number Diff line change
@@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The warning is legit, this belongs one level deeper.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Fixed in 4028584

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}
14 changes: 14 additions & 0 deletions acceptance/bundle/variables/complex-simple/output.txt
Original file line number Diff line number Diff line change
@@ -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"
}
}
]
1 change: 1 addition & 0 deletions acceptance/bundle/variables/complex-simple/script
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
$CLI bundle validate -o json | jq .resources.jobs.job1.job_clusters
Original file line number Diff line number Diff line change
@@ -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}
18 changes: 18 additions & 0 deletions acceptance/bundle/variables/complex-within-complex/output.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
Warning: unknown field: node_type_id
at resources.jobs.job1.job_clusters[0]
in databricks.yml:25: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
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
$CLI bundle validate -o json | jq .resources.jobs.job1.job_clusters
122 changes: 0 additions & 122 deletions bundle/config/mutator/resolve_variable_references_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,128 +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{
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{
Expand Down
Loading