Resolve variables in a loop (#2164)

## Changes
- Instead of doing 2 passes on variable resolution, do a loop until
there are no more updates (or we reach count 100).
- Stacked on top of #2163 which is a regression test for this:
acceptance/bundle/variables/complex-transitive-deep

## Tests
Existing tests, new regression tests.

These tests already passed before, added for completeness:
- acceptance/bundle/variables/cycle
- acceptance/bundle/variables/complex-cross-ref
This commit is contained in:
Denis Bilenko 2025-01-16 15:39:54 +01:00 committed by GitHub
parent f2bba632cb
commit 2e70558dc1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 156 additions and 12 deletions

View File

@ -0,0 +1,12 @@
bundle:
name: complex-cross-ref
variables:
a:
default:
a_1: 500
a_2: ${var.b.b_2}
b:
default:
b_1: ${var.a.a_1}
b_2: 2.5

View File

@ -0,0 +1,22 @@
{
"a": {
"default": {
"a_1": 500,
"a_2": 2.5
},
"value": {
"a_1": 500,
"a_2": 2.5
}
},
"b": {
"default": {
"b_1": 500,
"b_2": 2.5
},
"value": {
"b_1": 500,
"b_2": 2.5
}
}
}

View File

@ -0,0 +1 @@
$CLI bundle validate -o json | jq .variables

View File

@ -0,0 +1,7 @@
bundle:
name: cycle
variables:
a:
default:
hello: ${var.a}

View File

@ -0,0 +1,9 @@
Warning: Detected unresolved variables after 11 resolution rounds
Name: cycle
Target: default
Workspace:
User: $USERNAME
Path: /Workspace/Users/$USERNAME/.bundle/cycle/default
Found 1 warning

View File

@ -0,0 +1 @@
$CLI bundle validate

View File

@ -0,0 +1,10 @@
bundle:
name: cycle
variables:
a:
default:
hello: ${var.b}
b:
default:
hello: ${var.a}

View File

@ -0,0 +1,9 @@
Warning: Detected unresolved variables after 11 resolution rounds
Name: cycle
Target: default
Workspace:
User: $USERNAME
Path: /Workspace/Users/$USERNAME/.bundle/cycle/default
Found 1 warning

View File

@ -0,0 +1 @@
$CLI bundle validate

View File

@ -1,3 +1,3 @@
{
"spark.databricks.sql.initial.catalog.name": "${var.catalog}"
"spark.databricks.sql.initial.catalog.name": "hive_metastore"
}

View File

@ -0,0 +1,8 @@
bundle:
name: cycle
variables:
a:
default: ${var.b}
b:
default: ${var.a}

View File

@ -0,0 +1,14 @@
Error: cycle detected in field resolution: variables.a.default -> var.b -> var.a -> var.b
{
"a": {
"default": "${var.b}",
"value": "${var.b}"
},
"b": {
"default": "${var.a}",
"value": "${var.a}"
}
}
Exit code: 1

View File

@ -0,0 +1 @@
$CLI bundle validate -o json | jq .variables

View File

@ -3,6 +3,7 @@ package mutator
import (
"context"
"errors"
"fmt"
"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config"
@ -13,15 +14,37 @@ import (
"github.com/databricks/cli/libs/dyn/dynvar"
)
/*
For pathological cases, output and time grow exponentially.
On my laptop, timings for acceptance/bundle/variables/complex-cycle:
rounds time
9 0.10s
10 0.13s
11 0.27s
12 0.68s
13 1.98s
14 6.28s
15 21.70s
16 78.16s
*/
const maxResolutionRounds = 11
type resolveVariableReferences struct {
prefixes []string
pattern dyn.Pattern
lookupFn func(dyn.Value, dyn.Path, *bundle.Bundle) (dyn.Value, error)
skipFn func(dyn.Value) bool
extraRounds int
}
func ResolveVariableReferences(prefixes ...string) bundle.Mutator {
return &resolveVariableReferences{prefixes: prefixes, lookupFn: lookup}
return &resolveVariableReferences{
prefixes: prefixes,
lookupFn: lookup,
extraRounds: maxResolutionRounds - 1,
}
}
func ResolveVariableReferencesInLookup() bundle.Mutator {
@ -86,7 +109,36 @@ func (m *resolveVariableReferences) Apply(ctx context.Context, b *bundle.Bundle)
varPath := dyn.NewPath(dyn.Key("var"))
var diags diag.Diagnostics
maxRounds := 1 + m.extraRounds
for round := range maxRounds {
hasUpdates, newDiags := m.resolveOnce(b, prefixes, varPath)
diags = diags.Extend(newDiags)
if diags.HasError() {
break
}
if !hasUpdates {
break
}
if round >= maxRounds-1 {
diags = diags.Append(diag.Diagnostic{
Severity: diag.Warning,
Summary: fmt.Sprintf("Detected unresolved variables after %d resolution rounds", round+1),
// Would be nice to include names of the variables there, but that would complicate things more
})
break
}
}
return diags
}
func (m *resolveVariableReferences) resolveOnce(b *bundle.Bundle, prefixes []dyn.Path, varPath dyn.Path) (bool, diag.Diagnostics) {
var diags diag.Diagnostics
hasUpdates := false
err := b.Config.Mutate(func(root dyn.Value) (dyn.Value, error) {
// Synthesize a copy of the root that has all fields that are present in the type
// but not set in the dynamic value set to their corresponding empty value.
@ -129,6 +181,7 @@ func (m *resolveVariableReferences) Apply(ctx context.Context, b *bundle.Bundle)
if m.skipFn != nil && m.skipFn(v) {
return dyn.InvalidValue, dynvar.ErrSkipResolution
}
hasUpdates = true
return m.lookupFn(normalized, path, b)
}
}
@ -149,5 +202,6 @@ func (m *resolveVariableReferences) Apply(ctx context.Context, b *bundle.Bundle)
if err != nil {
diags = diags.Extend(diag.FromErr(err))
}
return diags
return hasUpdates, diags
}

View File

@ -66,11 +66,6 @@ func Initialize() bundle.Mutator {
"workspace",
"variables",
),
mutator.ResolveVariableReferences(
"bundle",
"workspace",
"variables",
),
mutator.MergeJobClusters(),
mutator.MergeJobParameters(),