Fix yaml config-remote-sync snapshot creation when bundle is deployed without any resources#4889
Open
ilyakuz-db wants to merge 21 commits intomainfrom
Open
Fix yaml config-remote-sync snapshot creation when bundle is deployed without any resources#4889ilyakuz-db wants to merge 21 commits intomainfrom
ilyakuz-db wants to merge 21 commits intomainfrom
Conversation
ilyakuz-db
changed the title
ilyakuz-db
had a problem deploying
to
test-trigger-is
GitHub Actions
Failure
#4866) ## Summary - Fix `bundle deployment migrate` for bundles with secret scopes to prevent phantom drift on `secret_scopes.*.permissions` after migration from Terraform to Direct engine. - Handle `databricks_secret_acl` in `ParseResourcesState`: multiple ACL resources per scope are mapped to a single `.permissions` state entry with the scope name as ID, similar to how `databricks_permissions` and `databricks_grants` are handled. - Expose resources.secret_scopes.foo.permissions as a separate entry in terraform JSON plan as well to match direct engine. ## Test plan - Re-enable the previously excluded secret scope migration acceptance tests. - New invariant test config for secret scope with ACLs.
## Summary - Pin `flit_core` from `>=3.11,<4` to `==3.12.0` in `python/pyproject.toml` - `uv lock` doesn't lock build-system dependencies, so an unpinned range allows non-reproducible builds This pull request was AI-assisted by Isaac.
ilyakuz-db
requested review from
andrewnester,
anton-107,
denik,
pietern,
shreyas-goenka and
simonfaltum
as code owners
Suggested reviewersBased on git history of the changed files, these people are best suited to review:
Confidence: low Eligible reviewersBased on CODEOWNERS, these people or teams could also review: @andrewnester, @anton-107, @pietern, @shreyas-goenka, @simonfaltum Suggestions based on git history of 5 changed files (1 scored). See CODEOWNERS for path-specific ownership rules. |
denik
approved these changes
Apr 2, 2026
| } | ||
|
|
||
| func (m *uploadStateForYamlSync) convertState(ctx context.Context, b *bundle.Bundle, snapshotPath string) (diags diag.Diagnostics) { | ||
| func (m *uploadStateForYamlSync) convertState(ctx context.Context, b *bundle.Bundle, snapshotPath string) (bool, diag.Diagnostics) { |
Contributor
There was a problem hiding this comment.
unrelated question, since it was added previously, but do you need to collect diagnostics? Why not just logs errors and warnings directly? (or logdiag.LogError/logdiag.LogError if you need diagnostics formatting).
| Ignore = [".databricks", "databricks.yml"] | ||
|
|
||
| [Env] | ||
| DATABRICKS_BUNDLE_ENABLE_EXPERIMENTAL_YAML_SYNC = "true" |
Contributor
There was a problem hiding this comment.
I think we should have a test with empty bundle already somewhere (if not, we should have it). We can then use EnvMatrix to run it both with DATABRICKS_BUNDLE_ENABLE_EXPERIMENTAL_YAML_SYNC and without.
Perhaps there are more tests where we need to test this variant?
Contributor
Author
There was a problem hiding this comment.
Added matrix to the acceptance/bundle/deploy/empty-bundle/test.toml
…les (#4896) ## Summary - Revert `github.com/fatih/color` from v1.19.0 back to v1.18.0 because v1.19.0 is too recent (reverts #4804) - Add `cooldown: default-days: 14` to both gomod dependabot ecosystems (`/` and `/tools`) to avoid upgrading dependencies too soon after release This pull request was AI-assisted by Isaac.
## Summary - Add `.github/actions/setup-jfrog` composite action that exchanges a GitHub OIDC token for a JFrog access token and configures Go (GOPROXY) and Python (UV_INDEX_URL + PIP_INDEX_URL) proxies using URL-embedded credentials - Replace the `jfrog/setup-jfrog-cli` third-party action in `setup-build-environment` with the new action, removing an external dependency - Remove the macOS exclusion — all runners now go through JFrog - Fail early with a clear error if `id-token: write` permission is missing - Verify token with an authenticated API call after exchange ## Test plan - [x] Verify OIDC token exchange works on Linux and macOS runners - [x] Verify Go and Python proxy URLs are configured correctly This pull request was AI-assisted by Isaac.
#4898) ## Summary - Use the new `setup-jfrog` composite action to proxy Go module downloads through JFrog Artifactory - Switch from `databricks-deco-testing-runner-group` to `databricks-large-runner-group` - Trigger workflow on changes to the `setup-jfrog` action ## Test plan - [x] Verify the release-snapshot workflow succeeds on this PR This pull request was AI-assisted by Isaac.
## Summary - Skip integration test triggering on PRs and pushes to main while the integration test infrastructure is not working - The required "Integration Tests" check still passes (marked as skipped) so PRs and merge queue are not blocked - Removed dependency on `test-trigger-is` environment and app tokens since no external workflows are triggered - Revert this PR when integration tests are working again ## Test plan - [x] Verify the "Integration Tests" check shows as passed with skip message on a PR - [x] Verify merge queue auto-approves correctly This pull request was AI-assisted by Isaac.
## Changes
When all EnvMatrix values are empty lists (e.g.,
`DATABRICKS_BUNDLE_ENGINE = []`), run the test directly without creating
a `#00` dummy subtest.
Before:
```
--- PASS: TestAccept/bundle/foo (0.00s)
--- PASS: TestAccept/bundle/foo/#00 (1.23s)
```
After:
```
--- PASS: TestAccept/bundle/foo (1.23s)
```
This allows tests to opt out of the default engine matrix by setting
`DATABRICKS_BUNDLE_ENGINE = []` without creating a confusingly-named
subtest.
## Tests
Added selftests:
- `selftest/envmatrix_empty` — verifies no subtest is created when all
EnvMatrix values are empty
- `selftest/envmatrix_mixed` — verifies empty vars are dropped while
non-empty vars still create subtests
## Changes - Add `.cursor/cli.json` to `.gitignore` so local Cursor CLI configuration is not tracked. ## Why Similar to `.claude/settings.local.json` which is already ignored, `.cursor/cli.json` is a local editor config file that should not be committed.
## Summary - Consolidate `.goreleaser-unix.yaml` and `.goreleaser-windows.yaml` into a single `.goreleaser.yaml` - Remove Docker build/push from goreleaser (to be handled separately) - Sign Windows binaries using jsign on Linux, replacing azuresigntool which required a Windows runner - Add `release-build.yml` workflow that builds all platforms and the Python wheel in parallel ## Test plan - [x] Verify `release-build` workflow succeeds on push to branch - [x] Verify Windows binary signatures in CI logs This pull request was AI-assisted by Isaac.
## Why The `suggest-reviewers` GitHub Action had two issues: 1. When a single person was the sole contributor to all changed files, confidence was reported as "low" instead of "high". This happened because `compute_confidence` treated `len(ss) < 2` as insufficient data, when it actually means there's one clear expert. 2. Output files (`out.*`, `output.txt`) were scored `0.0` and completely excluded from analysis. Reviewers should still verify output changes make sense, so these files deserve a small (but non-zero) contribution to scoring. ## Changes **Before:** Sole-author PRs always got "low" confidence. Output files were invisible to the scoring algorithm. PRs with exactly 2 contributors also always got "low" confidence (the `>= 3` guards prevented any comparison). **Now:** - `len(ss) == 1` (sole contributor) returns "high" confidence - `len(ss) == 2` compares top vs second using the same 2x/1.5x thresholds - Output files get weight `0.01 / total_files` instead of `0.0`, contributing signal without dominating scores - Removed dead `if weight == 0.0` guard that could never trigger after the weight change ## Test plan - Verified against PR #4857 (4 files, 2 output, pietern sole author): would now produce "high" instead of "low" - Logic review of all `compute_confidence` branches for correctness This pull request was AI-assisted by Isaac.
## Why `auth token` always outputs JSON, ignoring the `--output` flag. If you want to pipe just the token string into another tool, you have to parse JSON. That's unnecessary friction for a common scripting pattern. ## Changes `auth token` now respects `--output text` when explicitly set, outputting just the access token string (with a trailing newline) suitable for piping. JSON remains the default for backward compatibility. Only honors the explicit `--output text` flag, not implicit text mode (e.g. from `DATABRICKS_OUTPUT_FORMAT`), so existing scripts that parse JSON output won't break. Also fixes discarded write errors (`_, _` replaced with proper error returns). ## Test plan - [x] Unit tests for default (JSON), explicit `--output json`, and `--output text` modes - [x] Existing `cmd/auth` tests pass
Co-authored-by: Isaac
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
UploadStateForYamlSync now handles the case where the terraform state file doesn't exist. When ParseResourcesState returns nil (no state), the mutator skips snapshot creation and returns successfully. Errors in snapshot creation/upload are now warnings instead of fatal errors.
Why
Bug:
First deploy of an empty bundle with yaml sync enabled and terraform engine failed with open .../terraform.tfstate: no such file or directory. Terraform skips apply when there are no resources, so the state file is never created. The mutator tried to read it unconditionally.
Tests
Added acceptance/bundle/config-remote-sync/empty_deploy — deploys an empty bundle with yaml sync + terraform engine.