Skip to content

Commit 86622c7

Browse files
authored
Fix nil location in sequences in expanded globs (#4465)
## Changes Adding location to the sequence in expanded globs. Location is present for the list element, but not for the list itself. I.e., `resources.pipelines.pipeline.environment.dependencies[0]` has a location, but `resources.pipelines.pipeline.environment.dependencies` does not. This PR fixes that ## Why In config sync, we check whether the field location is nil to see if the value is a CLI default. In fields affected by `expand_globs` mutator, every sequence item has its own location, but not the sequence itself. That causes config remote sync to work incorrectly - the field is considered as default, and added to the root of the resource, instead of patching the existing field ## Tests 1. Some test updates in config-remote-sync to capture the issue 2. Unit test for preserving location <!-- If your PR needs to be included in the release notes for next release, add a separate entry in NEXT_CHANGELOG.md as part of your PR. -->
1 parent 7cc04d8 commit 86622c7

File tree

5 files changed

+64
-2
lines changed

5 files changed

+64
-2
lines changed

acceptance/bundle/config-remote-sync/pipeline_fields/databricks.yml.tmpl

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ resources:
1414
- success@example.com
1515
alerts:
1616
- on-update-success
17+
environment:
18+
dependencies:
19+
- --editable /Workspace/foo
1720
libraries:
1821
- notebook:
1922
path: /Users/{{workspace_user_name}}/notebook

acceptance/bundle/config-remote-sync/pipeline_fields/output.txt

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ Detected changes in 1 resource(s):
99

1010
Resource: resources.pipelines.my_pipeline
1111
configuration['key2']: add
12+
environment.dependencies: replace
1213
notifications[0].alerts: replace
1314
notifications[0].email_recipients: replace
1415
schema: replace
@@ -27,7 +28,7 @@ Resource: resources.pipelines.my_pipeline
2728
-
2829
resources:
2930
pipelines:
30-
@@ -7,16 +6,20 @@
31+
@@ -7,19 +6,24 @@
3132
name: test-pipeline-[UNIQUE_NAME]
3233
catalog: main
3334
- schema: default
@@ -42,6 +43,10 @@ Resource: resources.pipelines.my_pipeline
4243
alerts:
4344
- on-update-success
4445
+ - on-update-failure
46+
environment:
47+
dependencies:
48+
- --editable /Workspace/foo
49+
+ - --editable /Workspace/bar
4550
libraries:
4651
- notebook:
4752
path: /Users/{{workspace_user_name}}/notebook

acceptance/bundle/config-remote-sync/pipeline_fields/script

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ r["schema"] = "prod"
1515
if "tags" not in r:
1616
r["tags"] = {}
1717
r["tags"]["foo"] = "bar"
18+
r.setdefault("environment", {}).setdefault("dependencies", []).append("--editable /Workspace/bar")
1819
EOF
1920

2021
# TODO add support for permissions

bundle/libraries/expand_glob_references.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ func (e *expand) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
201201
v, err = dyn.MapByPattern(v, expander.pattern, func(p dyn.Path, lv dyn.Value) (dyn.Value, error) {
202202
d, output := expander.fn(ctx, b, p, lv)
203203
diags = diags.Extend(d)
204-
return dyn.V(output), nil
204+
return dyn.NewValue(output, lv.Locations()), nil
205205
})
206206
if err != nil {
207207
return dyn.InvalidValue, err

bundle/libraries/expand_glob_references_test.go

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import (
1313
"github.com/databricks/cli/libs/dyn"
1414
"github.com/databricks/databricks-sdk-go/service/compute"
1515
"github.com/databricks/databricks-sdk-go/service/jobs"
16+
"github.com/databricks/databricks-sdk-go/service/pipelines"
17+
"github.com/stretchr/testify/assert"
1618
"github.com/stretchr/testify/require"
1719
)
1820

@@ -238,3 +240,54 @@ func TestGlobReferencesExpandedForEnvironmentsDeps(t *testing.T) {
238240
"/some/local/path/to/whl/*.whl",
239241
}, env.Spec.Dependencies)
240242
}
243+
244+
func TestExpandGlobReferencesPreservesLocations(t *testing.T) {
245+
dir := t.TempDir()
246+
247+
b := &bundle.Bundle{
248+
SyncRootPath: dir,
249+
Config: config.Root{
250+
Resources: config.Resources{
251+
Jobs: map[string]*resources.Job{
252+
"job": {
253+
JobSettings: jobs.JobSettings{
254+
Tasks: []jobs.Task{
255+
{
256+
TaskKey: "task",
257+
Libraries: []compute.Library{
258+
{Whl: "/Workspace/remote.whl"},
259+
},
260+
},
261+
},
262+
},
263+
},
264+
},
265+
Pipelines: map[string]*resources.Pipeline{
266+
"pipeline": {
267+
CreatePipeline: pipelines.CreatePipeline{
268+
Environment: &pipelines.PipelinesEnvironment{
269+
Dependencies: []string{
270+
"--editable /Workspace/foo",
271+
},
272+
},
273+
},
274+
},
275+
},
276+
},
277+
},
278+
}
279+
280+
loc := dyn.Location{File: filepath.Join(dir, "resource.yml"), Line: 10, Column: 5}
281+
bundletest.SetLocation(b, ".", []dyn.Location{loc})
282+
283+
diags := bundle.Apply(context.Background(), b, ExpandGlobReferences())
284+
require.Empty(t, diags)
285+
286+
libs, err := dyn.GetByPath(b.Config.Value(), dyn.MustPathFromString("resources.jobs.job.tasks[0].libraries"))
287+
require.NoError(t, err)
288+
assert.Equal(t, loc.File, libs.Location().File)
289+
290+
deps, err := dyn.GetByPath(b.Config.Value(), dyn.MustPathFromString("resources.pipelines.pipeline.environment.dependencies"))
291+
require.NoError(t, err)
292+
assert.Equal(t, loc.File, deps.Location().File)
293+
}

0 commit comments

Comments
 (0)