Config remote sync command #4289
Open
+3,249
−12
Conversation
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
ilyakuz-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Collaborator
|
Commit: ab7b1f7
28 interesting tests: 7 KNOWN, 5 SKIP, 5 FAIL, 4 BUG, 4 RECOVERED, 3 flaky
Top 50 slowest tests (at least 2 minutes):
|
ilyakuz-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
ilyakuz-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
ilyakuz-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
ilyakuz-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
ilyakuz-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Contributor
Author
|
@denik I added acceptance tests and addressed all comments |
ilyakuz-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
ilyakuz-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
ilyakuz-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
denik
reviewed
Jan 22, 2026
Contributor
denik
left a comment
denik
left a comment
There was a problem hiding this comment.
suggestions: add permissions to some of the test configs.
| } | ||
|
|
||
| if save { | ||
| if err := configsync.SaveFiles(ctx, b, files); err != nil { |
Contributor
There was a problem hiding this comment.
It looks like --save will write the updated files and will also print a diff. Is that intentional? I'd expect that you either need a diff or in-place updates but not both.
Contributor
Author
There was a problem hiding this comment.
It is pretty useful, I can avoid reading the affected files in the UI when I need to pass this diff to the accept/reject UI
Also quite useful for debugging
cmd/bundle/config_remote_sync.go
Outdated
| cmd.Flags().BoolVar(&save, "save", false, "Write updated config files to disk") | ||
|
|
||
| cmd.RunE = func(cmd *cobra.Command, args []string) error { | ||
| b, _, err := utils.ProcessBundleRet(cmd, utils.ProcessOptions{}) |
Contributor
There was a problem hiding this comment.
I see that plan command does this:
cmd.RunE = func(cmd *cobra.Command, args []string) error {
opts := utils.ProcessOptions{
AlwaysPull: true,
FastValidate: true,
Build: true,
PreDeployChecks: true,
}
You need Build because it updates the configuration and hence the plan. You probably also want AlwaysPull as well.
Contributor
Author
There was a problem hiding this comment.
Yes, that was actually causing a bug with drift in globs, which I was planning to investigate later. Thanks!
|
|
||
| title "Updated configuration" | ||
| echo | ||
| cat databricks.yml |
Contributor
There was a problem hiding this comment.
can we record diff old_databricks.yml new_databricks.yml here instead of full config? It's hard to see if there are any subtle changes there.
Contributor
Author
There was a problem hiding this comment.
Let's try that, good idea
| my_job: | ||
| email_notifications: | ||
| on_success: | ||
| - success@example.com |
Contributor
There was a problem hiding this comment.
IMO a few of these tests can be combined into one, e.g. formatting_preserved, job_email_notifications
Contributor
Author
There was a problem hiding this comment.
I merged resource-specific tests together, let's keep more generic tests like target_override separate to be able to localise the issue quickly and extend them easily if needed
| @@ -0,0 +1,8 @@ | |||
| Local = true | |||
| Cloud = false | |||
Contributor
There was a problem hiding this comment.
Running some of these tests on Cloud would actually be useful here since extra fields backends adds can play a role.
Contributor
Author
There was a problem hiding this comment.
Makes sense, I enabled Cloud for resource-specific tests
| for task in r["tasks"]: | ||
| if task["task_key"] == "process": | ||
| task["new_cluster"]["num_workers"] = 5 | ||
| task["timeout_seconds"] = 3600 |
Contributor
There was a problem hiding this comment.
suggestion: also delete a task in the middle to see how it copes.
Contributor
Author
There was a problem hiding this comment.
Updated and also renamed keys to make test more straightforward
denik
reviewed
Jan 22, 2026
| jsonPointers = append(jsonPointers, targetPrefix+jsonPointer) | ||
| } | ||
|
|
||
| hasConfigValue := changeDesc.Old != nil || changeDesc.New != nil |
Contributor
There was a problem hiding this comment.
Do you have a test where a field is removed in remote and in config (New)?
Contributor
Author
There was a problem hiding this comment.
Added test that covers config edits, acceptance/bundle/config-remote-sync/config_edits
ilyakuz-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Contributor
Author
Good suggestions, I didn't test for permissions yet; there is a bug with them. I prepared test to capture the behavior, but let's fix it in a separate PR to unblock this one |
ilyakuz-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
New experimental command
databricks bundle config-remote-sync. The command fetches the latest remote changes of resources and compares them to the deployed state. With the--saveflag, it also writes changes back to YAML files.The command leverages the diffing algorithm of the
bundle plancommand. Also note that new dependency is added to do yaml patching that preserves commentsThis is a first PR, I will continue fixing all limitations in the next PRs
Current limitations:
bundle plantasks[task_key='my_task']need to be tested for cases when the object or parent doesn't existNo changelog entries are needed as the command is intended to be private for now, and we don't want to encourage users to use it due to the unstable API
Why
This command serves as the backend for the new visual authoring feature in DABs in the Workspace.
User edits job or pipeline in the Workspace UI, then clicks the "Sync" button, and then the new command applies the diff to the source configuration files. Then the user may accept or reject these changes in the editor
Tests
Currently, only unit tests to speed up devloop, but once I have full functionality, I'll add proper acceptance test coverage
Acceptance tests are failing because of the change in formatting (even though the command is hidden)