Uh oh!
There was an error while loading. Please reload this page.
direct: Pass changed fields into update mask for apps instead of wildcard - #4963
Conversation
pietern
left a comment
There was a problem hiding this comment.
@andrewnester If * doesn't work at all, then this can't regress anything.
Updating nested fields seems like a risk.
Uh oh!
There was an error while loading. Please reload this page.
| App: &config.App, | ||
| AppName: id, | ||
| UpdateMask: "*", | ||
| UpdateMask: updateMask, |
There was a problem hiding this comment.
Do we have any nested fields to worry about, eg under resources?
There was a problem hiding this comment.
Isn't collectUpdatePathsWithPrefix already return teh full nested path?
| >>> [CLI] apps get app-[UNIQUE_NAME] | ||
| { | ||
| "compute_size": "LARGE" | ||
| "compute_size": "MEDIUM" |
There was a problem hiding this comment.
I think it maybe did not work before with "*" or something because in the test itself we actually update to compute_size MEDIUM and that's what we expect
denik
commented
Apr 14, 2026
Are there cloud tests that this PR fixes? Could you mention them and maybe include the error we get for future reference. |
| // Deploy-only fields (source_code_path, config, | ||
| // git_source, lifecycle) are not part of apps.App and thus excluded from the request body. | ||
| if hasAppChanges(entry) { | ||
| fieldPaths := collectUpdatePathsWithPrefix(entry.Changes, "") |
There was a problem hiding this comment.
Why not just use a static list of fields here?
Paths in entry.Changes are really in a direct engine-specific format.
It's also against our general model: we update resource completely, not piecemeal.
There was a problem hiding this comment.
Why not just use a static list of fields here?
We already do similar pattern for Lakebase / postgres resources.
What if the list changes aka new fields are added in apps? How do we know about it and update the list?
There was a problem hiding this comment.
I think we should get rid of collectUpdatePathsWithPrefix entirely #4423 (comment)@pietern
There was a problem hiding this comment.
What if the list changes aka new fields are added in apps? How do we know about it and update the list?
We can add reflection-based unit test that validates that list is up to date. Similar to type_test.go that we already have.
There was a problem hiding this comment.
@denik we can do it separately together with changing Lakebase, I prefer this PR to focus on unblocking the tests first
Uh oh!
There was an error while loading. Please reload this page.
The updateMask/* issues is fixed in #4963
## Release v0.297.0 ### CLI * Auth commands now accept a profile name as a positional argument ([#4840](#4840)) * Add `auth logout` command for clearing cached OAuth tokens and optionally removing profiles ([#4613](#4613), [#4616](#4616), [#4647](#4647)) ### Bundles * Added support for lifecycle.started option for apps ([#4672](#4672)) * engine/direct: Fix permissions for resources.models ([#4941](#4941)) * Fix resource references not correctly resolved in apps config section ([#4964](#4964)) * Allow run_as for dashboards with embed_credentials set to false ([#4961](#4961)) * direct: Pass changed fields into update mask for apps instead of wildcard ([#4963](#4963)) * engine/direct: Fix deploy of configurations with dots in maps keys ([#4977](#4977))
## Summary Bump bundled Databricks CLI from v0.295.0 → v0.297.2. ## CLI changes since v0.295.0 ### v0.297.2 (2026-04-19) - **Fix `error downloading Terraform: unable to verify checksums signature: openpgp: key expired`** for `bundle deploy` — uses a hardcoded ArmoredPublicKey for the TF binary install ([#5019](databricks/cli#5019)). **Highly relevant** — extension users hitting this during `bundle deploy` flows are unblocked by the upgrade. ### v0.297.1 - Dep bump only: Go toolchain to 1.25.9. ### v0.297.0 - CLI: `auth` commands accept a profile name as a positional argument ([#4840](databricks/cli#4840)). - CLI: new `auth logout` command to clear cached OAuth tokens / remove profiles ([#4613](databricks/cli#4613), [#4616](databricks/cli#4616), [#4647](databricks/cli#4647)). - Bundles: `lifecycle.started` option for apps ([#4672](databricks/cli#4672)). - Bundles: resource references now resolved correctly in apps config ([#4964](databricks/cli#4964)). - Bundles: allow `run_as` for dashboards with `embed_credentials: false` ([#4961](databricks/cli#4961)). - Direct engine: permissions fix for `resources.models` ([#4941](databricks/cli#4941)); update mask fix for apps ([#4963](databricks/cli#4963)); dotted map keys fix ([#4977](databricks/cli#4977)). ### v0.296.0 - **Direct deployment engine for DABs is now in Public Preview** (`docs/direct.md`). Relevant to the extension's bundle deploy UX, though opt-in. - CLI: `auth` commands error when `--profile` and `--host` conflict ([#4841](databricks/cli#4841)). - CLI: `--force-refresh` flag on `databricks auth token` ([#4767](databricks/cli#4767)). - Bundles: `bundle deployment bind` now pulls remote state before modifying ([#4892](databricks/cli#4892)). - Bundles: dedupe grant entries ([#4801](databricks/cli#4801)). - Direct engine: several fixes (grants reordering, ALL_PRIVILEGES, secret scope recreation/permissions, bind/unbind for non-TF resources, removed principals). ## Relevance to the extension - **TF checksum fix (v0.297.2)** — unblocks `bundle deploy` for users hitting the expired-key error; this alone is a good reason to ship. - **Auth CLI surface changes** — extension shells out to `databricks auth …`; `--profile` + `--host` now conflict, and `auth logout` / positional profile are new. Worth a glance at auth-related code paths to make sure we aren't passing both. - **Direct deploy engine GA preview** — no action required today (opt-in), but worth tracking for future bundle deploy UX. ## Test plan - [ ] CI green (unit tests, integration tests pulling the pinned CLI). - [ ] Manually sanity-check `bundle deploy` from the extension against a simple bundle to confirm the TF download path works. - [ ] Verify `databricks auth login` / token refresh flows still work from the extension. This pull request and its description were written by Isaac.
…card (#4963) ## Changes Pass changed fields into update mask for apps instead of wildcard ## Why Apps Update API does not support "*" for update mask yet. ## Tests Existing (Cloud) tests pass ``` DATABRICKS_BUNDLE_ENGINE=direct go test ./acceptance -v -run TestAccept/bundle/run/app-with-job === RUN TestAccept ... --- PASS: TestAccept (32.48s) --- SKIP: TestAccept/bundle/run_as (0.00s) --- PASS: TestAccept/bundle/run/app-with-job (0.00s) --- PASS: TestAccept/bundle/run/app-with-job/DATABRICKS_BUNDLE_ENGINE=direct (294.22s) --- PASS: TestAccept/bundle/run/app-with-job/DATABRICKS_BUNDLE_ENGINE=terraform (299.11s) ```
## Release v0.297.0 ### CLI * Auth commands now accept a profile name as a positional argument ([#4840](#4840)) * Add `auth logout` command for clearing cached OAuth tokens and optionally removing profiles ([#4613](#4613), [#4616](#4616), [#4647](#4647)) ### Bundles * Added support for lifecycle.started option for apps ([#4672](#4672)) * engine/direct: Fix permissions for resources.models ([#4941](#4941)) * Fix resource references not correctly resolved in apps config section ([#4964](#4964)) * Allow run_as for dashboards with embed_credentials set to false ([#4961](#4961)) * direct: Pass changed fields into update mask for apps instead of wildcard ([#4963](#4963)) * engine/direct: Fix deploy of configurations with dots in maps keys ([#4977](#4977))
Changes
Pass changed fields into update mask for apps instead of wildcard
Why
Apps Update API does not support "*" for update mask yet.
Tests
Existing (Cloud) tests pass