Skip to content

Commit f1b068c

Browse files
authored
Use normalized short name for tag value in development mode (#821)
## Changes The jobs backend propagates job tags to the underlying cloud provider's resources. As such, they need to match the constraints a cloud provider places on tag values. The display name can contain anything. With this change, we modify the tag value to equal the short name as used in the name prefix. Additionally, we leverage tag normalization as introduced in #819 to make sure characters that aren't accepted are removed before using the value as a tag value. This is a new stab at #810 and should completely eliminate this class of problems. ## Tests Tests pass.
1 parent 775251d commit f1b068c

File tree

5 files changed

+83
-4
lines changed

5 files changed

+83
-4
lines changed

bundle/bundle.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/databricks/cli/libs/git"
2020
"github.com/databricks/cli/libs/locker"
2121
"github.com/databricks/cli/libs/log"
22+
"github.com/databricks/cli/libs/tags"
2223
"github.com/databricks/cli/libs/terraform"
2324
"github.com/databricks/databricks-sdk-go"
2425
sdkconfig "github.com/databricks/databricks-sdk-go/config"
@@ -46,6 +47,10 @@ type Bundle struct {
4647
// if true, we skip approval checks for deploy, destroy resources and delete
4748
// files
4849
AutoApprove bool
50+
51+
// Tagging is used to normalize tag keys and values.
52+
// The implementation depends on the cloud being targeted.
53+
Tagging tags.Cloud
4954
}
5055

5156
func Load(ctx context.Context, path string) (*Bundle, error) {

bundle/config/mutator/populate_current_user.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/databricks/cli/bundle"
99
"github.com/databricks/cli/bundle/config"
10+
"github.com/databricks/cli/libs/tags"
1011
)
1112

1213
type populateCurrentUser struct{}
@@ -35,6 +36,10 @@ func (m *populateCurrentUser) Apply(ctx context.Context, b *bundle.Bundle) error
3536
ShortName: getShortUserName(me.UserName),
3637
User: me,
3738
}
39+
40+
// Configure tagging object now that we know we have a valid client.
41+
b.Tagging = tags.ForCloud(w.Config)
42+
3843
return nil
3944
}
4045

bundle/config/mutator/process_target_mode.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,18 @@ func (m *processTargetMode) Name() string {
3232
func transformDevelopmentMode(b *bundle.Bundle) error {
3333
r := b.Config.Resources
3434

35-
prefix := "[dev " + b.Config.Workspace.CurrentUser.ShortName + "] "
35+
shortName := b.Config.Workspace.CurrentUser.ShortName
36+
prefix := "[dev " + shortName + "] "
37+
38+
// Generate a normalized version of the short name that can be used as a tag value.
39+
tagValue := b.Tagging.NormalizeValue(shortName)
3640

3741
for i := range r.Jobs {
3842
r.Jobs[i].Name = prefix + r.Jobs[i].Name
3943
if r.Jobs[i].Tags == nil {
4044
r.Jobs[i].Tags = make(map[string]string)
4145
}
42-
r.Jobs[i].Tags["dev"] = b.Config.Workspace.CurrentUser.DisplayName
46+
r.Jobs[i].Tags["dev"] = tagValue
4347
if r.Jobs[i].MaxConcurrentRuns == 0 {
4448
r.Jobs[i].MaxConcurrentRuns = developmentConcurrentRuns
4549
}
@@ -74,7 +78,7 @@ func transformDevelopmentMode(b *bundle.Bundle) error {
7478
} else {
7579
r.Experiments[i].Name = dir + "/" + prefix + base
7680
}
77-
r.Experiments[i].Tags = append(r.Experiments[i].Tags, ml.ExperimentTag{Key: "dev", Value: b.Config.Workspace.CurrentUser.DisplayName})
81+
r.Experiments[i].Tags = append(r.Experiments[i].Tags, ml.ExperimentTag{Key: "dev", Value: tagValue})
7882
}
7983

8084
for i := range r.ModelServingEndpoints {

bundle/config/mutator/process_target_mode_test.go

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
"github.com/databricks/cli/bundle"
1010
"github.com/databricks/cli/bundle/config"
1111
"github.com/databricks/cli/bundle/config/resources"
12+
"github.com/databricks/cli/libs/tags"
13+
sdkconfig "github.com/databricks/databricks-sdk-go/config"
1214
"github.com/databricks/databricks-sdk-go/service/iam"
1315
"github.com/databricks/databricks-sdk-go/service/jobs"
1416
"github.com/databricks/databricks-sdk-go/service/ml"
@@ -59,6 +61,10 @@ func mockBundle(mode config.Mode) *bundle.Bundle {
5961
},
6062
},
6163
},
64+
// Use AWS implementation for testing.
65+
Tagging: tags.ForCloud(&sdkconfig.Config{
66+
Host: "https://company.cloud.databricks.com",
67+
}),
6268
}
6369
}
6470

