Skip to content

Commit c91fe38

Browse files
denikdenik
authored
Fix dynamic_version when sync root != bundle root (#2805)
## Changes - When patching wheels with dynamic version, use paths relative to sync root rather than dynamic root. - Add debug output for cases where we don't do patching (was helpful when investigating this issue). However, skip the whole thing if no replacements are to be done (user is not using dynamic_version). ## Why Previous implementation incorrectly assumed that those paths are relative to bundle root, but we did not have a test with bundle root != sync root, so it was not caught. Inspired by #2784 ## Tests New test where wheel is referenced outside of bundle root and is patched with dynamic version.
1 parent 8640068 commit c91fe38

File tree

9 files changed

+145
-9
lines changed

9 files changed

+145
-9
lines changed

‎NEXT_CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,6 @@
99
### CLI
1010

1111
### Bundles
12+
- Fix dynamic\_version when sync root != bundle root ([#2805](https://github.com/databricks/cli/pull/2805))
1213

1314
### API Changes

‎acceptance/bundle/artifacts/whl_prebuilt_outside/test.toml‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
Badness = "Expect this to work, but deploy fails: ... is not contained in sync root path"
2-
31
[[Repls]]
42
Old = '\\'
53
New = '/'

‎acceptance/bundle/artifacts/whl_prebuilt_outside_dynamic/other_dab/dist/lib/other_test_code-0.0.1-py3-none-any.whl‎

1.85 KB
Binary file not shown.

‎acceptance/bundle/artifacts/whl_prebuilt_outside_dynamic/other_dab/dist/my_test_code-0.0.1-py3-none-any.whl‎

1.79 KB
Binary file not shown.

‎acceptance/bundle/artifacts/whl_prebuilt_outside_dynamic/output.txt‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
2+
>>> errcode [CLI] bundle deploy
3+
Uploading this_dab/.databricks/bundle/default/patched_wheels/art1_my_test_code/my_test_code-0.0.1+[TIMESTAMP_NS]-py3-none-any.whl...
4+
Uploading this_dab/.databricks/bundle/default/patched_wheels/art2_other_test_code/other_test_code-0.0.1+[TIMESTAMP_NS]-py3-none-any.whl...
5+
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/python-wheel/default/files...
6+
Deploying resources...
7+
Updating deployment state...
8+
Deployment complete!
9+
10+
=== Expecting to find two patched wheels in current directory
11+
>>> find.py --expect 2 whl
12+
.databricks/bundle/default/patched_wheels/art1_my_test_code/my_test_code-0.0.1+[TIMESTAMP_NS]-py3-none-any.whl
13+
.databricks/bundle/default/patched_wheels/art2_other_test_code/other_test_code-0.0.1+[TIMESTAMP_NS]-py3-none-any.whl
14+
15+
=== Expecting 2 wheels in libraries section in /jobs/create
16+
>>> jq -s .[] | select(.path=="/api/2.2/jobs/create") | .body.tasks out.requests.txt
17+
[
18+
{
19+
"environment_key": "test_env",
20+
"python_wheel_task": {
21+
"entry_point": "run",
22+
"package_name": "my_test_code"
23+
},
24+
"task_key": "ServerlessTestTask"
25+
},
26+
{
27+
"existing_cluster_id": "0717-132531-5opeqon1",
28+
"libraries": [
29+
{
30+
"whl": "/Workspace/Users/[USERNAME]/.bundle/python-wheel/default/artifacts/.internal/my_test_code-0.0.1+[TIMESTAMP_NS]-py3-none-any.whl"
31+
},
32+
{
33+
"whl": "/Workspace/Users/[USERNAME]/.bundle/python-wheel/default/artifacts/.internal/other_test_code-0.0.1+[TIMESTAMP_NS]-py3-none-any.whl"
34+
}
35+
],
36+
"python_wheel_task": {
37+
"entry_point": "run",
38+
"package_name": "my_test_code"
39+
},
40+
"task_key": "TestTask"
41+
}
42+
]
43+
44+
>>> jq -s .[] | select(.path=="/api/2.2/jobs/create") | .body.environments out.requests.txt
45+
[
46+
{
47+
"environment_key": "test_env",
48+
"spec": {
49+
"client": "1",
50+
"dependencies": [
51+
"/Workspace/Users/[USERNAME]/.bundle/python-wheel/default/artifacts/.internal/my_test_code-0.0.1+[TIMESTAMP_NS]-py3-none-any.whl",
52+
"/Workspace/Users/[USERNAME]/.bundle/python-wheel/default/artifacts/.internal/other_test_code-0.0.1+[TIMESTAMP_NS]-py3-none-any.whl"
53+
]
54+
}
55+
}
56+
]
57+
58+
=== Expecting 2 wheels to be uploaded
59+
>>> jq .path
60+
"/api/2.0/workspace-files/import-file/Workspace/Users/[USERNAME]/.bundle/python-wheel/default/artifacts/.internal/my_test_code-0.0.1+[TIMESTAMP_NS]-py3-none-any.whl"
61+
"/api/2.0/workspace-files/import-file/Workspace/Users/[USERNAME]/.bundle/python-wheel/default/artifacts/.internal/other_test_code-0.0.1+[TIMESTAMP_NS]-py3-none-any.whl"

‎acceptance/bundle/artifacts/whl_prebuilt_outside_dynamic/script‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
cd this_dab
2+
3+
trace errcode $CLI bundle deploy
4+
5+
title "Expecting to find two patched wheels in current directory"
6+
trace find.py --expect 2 whl
7+
8+
cd ..
9+
10+
title "Expecting 2 wheels in libraries section in /jobs/create"
11+
trace jq -s '.[] | select(.path=="/api/2.2/jobs/create") | .body.tasks' out.requests.txt
12+
trace jq -s '.[] | select(.path=="/api/2.2/jobs/create") | .body.environments' out.requests.txt
13+
14+
title "Expecting 2 wheels to be uploaded"
15+
trace jq .path < out.requests.txt | grep import | grep whl | sort
16+
17+
rm out.requests.txt

‎acceptance/bundle/artifacts/whl_prebuilt_outside_dynamic/test.toml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
[[Repls]]
2+
Old = '\\'
3+
New = '/'

‎acceptance/bundle/artifacts/whl_prebuilt_outside_dynamic/this_dab/databricks.yml‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
bundle:
2+
name: python-wheel
3+
4+
artifacts:
5+
art1:
6+
type: whl
7+
files:
8+
- source: ../other_dab/dist/*.whl
9+
dynamic_version: true
10+
art2:
11+
type: whl
12+
files:
13+
- source: ../other_dab/dist/lib/other_test_code*.whl
14+
dynamic_version: true
15+
16+
sync:
17+
paths:
18+
- ../other_dab
19+
exclude:
20+
- ../other_dab/**
21+
22+
resources:
23+
jobs:
24+
test_job:
25+
name: "[${bundle.target}] My Wheel Job"
26+
tasks:
27+
- task_key: TestTask
28+
existing_cluster_id: "0717-132531-5opeqon1"
29+
python_wheel_task:
30+
package_name: "my_test_code"
31+
entry_point: "run"
32+
libraries:
33+
- whl: ../other_dab/dist/*.whl
34+
- whl: ../other_dab/dist/lib/other_test_code-0.0.1-py3-none-any.whl
35+
- task_key: ServerlessTestTask
36+
python_wheel_task:
37+
package_name: "my_test_code"
38+
entry_point: "run"
39+
environment_key: "test_env"
40+
environments:
41+
- environment_key: "test_env"
42+
spec:
43+
client: "1"
44+
dependencies:
45+
- ../other_dab/dist/*.whl
46+
- ../other_dab/dist/lib/other_test_code-0.0.1-py3-none-any.whl

‎bundle/libraries/switch_to_patched_wheels.go‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,17 @@ import (
88
"github.com/databricks/cli/bundle/config"
99
"github.com/databricks/cli/libs/diag"
1010
"github.com/databricks/cli/libs/log"
11+
"github.com/databricks/cli/libs/utils"
1112
)
1213

1314
type switchToPatchedWheels struct{}
1415

1516
func (c switchToPatchedWheels) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
16-
replacements := getReplacements(ctx, b.Config.Artifacts, b.BundleRoot.Native())
17+
replacements := getReplacements(ctx, b.Config.Artifacts, b.SyncRoot.Native())
18+
19+
if len(replacements) == 0 {
20+
return nil
21+
}
1722

1823
for jobName, jobRef := range b.Config.Resources.Jobs {
1924
if jobRef == nil {
@@ -27,13 +32,14 @@ func (c switchToPatchedWheels) Apply(ctx context.Context, b *bundle.Bundle) diag
2732
}
2833

2934
for taskInd, task := range job.Tasks {
30-
3135
// Update resources.jobs.*.task[*].libraries[*].whl
3236
for libInd, lib := range task.Libraries {
3337
repl := replacements[lib.Whl]
3438
if repl != "" {
3539
log.Debugf(ctx, "Updating resources.jobs.%s.task[%d].libraries[%d].whl from %s to %s", jobName, taskInd, libInd, lib.Whl, repl)
3640
job.Tasks[taskInd].Libraries[libInd].Whl = repl
41+
} else {
42+
log.Debugf(ctx, "Not updating resources.jobs.%s.task[%d].libraries[%d].whl from %s. Available replacements: %v", jobName, taskInd, libInd, lib.Whl, utils.SortedKeys(replacements))
3743
}
3844
}
3945

@@ -46,6 +52,8 @@ func (c switchToPatchedWheels) Apply(ctx context.Context, b *bundle.Bundle) diag
4652
if repl != "" {
4753
log.Debugf(ctx, "Updating resources.jobs.%s.task[%d].for_each_task.task.libraries[%d].whl from %s to %s", jobName, taskInd, libInd, lib.Whl, repl)
4854
foreachptr.Task.Libraries[libInd].Whl = repl
55+
} else {
56+
log.Debugf(ctx, "Not updating resources.jobs.%s.task[%d].for_each_task.task.libraries[%d].whl from %s. Available replacements: %v", jobName, taskInd, libInd, lib.Whl, utils.SortedKeys(replacements))
4957
}
5058
}
5159
}
@@ -62,6 +70,8 @@ func (c switchToPatchedWheels) Apply(ctx context.Context, b *bundle.Bundle) diag
6270
if repl != "" {
6371
log.Debugf(ctx, "Updating resources.jobs.%s.environments[%d].spec.dependencies[%d] from %s to %s", jobName, envInd, depInd, dep, repl)
6472
specptr.Dependencies[depInd] = repl
73+
} else {
74+
log.Debugf(ctx, "Not updating resources.jobs.%s.environments[%d].spec.dependencies[%d] from %s. Available replacements: %v", jobName, envInd, depInd, dep, utils.SortedKeys(replacements))
6575
}
6676
}
6777
}
@@ -70,19 +80,19 @@ func (c switchToPatchedWheels) Apply(ctx context.Context, b *bundle.Bundle) diag
7080
return nil
7181
}
7282

73-
func getReplacements(ctx context.Context, artifacts config.Artifacts, bundleRoot string) map[string]string {
83+
func getReplacements(ctx context.Context, artifacts config.Artifacts, root string) map[string]string {
7484
result := make(map[string]string)
7585
for _, artifact := range artifacts {
7686
for _, file := range artifact.Files {
7787
if file.Patched != "" {
78-
source, err := filepath.Rel(bundleRoot, file.Source)
88+
source, err := filepath.Rel(root, file.Source)
7989
if err != nil {
80-
log.Debugf(ctx, "Failed to get relative path (%s, %s): %s", bundleRoot, file.Source, err)
90+
log.Debugf(ctx, "Failed to get relative path (%s, %s): %s", root, file.Source, err)
8191
continue
8292
}
83-
patched, err := filepath.Rel(bundleRoot, file.Patched)
93+
patched, err := filepath.Rel(root, file.Patched)
8494
if err != nil {
85-
log.Debugf(ctx, "Failed to get relative path (%s, %s): %s", bundleRoot, file.Patched, err)
95+
log.Debugf(ctx, "Failed to get relative path (%s, %s): %s", root, file.Patched, err)
8696
continue
8797
}
8898
result[source] = patched

0 commit comments

Comments
 (0)