Skip to content

Commit

Permalink
Retain location information of variable reference (#1333)
Browse files Browse the repository at this point in the history
## Changes

Variable substitution works as if the variable reference is literally
replaced with its contents.

The following fields should be interpreted in the same way regardless of
where the variable is defined:
```yaml
foo: ${var.some_path}
bar: "./${var.some_path}"
```

Before this change, `foo` would inherit the location information of the
variable definition. After this change, it uses the location information
of the variable reference, making the behavior for `foo` and `bar`
identical.

Fixes #1330.

## Tests

The new test passes only with the fix.
  • Loading branch information
pietern authored Apr 3, 2024
1 parent f28a9d7 commit a95b1c7
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 1 deletion.
33 changes: 33 additions & 0 deletions bundle/tests/relative_path_translation/databricks.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
bundle:
name: relative_path_translation

include:
- resources/*.yml

variables:
file_path:
# This path is expected to be resolved relative to where it is used.
default: ../src/file1.py

workspace:
file_path: /remote

targets:
default:
default: true

override:
variables:
file_path: ./src/file2.py

resources:
jobs:
job:
tasks:
- task_key: local
spark_python_task:
python_file: ./src/file2.py

- task_key: variable_reference
spark_python_task:
python_file: ${var.file_path}
14 changes: 14 additions & 0 deletions bundle/tests/relative_path_translation/resources/job.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
resources:
jobs:
job:
tasks:
- task_key: local
spark_python_task:
python_file: ../src/file1.py

- task_key: variable_reference
spark_python_task:
# Note: this is a pure variable reference yet needs to persist the location
# of the reference, not the location of the variable value.
# Also see https://github.com/databricks/cli/issues/1330.
python_file: ${var.file_path}
Empty file.
Empty file.
53 changes: 53 additions & 0 deletions bundle/tests/relative_path_translation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package config_tests

import (
"context"
"testing"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/phases"
"github.com/databricks/databricks-sdk-go/config"
"github.com/databricks/databricks-sdk-go/experimental/mocks"
"github.com/databricks/databricks-sdk-go/service/iam"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
)

func configureMock(t *testing.T, b *bundle.Bundle) {
// Configure mock workspace client
m := mocks.NewMockWorkspaceClient(t)
m.WorkspaceClient.Config = &config.Config{
Host: "https://mock.databricks.workspace.com",
}
m.GetMockCurrentUserAPI().EXPECT().Me(mock.Anything).Return(&iam.User{
UserName: "user@domain.com",
}, nil)
b.SetWorkpaceClient(m.WorkspaceClient)
}

func TestRelativePathTranslationDefault(t *testing.T) {
b := loadTarget(t, "./relative_path_translation", "default")
configureMock(t, b)

diags := bundle.Apply(context.Background(), b, phases.Initialize())
require.NoError(t, diags.Error())

t0 := b.Config.Resources.Jobs["job"].Tasks[0]
assert.Equal(t, "/remote/src/file1.py", t0.SparkPythonTask.PythonFile)
t1 := b.Config.Resources.Jobs["job"].Tasks[1]
assert.Equal(t, "/remote/src/file1.py", t1.SparkPythonTask.PythonFile)
}

func TestRelativePathTranslationOverride(t *testing.T) {
b := loadTarget(t, "./relative_path_translation", "override")
configureMock(t, b)

diags := bundle.Apply(context.Background(), b, phases.Initialize())
require.NoError(t, diags.Error())

t0 := b.Config.Resources.Jobs["job"].Tasks[0]
assert.Equal(t, "/remote/src/file2.py", t0.SparkPythonTask.PythonFile)
t1 := b.Config.Resources.Jobs["job"].Tasks[1]
assert.Equal(t, "/remote/src/file2.py", t1.SparkPythonTask.PythonFile)
}
7 changes: 6 additions & 1 deletion libs/dyn/dynvar/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,12 @@ func (r *resolver) resolveRef(ref ref, seen []string) (dyn.Value, error) {
if ref.isPure() && complete {
// If the variable reference is pure, we can substitute it.
// This is useful for interpolating values of non-string types.
return resolved[0], nil
//
// Note: we use the location of the variable reference to preserve the information
// of where it is used. This also means that relative path resolution is done
// relative to where a variable is used, not where it is defined.
//
return dyn.NewValue(resolved[0].Value(), ref.value.Location()), nil
}

// Not pure; perform string interpolation.
Expand Down

0 comments on commit a95b1c7

Please sign in to comment.