Add warning for when required bundle fields are not set #3044

shreyas-goenka · 2025-06-12T11:52:01Z

Changes

This PR adds code to emit warnings when required values are not set in the bundle configuration tree.

Note: We do not show a warning for zero values like "" or 0, only when the value is actually omitted from YAML. Acceptance test at bundle/validate/required has coverage for these cases.

Why

Better user experience for the customers.

Tests

A new acceptance test at bundle/validate/required. The warnings also show up in existing acceptance tests.

bundle/config/validate/required.go

andrewnester · 2025-07-17T14:29:05Z

bundle/config/validate/required.go

+		for _, field := range fields {
+			vv := v.Get(field)
+			if vv.Kind() == dyn.KindInvalid || vv.Kind() == dyn.KindNil {
+				diags = diags.Append(diag.Diagnostic{


I think it can be converted to use logdiag.LogDiag

It's intentionally collection based because of the sorting needed below. bundle.Apply calls logdiag.LogDiag anyway on these.

If I understood the idea behind logdiag is that eventually we want to replace all mutator to never return anything. Can you instead sort fields to make it determenistic?

andrewnester · 2025-07-17T14:31:46Z

bundle/config/validate/required.go

+	}
+
+	// Sort diagnostics to make them deterministic
+	sort.Slice(diags, func(i, j int) bool {


This purely for tests correct?

andrewnester · 2025-07-17T14:33:26Z

bundle/internal/bundletest/mutator_benchmark_test.go

+	"github.com/stretchr/testify/assert"
+)
+
+func benchmarkRequiredMutator(b *testing.B, numJobs int) {


Should it be a separate PR instead?

I think the current PR is a better destination because the feature is co-versioned with its benchmarks. I don't mind either way though.

andrewnester · 2025-07-18T12:31:55Z

bundle/config/validate/required.go

+		for _, field := range fields {
+			vv := v.Get(field)
+			if vv.Kind() == dyn.KindInvalid || vv.Kind() == dyn.KindNil {
+				diags = diags.Append(diag.Diagnostic{


If I understood the idea behind logdiag is that eventually we want to replace all mutator to never return anything. Can you instead sort fields to make it determenistic?

shreyas-goenka · 2025-07-18T12:51:29Z

@andrewnester I tried it out. Sorting by fields alone does not make the diagnostics deterministic. Loading the bundle configuration itself is non-deterministic w.r.t the order of fields in a map.

I think the current approach does not go against the spirit of logdiag. The diagnostics are all collected in a single function and are streamed as soon as the mutator exits.

## Release v0.261.0 ### Notable Changes The following CLI commands now have additional required positional arguments: * `alerts-v2 update-alert ID UPDATE_MASK` - Update an alert (v2) * `database update-database-instance NAME UPDATE_MASK` - Update a database instance * `external-lineage create-external-lineage-relationship SOURCE TARGET` - Create an external lineage relationship * `external-lineage update-external-lineage-relationship UPDATE_MASK SOURCE TARGET` - Update an external lineage relationship * `external-metadata update-external-metadata NAME UPDATE_MASK SYSTEM_TYPE ENTITY_TYPE` - Update external metadata * `feature-store update-online-store NAME UPDATE_MASK CAPACITY` - Update an online store * `lakeview create-schedule DASHBOARD_ID CRON_SCHEDULE` - Create a schedule * `lakeview create-subscription DASHBOARD_ID SCHEDULE_ID SUBSCRIBER` - Create a subscription * `lakeview update-schedule DASHBOARD_ID SCHEDULE_ID CRON_SCHEDULE` - Update a schedule * `network-connectivity update-private-endpoint-rule NETWORK_CONNECTIVITY_CONFIG_ID PRIVATE_ENDPOINT_RULE_ID UPDATE_MASK` - Update a private endpoint rule ### CLI * Add required query parameters as positional arguments in CLI commands ([#3289](#3289)) ### Bundles * Fixed an issue where `allow_duplicate_names` field on the pipeline definition was ignored by the bundle ([#3274](#3274)) * Add warning for when required bundle fields are not set ([#3044](#3044))

denik · 2025-07-29T08:36:52Z

acceptance/bundle/quality_monitor/output.txt

 >>> [CLI] bundle validate -o json -t development
+Warning: required field "quartz_cron_expression" is not set
+  at resources.quality_monitors.my_monitor.schedule
+  in databricks.yml:17:9


This warning should not be there, actually. The whole 'schedule' field is cleared by dev mode and it's okay, not user's problem. However, in the current implementation it is set to null. That null should be treated as absence.

I noticed this because my PR improving null handling actually makes schedule disappear and that removes the warnings: https://github.com/databricks/cli/pull/3230/files#diff-18b1807771a343c3d7e7c543232e7ec53637f4a65b1191ab04becc7706b5fcd5L3

However, we should probably look into fixing null handling in required logic specifically. It may also come from other places.

The whole 'schedule' field is cleared by dev mode and it's okay

That seems like a mistake if this is a field that is required by the API. Does the API succeed even if this field is missing?

## Why This PR mirrors the validation we added for required fields not being set: #3044 ## Tests A new acceptance test.

shreyas-goenka added 6 commits June 11, 2025 19:27

Generate required fields for DABs

b7e0bed

some cleanup

4c327c5

some cleanup

f612f5a

add generate

9199d25

fix and generate

de8477e

Merge remote-tracking branch 'origin' into generate-required-v3

849787f

shreyas-goenka temporarily deployed to test-trigger-is June 12, 2025 11:52 — with GitHub Actions Inactive

shreyas-goenka changed the base branch from main to generate-required-v3 June 12, 2025 11:52

shreyas-goenka temporarily deployed to test-trigger-is June 12, 2025 12:08 — with GitHub Actions Inactive

shreyas-goenka temporarily deployed to test-trigger-is June 12, 2025 12:16 — with GitHub Actions Inactive

shreyas-goenka temporarily deployed to test-trigger-is June 12, 2025 12:19 — with GitHub Actions Inactive

shreyas-goenka temporarily deployed to test-trigger-is June 12, 2025 12:42 — with GitHub Actions Inactive

shreyas-goenka temporarily deployed to test-trigger-is June 12, 2025 12:44 — with GitHub Actions Inactive

shreyas-goenka temporarily deployed to test-trigger-is June 12, 2025 12:50 — with GitHub Actions Inactive

shreyas-goenka marked this pull request as ready for review June 12, 2025 12:56

shreyas-goenka requested review from andrewnester, anton-107, denik and pietern as code owners June 12, 2025 12:56

minor rename to type

427ae3b

denik reviewed Jun 13, 2025

View reviewed changes

bundle/config/validate/required.go Outdated Show resolved Hide resolved

shreyas-goenka temporarily deployed to test-trigger-is June 13, 2025 12:18 — with GitHub Actions Inactive

shreyas-goenka temporarily deployed to test-trigger-is June 13, 2025 12:22 — with GitHub Actions Inactive

shreyas-goenka requested a review from denik June 13, 2025 12:24

shreyas-goenka temporarily deployed to test-trigger-is June 13, 2025 12:26 — with GitHub Actions Inactive

shreyas-goenka temporarily deployed to test-trigger-is June 13, 2025 14:28 — with GitHub Actions Inactive

shreyas-goenka temporarily deployed to test-trigger-is June 13, 2025 14:38 — with GitHub Actions Inactive

shreyas-goenka marked this pull request as draft June 16, 2025 13:19

shreyas-goenka temporarily deployed to test-trigger-is June 17, 2025 16:24 — with GitHub Actions Inactive

shreyas-goenka temporarily deployed to test-trigger-is June 17, 2025 16:42 — with GitHub Actions Inactive

revert node test

2c68220

shreyas-goenka temporarily deployed to test-trigger-is July 11, 2025 12:42 — with GitHub Actions Inactive

fix tests

e12d8f2

shreyas-goenka temporarily deployed to test-trigger-is July 11, 2025 13:26 — with GitHub Actions Inactive

lint

93ca657

shreyas-goenka temporarily deployed to test-trigger-is July 11, 2025 13:44 — with GitHub Actions Inactive

typo

2229963

shreyas-goenka temporarily deployed to test-trigger-is July 11, 2025 13:47 — with GitHub Actions Inactive

add empty case

b4e1596

shreyas-goenka temporarily deployed to test-trigger-is July 11, 2025 13:51 — with GitHub Actions Inactive

update test and make it better

6e9988f

shreyas-goenka marked this pull request as ready for review July 11, 2025 13:59

shreyas-goenka changed the title ~~Validate required bundle fields~~ Warn if required bundle fields are not set Jul 11, 2025

shreyas-goenka temporarily deployed to test-trigger-is July 11, 2025 13:59 — with GitHub Actions Inactive

shreyas-goenka changed the title ~~Warn if required bundle fields are not set~~ Add warning for when required bundle fields are not set Jul 11, 2025

andrewnester reviewed Jul 17, 2025

View reviewed changes

shreyas-goenka requested a review from andrewnester July 18, 2025 11:07

andrewnester approved these changes Jul 18, 2025

View reviewed changes

shreyas-goenka added 2 commits July 18, 2025 14:52

Merge remote-tracking branch 'origin' into validate-on-required

7031ce7

update golden files

8a7e423

shreyas-goenka temporarily deployed to test-trigger-is July 18, 2025 12:53 — with GitHub Actions Inactive

shreyas-goenka added this pull request to the merge queue Jul 18, 2025

Merged via the queue into main with commit b7c67d4 Jul 18, 2025
13 checks passed

shreyas-goenka deleted the validate-on-required branch July 18, 2025 14:53

shreyas-goenka mentioned this pull request Jul 25, 2025

Add warning when an invalid value is specified for enum field #3050

Merged

denik reviewed Jul 29, 2025

View reviewed changes

github-merge-queue bot pushed a commit that referenced this pull request Aug 8, 2025

Add warning when an invalid value is specified for enum field (#3050)

d13598f

## Why This PR mirrors the validation we added for required fields not being set: #3044 ## Tests A new acceptance test.

Add warning for when required bundle fields are not set #3044

Add warning for when required bundle fields are not set #3044

Uh oh!

Conversation

shreyas-goenka commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Why

Tests

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shreyas-goenka commented Jul 18, 2025

Uh oh!

Uh oh!

denik Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

shreyas-goenka commented Jun 12, 2025 •

edited

Loading

denik Jul 29, 2025 •

edited

Loading