Skip to content

Commit 634cea7

Browse files
denikIsaac
andauthored
Do not check required fields inside a pure reference (#7226)
## Changes The required-field warning check (`bundle/config/validate/required.go`) skips a value that is a pure `${...}` reference. ## Why `schedule: ${resources.jobs.base.schedule}` produced `required field "quartz_cron_expression" is not set` (and `timezone_id`) for the referring job, although the referenced schedule sets both. The same happens on main for e.g. `queue: ${...}`. The referenced value is checked where it is defined. Stacked on #7225 (the test only gets past validate with that fix). Port note: the fix lives in `warnForMissingFields` and is dyn-specific (it walks the dyn tree); a typed walker needs the same skip for fields held as references. ## Tests - Unit test in `bundle/config/validate/required_test.go`. - `acceptance/bundle/validate/references/job_schedule_whole_reference`: warnings gone, Badness removed. This pull request and its description were written by Isaac. --------- Co-authored-by: Isaac <no-reply@databricks.com>
1 parent da7a8be commit 634cea7

5 files changed

Lines changed: 38 additions & 18 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* Removed spurious "required field is not set" warnings for a field whose value is a reference to a whole block, such as `schedule: ${resources.jobs.base.schedule}`. ([#7226](https://github.com/databricks/cli/pull/7226))

‎acceptance/bundle/validate/references/job_schedule_whole_reference/output.txt‎

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,14 @@
11

22
>>> [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
10-
113
Name: test-bundle
124
Target: default
135
Workspace:
146
User: [USERNAME]
157
Path: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default
168

17-
Found 2 warnings
9+
Validation OK!
1810

1911
>>> [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-
2812
{
2913
"pause_status": "UNPAUSED",
3014
"quartz_cron_expression": "0 0 * * * ?",

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

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

‎bundle/config/validate/required.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/databricks/cli/bundle/internal/validation/generated"
1212
"github.com/databricks/cli/libs/diag"
1313
"github.com/databricks/cli/libs/dyn"
14+
"github.com/databricks/cli/libs/dyn/dynvar"
1415
"github.com/databricks/cli/libs/structs/structpath"
1516
)
1617

@@ -50,6 +51,12 @@ func warnForMissingFields(ctx context.Context, b *bundle.Bundle) diag.Diagnostic
5051
return nil
5152
}
5253

54+
// A pure reference (e.g. "${resources.jobs.base.schedule}") has no fields of its own;
55+
// the referenced value is checked where it is defined.
56+
if s, ok := v.AsString(); ok && dynvar.IsPureVariableReference(s) {
57+
return nil
58+
}
59+
5360
fields := generated.RequiredFields[pattern.String()]
5461
for _, field := range fields {
5562
vv := v.Get(field)
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package validate
2+
3+
import (
4+
"testing"
5+
6+
"github.com/databricks/cli/bundle"
7+
"github.com/databricks/cli/bundle/config"
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
10+
)
11+
12+
func TestRequiredSkipsPureReference(t *testing.T) {
13+
root, diags := config.LoadFromBytes("databricks.yml", []byte(`
14+
resources:
15+
jobs:
16+
viaref:
17+
schedule: ${resources.jobs.base.schedule}
18+
missing:
19+
schedule:
20+
timezone_id: UTC
21+
`))
22+
require.Empty(t, diags)
23+
b := &bundle.Bundle{Config: *root}
24+
25+
diags = Required().Apply(t.Context(), b)
26+
require.Len(t, diags, 1)
27+
assert.Equal(t, `required field "quartz_cron_expression" is not set`, diags[0].Summary)
28+
assert.Equal(t, "resources.jobs.missing.schedule", diags[0].Paths[0].String())
29+
}

0 commit comments

Comments
 (0)