Skip to content

Commit 048a854

Browse files
authored
Remove dev prefix in config-remote-sync when resource is renamed (#4919)
## Changes Remove current dev-prefix when config-remote-sync applies resource name changes to avoid duplicating it in the config ## Why If we don't do that users always need to remove dev prefix manually even for simple renames like `[prefix] foo_1 => [prefix] foo_2`. If they don't do it that leads to job names like this: `[dev username] [dev username] my_job` ## Tests <!-- How have you tested the changes? --> Update 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 a072b6c commit 048a854

File tree

8 files changed

+128
-7
lines changed

8 files changed

+128
-7
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ bundle:
44
resources:
55
jobs:
66
my_job:
7+
name: test-job-$UNIQUE_NAME
78
email_notifications:
89
on_success:
910
- success@example.com

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ Resource: resources.jobs.my_job
1414
email_notifications.on_failure: add
1515
job_clusters[job_cluster_key='test_cluster']: remove
1616
job_clusters[job_cluster_key='test_cluster_renamed']: add
17+
name: replace
1718
parameters[name='region']: add
1819
tags['team']: add
1920
tasks[task_key='inline_cluster_task'].new_cluster.num_workers: replace
@@ -29,15 +30,20 @@ Resource: resources.jobs.my_job
2930
>>> diff.py databricks.yml.backup databricks.yml
3031
--- databricks.yml.backup
3132
+++ databricks.yml
32-
@@ -8,4 +8,7 @@
33+
@@ -5,8 +5,11 @@
34+
jobs:
35+
my_job:
36+
- name: test-job-[UNIQUE_NAME]
37+
+ name: renamed-job-[UNIQUE_NAME]
38+
email_notifications:
3339
on_success:
3440
- success@example.com
3541
+ no_alert_for_skipped_runs: true
3642
+ on_failure:
3743
+ - failure@example.com
3844
parameters:
3945
- name: catalog
40-
@@ -13,8 +16,11 @@
46+
@@ -14,8 +17,11 @@
4147
- name: env
4248
default: dev
4349
+ - default: us-east-1
@@ -52,7 +58,7 @@ Resource: resources.jobs.my_job
5258
+ - samples.nyctaxi.trips
5359
environments:
5460
- environment_key: default
55-
@@ -24,14 +30,14 @@
61+
@@ -25,14 +31,14 @@
5662
- ./*.whl
5763
job_clusters:
5864
- - job_cluster_key: test_cluster
@@ -70,7 +76,7 @@ Resource: resources.jobs.my_job
7076
+ job_cluster_key: test_cluster_renamed
7177
- task_key: inline_cluster_task
7278
notebook_task:
73-
@@ -40,5 +46,7 @@
79+
@@ -41,5 +47,7 @@
7480
spark_version: [[DEFAULT_SPARK_VERSION]]
7581
node_type_id: [NODE_TYPE_ID]
7682
- num_workers: 1

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ job_id="$(read_id.py my_job)"
1717
title "Modify job fields remotely"
1818
echo
1919
edit_resource.py jobs $job_id <<EOF
20+
# Rename the job remotely; config-remote-sync should strip the dev prefix
21+
r["name"] = r["name"].replace("test-job-$UNIQUE_NAME", "renamed-job-$UNIQUE_NAME")
2022
r["email_notifications"]["on_failure"] = ["failure@example.com"]
2123
r["email_notifications"]["no_alert_for_skipped_runs"] = True
2224
r["parameters"].append({"name": "region", "default": "us-east-1"})

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ Detected changes in 1 resource(s):
1010
Resource: resources.pipelines.my_pipeline
1111
configuration['key2']: add
1212
environment.dependencies: replace
13+
name: replace
1314
notifications[0].alerts: replace
1415
notifications[0].email_recipients: replace
1516
root_path: add
@@ -23,8 +24,11 @@ Resource: resources.pipelines.my_pipeline
2324
>>> diff.py databricks.yml.backup databricks.yml
2425
--- databricks.yml.backup
2526
+++ databricks.yml
26-
@@ -7,18 +7,25 @@
27-
name: test-pipeline-[UNIQUE_NAME]
27+
@@ -5,20 +5,27 @@
28+
pipelines:
29+
my_pipeline:
30+
- name: test-pipeline-[UNIQUE_NAME]
31+
+ name: renamed-pipeline-[UNIQUE_NAME]
2832
catalog: main
2933
- schema: default
3034
+ schema: prod

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ pipeline_id="$(read_id.py my_pipeline)"
1313

1414
title "Modify pipeline fields remotely"
1515
edit_resource.py pipelines $pipeline_id <<EOF
16+
# Rename the pipeline remotely; config-remote-sync should strip the dev prefix
17+
r["name"] = r["name"].replace("test-pipeline-$UNIQUE_NAME", "renamed-pipeline-$UNIQUE_NAME")
1618
r["configuration"]["key2"] = "value2"
1719
r["notifications"][0]["email_recipients"].append("admin@example.com")
1820
r["notifications"][0]["alerts"].append("on-update-failure")

bundle/configsync/defaults.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,3 +184,33 @@ func resetValueIfNeeded(path string, value any) any {
184184
}
185185
return value
186186
}
187+
188+
// prefixedNameFields lists resource name field patterns where the name prefix
189+
// (e.g. "[dev user] ") is applied during deployment and should be stripped
190+
// when syncing remote changes back to config.
191+
var prefixedNameFields = []string{
192+
"resources.jobs.*.name",
193+
"resources.pipelines.*.name",
194+
"resources.dashboards.*.display_name",
195+
}
196+
197+
// stripNamePrefix strips the configured name prefix from name field values
198+
// so that the raw (unprefixed) name is written back to the config YAML.
199+
func stripNamePrefix(path string, value any, prefix string) any {
200+
if prefix == "" {
201+
return value
202+
}
203+
204+
s, ok := value.(string)
205+
if !ok {
206+
return value
207+
}
208+
209+
for _, pattern := range prefixedNameFields {
210+
if matchPattern(pattern, path) {
211+
return strings.TrimPrefix(s, prefix)
212+
}
213+
}
214+
215+
return value
216+
}

bundle/configsync/diff.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,13 +153,15 @@ func DetectChanges(ctx context.Context, b *bundle.Bundle, engine engine.EngineTy
153153
continue
154154
}
155155

156-
change, err := convertChangeDesc(resourceKey+"."+path, changeDesc)
156+
fullPath := resourceKey + "." + path
157+
change, err := convertChangeDesc(fullPath, changeDesc)
157158
if err != nil {
158159
return nil, fmt.Errorf("failed to compute config change for path %s: %w", path, err)
159160
}
160161
if change.Operation == OperationSkip {
161162
continue
162163
}
164+
change.Value = stripNamePrefix(fullPath, change.Value, b.Config.Presets.NamePrefix)
163165
resourceChanges[path] = change
164166
}
165167
}

bundle/configsync/diff_test.go

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,80 @@ import (
66
"github.com/stretchr/testify/assert"
77
)
88

9+
func TestStripNamePrefix(t *testing.T) {
10+
tests := []struct {
11+
name string
12+
path string
13+
value any
14+
prefix string
15+
want any
16+
}{
17+
{
18+
name: "job name with normal prefix",
19+
path: "resources.jobs.my_job.name",
20+
value: "[dev user] my_job",
21+
prefix: "[dev user] ",
22+
want: "my_job",
23+
},
24+
{
25+
name: "pipeline name with normal prefix",
26+
path: "resources.pipelines.my_pipeline.name",
27+
value: "[dev user] my_pipeline",
28+
prefix: "[dev user] ",
29+
want: "my_pipeline",
30+
},
31+
{
32+
name: "dashboard display_name with prefix",
33+
path: "resources.dashboards.my_dash.display_name",
34+
value: "[dev user] my_dash",
35+
prefix: "[dev user] ",
36+
want: "my_dash",
37+
},
38+
{
39+
name: "name does not start with prefix",
40+
path: "resources.jobs.my_job.name",
41+
value: "my_job",
42+
prefix: "[dev user] ",
43+
want: "my_job",
44+
},
45+
{
46+
name: "empty prefix is noop",
47+
path: "resources.jobs.my_job.name",
48+
value: "[dev user] my_job",
49+
prefix: "",
50+
want: "[dev user] my_job",
51+
},
52+
{
53+
name: "non-name field is not stripped",
54+
path: "resources.jobs.my_job.description",
55+
value: "[dev user] some description",
56+
prefix: "[dev user] ",
57+
want: "[dev user] some description",
58+
},
59+
{
60+
name: "non-string value is unchanged",
61+
path: "resources.jobs.my_job.name",
62+
value: 42,
63+
prefix: "[dev user] ",
64+
want: 42,
65+
},
66+
{
67+
name: "nil value is unchanged",
68+
path: "resources.jobs.my_job.name",
69+
value: nil,
70+
prefix: "[dev user] ",
71+
want: nil,
72+
},
73+
}
74+
75+
for _, tt := range tests {
76+
t.Run(tt.name, func(t *testing.T) {
77+
got := stripNamePrefix(tt.path, tt.value, tt.prefix)
78+
assert.Equal(t, tt.want, got)
79+
})
80+
}
81+
}
82+
983
func TestMatchPattern(t *testing.T) {
1084
tests := []struct {
1185
name string

0 commit comments

Comments
 (0)