Skip to content

Commit e7fa565

Browse files
authored
Fix yaml formatting in config-remote-sync (#4468)
## Changes Fix cases where the config-remote-sync command formats nodes in [Flow style](https://www.yaml.info/learn/flowstyle.html) ## Why It happens when a neighbor node is removed during sync, and new fields are then added to the same parent. Empty nodes can't be expressed in block style, therefore, yaml patcher falls back to using Flow formatting, which is then inherited by created child nodes TODO: A good alternative is to remove parent nodes during the Resolve phase, when all of their keys/items are removed, but this is a separate task and needs to be tested for edge cases. I'll clean up this fix if the parent removal is a valid option ## Tests Added acceptance tests <!-- If your PR needs to be included in the release notes for next release, add a separate entry in NEXT_CHANGELOG.md as part of your PR. -->
1 parent 810321a commit e7fa565

File tree

7 files changed

+117
-9
lines changed

7 files changed

+117
-9
lines changed

acceptance/bundle/config-remote-sync/formatting_preserved/databricks.yml.tmpl

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,8 @@ resources:
2323
tags:
2424
env: dev # environment tag
2525
team: data-eng
26+
27+
# Flow-style formatting (should be preserved)
28+
parameters:
29+
- {name: catalog, default: main}
30+
- {name: schema, default: dev}

acceptance/bundle/config-remote-sync/formatting_preserved/output.txt

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,17 @@ Resource: resources.jobs.my_job
3131
+ max_concurrent_runs: 5
3232
# Task configuration
3333
tasks:
34-
@@ -19,5 +17,4 @@
34+
@@ -19,10 +17,8 @@
3535
node_type_id: [NODE_TYPE_ID]
3636
num_workers: 1 # inline comment about workers
3737
-
3838
# Tags for categorization
3939
tags:
40+
env: dev # environment tag
41+
team: data-eng
42+
-
43+
# Flow-style formatting (should be preserved)
44+
parameters:
4045

4146
>>> [CLI] bundle destroy --auto-approve
4247
The following resources will be deleted:

acceptance/bundle/config-remote-sync/job_fields/out.test.toml

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

acceptance/bundle/config-remote-sync/job_fields/output.txt

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ Resource: resources.jobs.my_job
1414
email_notifications.on_failure: add
1515
parameters: replace
1616
tags['team']: add
17-
trigger.periodic.interval: replace
17+
trigger.pause_status: add
18+
trigger.periodic: remove
19+
trigger.table_update: add
1820

1921

2022

@@ -29,7 +31,7 @@ Resource: resources.jobs.my_job
2931
-
3032
resources:
3133
jobs:
32-
@@ -8,12 +7,17 @@
34+
@@ -8,13 +7,19 @@
3335
on_success:
3436
- success@example.com
3537
+ no_alert_for_skipped_runs: true
@@ -47,12 +49,16 @@ Resource: resources.jobs.my_job
4749
+ - default: us-east-1
4850
+ name: region
4951
trigger:
50-
periodic:
52+
- periodic:
5153
- interval: 1
52-
+ interval: 2
53-
unit: DAYS
54+
- unit: DAYS
55+
+ pause_status: UNPAUSED
56+
+ table_update:
57+
+ table_names:
58+
+ - samples.nyctaxi.trips
5459
environments:
55-
@@ -31,5 +35,6 @@
60+
- environment_key: default
61+
@@ -31,5 +36,6 @@
5662
node_type_id: [NODE_TYPE_ID]
5763
num_workers: 1
5864
-

acceptance/bundle/config-remote-sync/job_fields/script

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ edit_resource.py jobs $job_id <<EOF
1818
r["email_notifications"]["on_failure"] = ["failure@example.com"]
1919
r["email_notifications"]["no_alert_for_skipped_runs"] = True
2020
r["parameters"].append({"name": "region", "default": "us-east-1"})
21-
r["trigger"]["periodic"]["interval"] = 2
21+
r["trigger"] = {"pause_status": "UNPAUSED", "table_update": {"table_names": ["samples.nyctaxi.trips"]}}
22+
2223
if "tags" not in r:
2324
r["tags"] = {}
2425
r["tags"]["team"] = "data"

acceptance/bundle/config-remote-sync/job_fields/test.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
Cloud = true
2+
RequiresUnityCatalog = true
23

34
RecordRequests = false
45
Ignore = [".databricks", "dummy.whl", "databricks.yml", "databricks.yml.backup"]

bundle/configsync/patch.go

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package configsync
22

33
import (
4+
"bytes"
45
"context"
56
"errors"
67
"fmt"
@@ -14,12 +15,14 @@ import (
1415
"github.com/databricks/cli/libs/structs/structpath"
1516
"github.com/palantir/pkg/yamlpatch/gopkgv3yamlpatcher"
1617
"github.com/palantir/pkg/yamlpatch/yamlpatch"
18+
"go.yaml.in/yaml/v3"
1719
)
1820

1921
// ApplyChangesToYAML generates YAML files for the given field changes.
2022
func ApplyChangesToYAML(ctx context.Context, b *bundle.Bundle, fieldChanges []FieldChange) ([]FileChange, error) {
2123
originalFiles := make(map[string][]byte)
2224
modifiedFiles := make(map[string][]byte)
25+
fileFieldChanges := make(map[string][]FieldChange)
2326

2427
for _, fieldChange := range fieldChanges {
2528
filePath := fieldChange.FilePath
@@ -39,14 +42,22 @@ func ApplyChangesToYAML(ctx context.Context, b *bundle.Bundle, fieldChanges []Fi
3942
}
4043

4144
modifiedFiles[filePath] = modifiedContent
45+
fileFieldChanges[filePath] = append(fileFieldChanges[filePath], fieldChange)
4246
}
4347

4448
var result []FileChange
4549
for filePath := range modifiedFiles {
50+
// TODO: A good alternative approach is to remove parent nodes during the Resolve phase,
51+
// when all of their keys/items are removed, but this should be tested for edge cases.
52+
// In this case flow style will never appear because empty nodes are never serialized and we won't need clearAddedFlowStyle
53+
normalized, err := clearAddedFlowStyle(modifiedFiles[filePath], fileFieldChanges[filePath])
54+
if err != nil {
55+
return nil, fmt.Errorf("failed to normalize YAML style in %s: %w", filePath, err)
56+
}
4657
result = append(result, FileChange{
4758
Path: filePath,
4859
OriginalContent: string(originalFiles[filePath]),
49-
ModifiedContent: string(modifiedFiles[filePath]),
60+
ModifiedContent: string(normalized),
5061
})
5162
}
5263

@@ -262,3 +273,81 @@ func strPathToJSONPointer(pathStr string) (string, error) {
262273
}
263274
return "/" + strings.Join(parts, "/"), nil
264275
}
276+
277+
// clearAddedFlowStyle clears FlowStyle on YAML nodes along the changed field paths.
278+
// This prevents flow-style formatting (e.g. {key: value}) that yaml.v3 introduces
279+
// when empty mappings are serialized as "{}" during patch operations
280+
func clearAddedFlowStyle(content []byte, fieldChanges []FieldChange) ([]byte, error) {
281+
var doc yaml.Node
282+
if err := yaml.Unmarshal(content, &doc); err != nil {
283+
return content, nil
284+
}
285+
for _, fc := range fieldChanges {
286+
for _, candidate := range fc.FieldCandidates {
287+
clearFlowStyleAlongPath(&doc, candidate)
288+
}
289+
}
290+
var buf bytes.Buffer
291+
enc := yaml.NewEncoder(&buf)
292+
enc.SetIndent(2)
293+
if err := enc.Encode(&doc); err != nil {
294+
return nil, err
295+
}
296+
return buf.Bytes(), enc.Close()
297+
}
298+
299+
// clearFlowStyleAlongPath navigates the YAML tree along the given structpath,
300+
// clearing FlowStyle on every node from root to leaf (inclusive).
301+
func clearFlowStyleAlongPath(doc *yaml.Node, pathStr string) {
302+
node, err := structpath.ParsePath(pathStr)
303+
if err != nil {
304+
return
305+
}
306+
307+
current := doc
308+
if current.Kind == yaml.DocumentNode && len(current.Content) > 0 {
309+
current = current.Content[0]
310+
}
311+
312+
for _, n := range node.AsSlice() {
313+
current.Style &^= yaml.FlowStyle
314+
315+
if key, ok := n.StringKey(); ok {
316+
if current.Kind != yaml.MappingNode {
317+
return
318+
}
319+
found := false
320+
// current.Content: [key1, val1, key2, val2, ...]
321+
for i := 0; i+1 < len(current.Content); i += 2 {
322+
if current.Content[i].Value == key {
323+
current = current.Content[i+1]
324+
found = true
325+
break
326+
}
327+
}
328+
if !found {
329+
return
330+
}
331+
continue
332+
}
333+
334+
if idx, ok := n.Index(); ok {
335+
if current.Kind != yaml.SequenceNode || idx < 0 || idx >= len(current.Content) {
336+
return
337+
}
338+
current = current.Content[idx]
339+
continue
340+
}
341+
342+
return
343+
}
344+
345+
clearFlowStyleNodes(current)
346+
}
347+
348+
func clearFlowStyleNodes(node *yaml.Node) {
349+
node.Style &^= yaml.FlowStyle
350+
for _, child := range node.Content {
351+
clearFlowStyleNodes(child)
352+
}
353+
}

0 commit comments

Comments
 (0)