@@ -68,14 +74,71 @@ func TestProcessTargetModeDevelopment(t *testing.T) {
6874
m := ProcessTargetMode()
6975
err := m.Apply(context.Background(), bundle)
7076
require.NoError(t, err)
77+
78+
// Job 1
7179
assert.Equal(t, "[dev lennart] job1", bundle.Config.Resources.Jobs["job1"].Name)
80+
assert.Equal(t, bundle.Config.Resources.Jobs["job1"].Tags["dev"], "lennart")
81+
82+
// Pipeline 1
7283
assert.Equal(t, "[dev lennart] pipeline1", bundle.Config.Resources.Pipelines["pipeline1"].Name)
84+
assert.True(t, bundle.Config.Resources.Pipelines["pipeline1"].PipelineSpec.Development)
85+
86+
// Experiment 1
7387
assert.Equal(t, "/Users/[email protected]/[dev lennart] experiment1", bundle.Config.Resources.Experiments["experiment1"].Name)
88+
assert.Contains(t, bundle.Config.Resources.Experiments["experiment1"].Experiment.Tags, ml.ExperimentTag{Key: "dev", Value: "lennart"})
89+
90+
// Experiment 2
7491
assert.Equal(t, "[dev lennart] experiment2", bundle.Config.Resources.Experiments["experiment2"].Name)
92+
assert.Contains(t, bundle.Config.Resources.Experiments["experiment2"].Experiment.Tags, ml.ExperimentTag{Key: "dev", Value: "lennart"})
93+
94+
// Model 1
7595
assert.Equal(t, "[dev lennart] model1", bundle.Config.Resources.Models["model1"].Name)
96+
97+
// Model serving endpoint 1
7698
assert.Equal(t, "dev_lennart_servingendpoint1", bundle.Config.Resources.ModelServingEndpoints["servingendpoint1"].Name)
7799
assert.Equal(t, "dev", bundle.Config.Resources.Experiments["experiment1"].Experiment.Tags[0].Key)
78-
assert.True(t, bundle.Config.Resources.Pipelines["pipeline1"].PipelineSpec.Development)
100+
}
101+
102+
func TestProcessTargetModeDevelopmentTagNormalizationForAws(t *testing.T) {
103+
bundle := mockBundle(config.Development)
104+
bundle.Tagging = tags.ForCloud(&sdkconfig.Config{
105+
Host: "https://dbc-XXXXXXXX-YYYY.cloud.databricks.com/",
106+
})
107+
108+
bundle.Config.Workspace.CurrentUser.ShortName = "Héllö wörld?!"
109+
err := ProcessTargetMode().Apply(context.Background(), bundle)
110+
require.NoError(t, err)
111+
112+
// Assert that tag normalization took place.
113+
assert.Equal(t, "Hello world__", bundle.Config.Resources.Jobs["job1"].Tags["dev"])
114+
}
115+
116+
func TestProcessTargetModeDevelopmentTagNormalizationForAzure(t *testing.T) {
117+
bundle := mockBundle(config.Development)
118+
bundle.Tagging = tags.ForCloud(&sdkconfig.Config{
119+
Host: "https://adb-xxx.y.azuredatabricks.net/",
120+
})
121+
122+
bundle.Config.Workspace.CurrentUser.ShortName = "Héllö wörld?!"
123+
err := ProcessTargetMode().Apply(context.Background(), bundle)
124+
require.NoError(t, err)
125+
126+
// Assert that tag normalization took place (Azure allows more characters than AWS).
127+
assert.Equal(t, "Héllö wörld?!", bundle.Config.Resources.Jobs["job1"].Tags["dev"])
128+
}
129+
130+
func TestProcessTargetModeDevelopmentTagNormalizationForGcp(t *testing.T) {
131+
bundle := mockBundle(config.Development)
132+
bundle.Tagging = tags.ForCloud(&sdkconfig.Config{
133+
Host: "https://123.4.gcp.databricks.com/",
134+
})
135+
136+
bundle.Config.Workspace.CurrentUser.ShortName = "Héllö wörld?!"
137+
err := ProcessTargetMode().Apply(context.Background(), bundle)
138+
require.NoError(t, err)
139+
140+
// Assert that tag normalization took place.
141+
assert.Equal(t, "Hello_world", bundle.Config.Resources.Jobs["job1"].Tags["dev"])
79142
}
80143

81144
func TestProcessTargetModeDefault(t *testing.T) {

libs/template/renderer_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/databricks/cli/bundle/config/mutator"
1818
"github.com/databricks/cli/bundle/phases"
1919
"github.com/databricks/cli/cmd/root"
20+
"github.com/databricks/cli/libs/tags"
2021
"github.com/databricks/databricks-sdk-go"
2122
workspaceConfig "github.com/databricks/databricks-sdk-go/config"
2223
"github.com/databricks/databricks-sdk-go/service/iam"
@@ -66,6 +67,7 @@ func assertBuiltinTemplateValid(t *testing.T, settings map[string]any, target st
6667

6768
// Apply initialize / validation mutators
6869
b.Config.Workspace.CurrentUser = &bundleConfig.User{User: cachedUser}
70+
b.Tagging = tags.ForCloud(w.Config)
6971
b.WorkspaceClient()
7072
b.Config.Bundle.Terraform = &bundleConfig.Terraform{
7173
ExecPath: "sh",

0 commit comments

Comments
 (0)