Skip to content

Commit ccb2482

Browse files
shreyas-goenkashreyas-goenka
committed
Suppress /Workspace prefix diff for experiments via OverrideChangeDesc
Use OverrideChangeDesc to normalize the /Workspace prefix on experiment names instead of stripping it in apply_presets, matching the Terraform provider's experimentNameSuppressDiff behavior. Add acceptance test coverage for both /Workspace/Users/... and /Users/... path forms. Co-authored-by: Isaac
1 parent 9d4c607 commit ccb2482

File tree

6 files changed

+76
-3
lines changed

6 files changed

+76
-3
lines changed

‎acceptance/bundle/deployment/bind/experiment/out.plan1.direct.json‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
{
2+
"plan": {
3+
"resources.experiments.experiment1": {
4+
"action": "skip",
5+
"name_change": {
6+
"action": "skip",
7+
"reason": "alias",
8+
"old": "/Workspace/Users/[USERNAME]/test-experiment[UNIQUE_NAME]",
9+
"new": "/Workspace/Users/[USERNAME]/test-experiment[UNIQUE_NAME]",
10+
"remote": "/Users/[USERNAME]/test-experiment[UNIQUE_NAME]"
11+
}
12+
}
13+
}
14+
}

‎acceptance/bundle/deployment/bind/experiment/out.plan1.terraform.json‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"plan": {
3+
"resources.experiments.experiment1": {
4+
"action": "skip",
5+
"name_change": null
6+
}
7+
}
8+
}

‎acceptance/bundle/deployment/bind/experiment/out.plan2.direct.json‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
{
2+
"plan": {
3+
"resources.experiments.experiment1": {
4+
"action": "skip",
5+
"name_change": {
6+
"action": "skip",
7+
"reason": "remote_already_set",
8+
"old": "/Workspace/Users/[USERNAME]/test-experiment[UNIQUE_NAME]",
9+
"new": "/Users/[USERNAME]/test-experiment[UNIQUE_NAME]",
10+
"remote": "/Users/[USERNAME]/test-experiment[UNIQUE_NAME]"
11+
}
12+
}
13+
}
14+
}

‎acceptance/bundle/deployment/bind/experiment/out.plan2.terraform.json‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"plan": {
3+
"resources.experiments.experiment1": {
4+
"action": "skip",
5+
"name_change": null
6+
}
7+
}
8+
}

‎acceptance/bundle/deployment/bind/experiment/script‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,20 @@ trace $CLI bundle deployment bind experiment1 ${EXPERIMENT_ID} --auto-approve
1717

1818
trace $CLI bundle deploy --force-lock --auto-approve
1919

20+
# Plan should show no changes with /Workspace/Users/... path in config.
21+
# Use --output json to verify the change details including skip reason.
22+
# Write per-engine output since direct includes change details that terraform doesn't.
23+
# Filter to only the name change (tags/artifact_location may differ between local and cloud).
24+
$CLI bundle plan --output json | jq '{plan: .plan | map_values({action, name_change: (.changes.name // null)})}' > out.plan1.$DATABRICKS_BUNDLE_ENGINE.json
25+
2026
trace $CLI experiments get-experiment ${EXPERIMENT_ID} | jq '{name: .experiment.name, lifecycle_stage: .experiment.lifecycle_stage}'
2127

28+
# Now change the config to use /Users/... path (without /Workspace prefix).
29+
# Plan should still show no changes because the diff is suppressed.
30+
EXPERIMENT_NAME="//Users/${CURRENT_USER_NAME}/test-experiment$UNIQUE_NAME"
31+
envsubst < databricks.yml.tmpl > databricks.yml
32+
$CLI bundle plan --output json | jq '{plan: .plan | map_values({action, name_change: (.changes.name // null)})}' > out.plan2.$DATABRICKS_BUNDLE_ENGINE.json
33+
2234
trace $CLI bundle deployment unbind experiment1
2335

2436
trace $CLI bundle destroy --auto-approve

‎bundle/direct/dresources/experiment.go‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,12 @@ func (r *ResourceExperiment) DoDelete(ctx context.Context, id string) error {
8080
// so remote returns "/Users/..." while the config has "/Workspace/Users/...".
8181
//
8282
// This matches the Terraform provider's experimentNameSuppressDiff behavior.
83-
// https://github.com/databricks/terraform-provider-databricks/blob/v1.65.1/mlflow/resource_mlflow_experiment.go#L35
83+
// https://github.com/databricks/terraform-provider-databricks/blob/8945a7b2328659b1fc976d04e32457305860131f/mlflow/resource_mlflow_experiment.go#L13
8484
func (*ResourceExperiment) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, change *ChangeDesc, _ *ml.Experiment) error {
85+
if change.Action == deployplan.Skip {
86+
return nil
87+
}
88+
8589
if path.String() != "name" {
8690
return nil
8791
}
@@ -92,11 +96,24 @@ func (*ResourceExperiment) OverrideChangeDesc(_ context.Context, path *structpat
9296
return nil
9397
}
9498

95-
normalizedNew := strings.TrimSuffix(strings.TrimPrefix(newStr, "/Workspace"), "/")
96-
normalizedRemote := strings.TrimSuffix(strings.TrimPrefix(remoteStr, "/Workspace"), "/")
99+
// Normalize by stripping the /Workspace/ prefix (keeping the trailing slash
100+
// to avoid false matches like "/WorkspaceExtra/...").
101+
normalizedNew := stripWorkspacePrefix(newStr)
102+
normalizedRemote := stripWorkspacePrefix(remoteStr)
97103
if normalizedNew == normalizedRemote {
98104
change.Action = deployplan.Skip
105+
change.Reason = deployplan.ReasonAlias
99106
}
100107

101108
return nil
102109
}
110+
111+
// stripWorkspacePrefix removes the "/Workspace" portion from paths like
112+
// "/Workspace/Users/..." while preserving the leading slash. Uses "/Workspace/"
113+
// with trailing slash to avoid false matches on paths like "/WorkspaceExtra/...".
114+
func stripWorkspacePrefix(s string) string {
115+
if strings.HasPrefix(s, "/Workspace/") {
116+
return s[len("/Workspace"):]
117+
}
118+
return s
119+
}

0 commit comments

Comments
 (0)