Skip to content

Commit 8b4a341

Browse files
denikIsaac
andauthored
Do not write job defaults into a reference to a whole block (#7225)
## Changes `bundle.SetDefault` skips a matched value that is a pure `${...}` reference instead of indexing into it. Applies to every default set in `resourcemutator` (e.g. `schedule`/`trigger`/`continuous.pause_status`). ## Why `schedule: ${resources.jobs.base.schedule}` made `bundle validate` fail with `expected a map to index "pause_status", found string`. The referenced schedule gets the default itself, so the resolved value has it (see the plan output in the test). Port note: the fix lives in `bundle/set_default.go` and is dyn-specific (a reference is a string in the dyn tree); on typed structs the equivalent is to skip a field held as a reference. ## Tests - Unit test in `bundle/set_default_test.go`. - `acceptance/bundle/validate/references/job_{schedule,trigger}_whole_reference`: Badness removed; validate and plan succeed. The schedule test keeps a Badness for the spurious required-field warnings, fixed in the follow-up. This pull request and its description were written by Isaac. --------- Co-authored-by: Isaac <no-reply@databricks.com>
1 parent d3e5dde commit 8b4a341

9 files changed

Lines changed: 87 additions & 13 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* Fixed `bundle validate` failing with "expected a map to index" when a job `schedule`, `trigger` or `continuous` is a reference to a whole block, such as `${resources.jobs.base.schedule}`. ([#7225](https://github.com/databricks/cli/pull/7225))
Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,32 @@
11

2-
>>> musterr [CLI] bundle validate
3-
Error: expected a map to index "pause_status", found string
2+
>>> [CLI] bundle validate
3+
Warning: required field "quartz_cron_expression" is not set
4+
at resources.jobs.viaref.schedule
5+
in databricks.yml:9:17
6+
7+
Warning: required field "timezone_id" is not set
8+
at resources.jobs.viaref.schedule
9+
in databricks.yml:9:17
410

511
Name: test-bundle
612
Target: default
713
Workspace:
814
User: [USERNAME]
915
Path: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default
1016

11-
Found 1 error
17+
Found 2 warnings
18+
19+
>>> [CLI] bundle plan -o json
20+
Warning: required field "quartz_cron_expression" is not set
21+
at resources.jobs.viaref.schedule
22+
in databricks.yml:9:17
23+
24+
Warning: required field "timezone_id" is not set
25+
at resources.jobs.viaref.schedule
26+
in databricks.yml:9:17
27+
28+
{
29+
"pause_status": "UNPAUSED",
30+
"quartz_cron_expression": "0 0 * * * ?",
31+
"timezone_id": "UTC"
32+
}
Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
# The schedule of viaref is a reference to the whole schedule of another job.
2-
# The defaults set schedule.pause_status (and trigger, continuous) of every job by path, which cannot be done in a reference.
3-
trace musterr $CLI bundle validate
2+
# The defaults (e.g. pause_status) are not written into the reference; the referenced schedule gets them.
3+
trace $CLI bundle validate
4+
trace $CLI bundle plan -o json | jq '.plan."resources.jobs.viaref".new_state.value.schedule'
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
Badness = "The default of schedule.pause_status is written by path, which fails with 'expected a map to index pause_status, found string' when the schedule is a reference to a whole schedule of another job; validate should accept it."
1+
Badness = "The required-field check looks inside the reference: it warns that quartz_cron_expression and timezone_id are not set at resources.jobs.viaref.schedule, although the referenced schedule sets both."
Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
11

2-
>>> musterr [CLI] bundle validate
3-
Error: expected a map to index "pause_status", found string
4-
2+
>>> [CLI] bundle validate
53
Name: test-bundle
64
Target: default
75
Workspace:
86
User: [USERNAME]
97
Path: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default
108

11-
Found 1 error
9+
Validation OK!
10+
11+
>>> [CLI] bundle plan -o json
12+
{
13+
"file_arrival": {
14+
"url": "s3://bucket/path"
15+
},
16+
"pause_status": "UNPAUSED"
17+
}
Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
# The trigger of viaref is a reference to the whole trigger of another job.
2-
# The defaults set trigger.pause_status (and trigger, continuous) of every job by path, which cannot be done in a reference.
3-
trace musterr $CLI bundle validate
2+
# The defaults (e.g. pause_status) are not written into the reference; the referenced trigger gets them.
3+
trace $CLI bundle validate
4+
trace $CLI bundle plan -o json | jq '.plan."resources.jobs.viaref".new_state.value.trigger'

‎acceptance/bundle/validate/references/job_trigger_whole_reference/test.toml‎

Lines changed: 0 additions & 1 deletion
This file was deleted.

‎bundle/set_default.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66

77
"github.com/databricks/cli/libs/diag"
88
"github.com/databricks/cli/libs/dyn"
9+
"github.com/databricks/cli/libs/dyn/dynvar"
910
"github.com/databricks/cli/libs/logdiag"
1011
)
1112

@@ -30,6 +31,11 @@ func (m *setDefault) Name() string {
3031
func (m *setDefault) Apply(ctx context.Context, b *Bundle) diag.Diagnostics {
3132
err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) {
3233
return dyn.MapByPattern(v, m.pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) {
34+
// A default cannot be written into a pure reference to a whole block
35+
// (e.g. "${resources.jobs.base.schedule}"), so leave it to the referenced value.
36+
if s, ok := v.AsString(); ok && dynvar.IsPureVariableReference(s) {
37+
return v, nil
38+
}
3339
_, err := dyn.GetByPath(v, m.key)
3440
switch {
3541
case dyn.IsNoSuchKeyError(err):

‎bundle/set_default_test.go‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package bundle
2+
3+
import (
4+
"testing"
5+
6+
"github.com/databricks/cli/bundle/config"
7+
"github.com/databricks/cli/libs/dyn"
8+
"github.com/databricks/cli/libs/logdiag"
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
)
12+
13+
func TestSetDefaultSkipsPureReference(t *testing.T) {
14+
root, diags := config.LoadFromBytes("databricks.yml", []byte(`
15+
resources:
16+
jobs:
17+
base:
18+
schedule:
19+
quartz_cron_expression: "0 0 * * * ?"
20+
viaref:
21+
schedule: ${resources.jobs.base.schedule}
22+
`))
23+
require.Empty(t, diags)
24+
b := &Bundle{Config: *root}
25+
26+
ctx := logdiag.InitContext(t.Context())
27+
logdiag.SetCollect(ctx, true)
28+
SetDefault(ctx, b, "resources.jobs.*.schedule.pause_status", "UNPAUSED")
29+
require.Empty(t, logdiag.FlushCollected(ctx))
30+
31+
v := b.Config.Value()
32+
base, err := dyn.GetByPath(v, dyn.MustPathFromString("resources.jobs.base.schedule.pause_status"))
33+
require.NoError(t, err)
34+
assert.Equal(t, "UNPAUSED", base.MustString())
35+
36+
viaref, err := dyn.GetByPath(v, dyn.MustPathFromString("resources.jobs.viaref.schedule"))
37+
require.NoError(t, err)
38+
assert.Equal(t, "${resources.jobs.base.schedule}", viaref.MustString())
39+
}

0 commit comments

Comments
 (0)