Skip to content

Commit 8b0dda2

Browse files
authored
Fix config sync for perfomance mode and timeouts (#4908)
## Changes Use the same logic for server-side defaults as in resources.yml logic. I have a PR that updates `config-remote-sync` to use `resources.yml`, but it's not yet ready to merge #4677 ## Why <!-- Why are these changes needed? Provide the context that the reviewer might be missing. For example, were there any decisions behind the change that are not reflected in the code itself? --> Changes in performance mode were incorrectly ignored previously ## Tests <!-- How have you tested the changes? --> <!-- 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 aba09af commit 8b0dda2

3 files changed

Lines changed: 15 additions & 5 deletions

File tree

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ Resource: resources.jobs.my_job
2626
email_notifications.on_failure[0]: replace
2727
max_concurrent_runs: replace
2828
tags['env']: remove
29+
timeout_seconds: remove
2930

3031

3132

@@ -41,15 +42,16 @@ Resource: resources.jobs.my_job
4142
+ - remote-failure@example.com
4243
parameters:
4344
- name: catalog
44-
@@ -35,7 +35,6 @@
45+
@@ -35,8 +35,6 @@
4546
unit: DAYS
4647
tags:
4748
- env: config-production
4849
team: data-team
4950
- max_concurrent_runs: 3
51+
- timeout_seconds: 3600
5052
+ max_concurrent_runs: 5
51-
timeout_seconds: 3600
5253
environments:
54+
- environment_key: default
5355

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

bundle/configsync/defaults.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,20 @@ var serverSideDefaults = map[string]any{
9797
"resources.pipelines.*.continuous": false,
9898
}
9999

100-
func shouldSkipField(path string, value any) bool {
100+
// shouldSkipField checks if a field should be skipped in change detection.
101+
// When hasConfigValue is true (field is set in config or saved state), only
102+
// "always skip" fields are skipped. Backend defaults are only skipped when the
103+
// field is not in config/state, matching the behavior of shouldSkipBackendDefault
104+
// in the direct deployment engine.
105+
func shouldSkipField(path string, value any, hasConfigValue bool) bool {
101106
for pattern, expected := range serverSideDefaults {
102107
if matchPattern(pattern, path) {
103108
if _, ok := expected.(skipAlways); ok {
104109
return true
105110
}
111+
if hasConfigValue {
112+
return false
113+
}
106114
if _, ok := expected.(skipIfZeroOrNil); ok {
107115
return value == nil || value == int64(0)
108116
}

bundle/configsync/diff.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func filterEntityDefaults(basePath string, value any) any {
7070
for key, val := range m {
7171
fieldPath := basePath + "." + key
7272

73-
if shouldSkipField(fieldPath, val) {
73+
if shouldSkipField(fieldPath, val, false) {
7474
continue
7575
}
7676

@@ -91,7 +91,7 @@ func convertChangeDesc(path string, cd *deployplan.ChangeDesc) (*ConfigChangeDes
9191
return nil, fmt.Errorf("failed to normalize remote value: %w", err)
9292
}
9393

94-
if shouldSkipField(path, normalizedValue) {
94+
if shouldSkipField(path, normalizedValue, hasConfigValue) {
9595
return &ConfigChangeDesc{
9696
Operation: OperationSkip,
9797
}, nil

0 commit comments

Comments
 (0)