From 3a40b0371ee39c4fb287db5b62f4b36ef8409e73 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 19 Aug 2025 16:41:13 +0200 Subject: [PATCH 01/13] Fix 'bundle plan' to not create workspace objects or upload the files --- acceptance/bundle/plan/no_upload/.gitignore | 1 + .../bundle/plan/no_upload/databricks.yml | 20 ++++++++++++ acceptance/bundle/plan/no_upload/my_script.py | 1 + .../bundle/plan/no_upload/out.test.toml | 5 +++ acceptance/bundle/plan/no_upload/output.txt | 7 ++++ acceptance/bundle/plan/no_upload/script | 9 ++++++ acceptance/bundle/plan/no_upload/test.toml | 3 ++ acceptance/bundle/plan/no_upload/test.whl | 0 bundle/phases/deploy.go | 32 +++++++++++-------- 9 files changed, 65 insertions(+), 13 deletions(-) create mode 100644 acceptance/bundle/plan/no_upload/.gitignore create mode 100644 acceptance/bundle/plan/no_upload/databricks.yml create mode 100644 acceptance/bundle/plan/no_upload/my_script.py create mode 100644 acceptance/bundle/plan/no_upload/out.test.toml create mode 100644 acceptance/bundle/plan/no_upload/output.txt create mode 100644 acceptance/bundle/plan/no_upload/script create mode 100644 acceptance/bundle/plan/no_upload/test.toml create mode 100644 acceptance/bundle/plan/no_upload/test.whl diff --git a/acceptance/bundle/plan/no_upload/.gitignore b/acceptance/bundle/plan/no_upload/.gitignore new file mode 100644 index 0000000000..15bcc6dd09 --- /dev/null +++ b/acceptance/bundle/plan/no_upload/.gitignore @@ -0,0 +1 @@ +.databricks diff --git a/acceptance/bundle/plan/no_upload/databricks.yml b/acceptance/bundle/plan/no_upload/databricks.yml new file mode 100644 index 0000000000..9b036d72f1 --- /dev/null +++ b/acceptance/bundle/plan/no_upload/databricks.yml @@ -0,0 +1,20 @@ +bundle: + name: plan-no-upload + +resources: + jobs: + my_job: + name: my-job + tasks: + - task_key: task1 + spark_python_task: + python_file: "./my_script.py" + new_cluster: + num_workers: 1 + spark_version: 13.3.x-scala2.12 + node_type_id: i3.xlarge + driver_node_type_id: i3.xlarge + custom_tags: + "ResourceClass": "SingleNode" + libraries: + - whl: "*.whl" diff --git a/acceptance/bundle/plan/no_upload/my_script.py b/acceptance/bundle/plan/no_upload/my_script.py new file mode 100644 index 0000000000..7df869a15e --- /dev/null +++ b/acceptance/bundle/plan/no_upload/my_script.py @@ -0,0 +1 @@ +print("Hello, World!") diff --git a/acceptance/bundle/plan/no_upload/out.test.toml b/acceptance/bundle/plan/no_upload/out.test.toml new file mode 100644 index 0000000000..8f3575be7b --- /dev/null +++ b/acceptance/bundle/plan/no_upload/out.test.toml @@ -0,0 +1,5 @@ +Local = true +Cloud = false + +[EnvMatrix] + DATABRICKS_CLI_DEPLOYMENT = ["terraform", "direct-exp"] diff --git a/acceptance/bundle/plan/no_upload/output.txt b/acceptance/bundle/plan/no_upload/output.txt new file mode 100644 index 0000000000..977873ad00 --- /dev/null +++ b/acceptance/bundle/plan/no_upload/output.txt @@ -0,0 +1,7 @@ + +>>> [CLI] bundle plan +create jobs.my_job + +>>> jq -s .[] | select(.path=="/api/2.0/workspace/delete") | select(.body.path | test(".*/artifacts/.internal")) out.requests.txt + +>>> jq -s .[] | select(.path | contains("/api/2.0/workspace-files/import-file")) out.requests.txt diff --git a/acceptance/bundle/plan/no_upload/script b/acceptance/bundle/plan/no_upload/script new file mode 100644 index 0000000000..4329f6aee2 --- /dev/null +++ b/acceptance/bundle/plan/no_upload/script @@ -0,0 +1,9 @@ +trace $CLI bundle plan + +# Expect no clean up requests +trace jq -s '.[] | select(.path=="/api/2.0/workspace/delete") | select(.body.path | test(".*/artifacts/.internal"))' out.requests.txt + +# Expect no upload requests +trace jq -s '.[] | select(.path | contains("/api/2.0/workspace-files/import-file"))' out.requests.txt + +rm out.requests.txt diff --git a/acceptance/bundle/plan/no_upload/test.toml b/acceptance/bundle/plan/no_upload/test.toml new file mode 100644 index 0000000000..9efba53719 --- /dev/null +++ b/acceptance/bundle/plan/no_upload/test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +RecordRequests = true diff --git a/acceptance/bundle/plan/no_upload/test.whl b/acceptance/bundle/plan/no_upload/test.whl new file mode 100644 index 0000000000..e69de29bb2 diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index 1f6043f457..4b86025608 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -144,12 +144,21 @@ func deployCore(ctx context.Context, b *bundle.Bundle) { } } +// uploadLibraries uploads libraries to the workspace. +// It also cleans up the artifacts directory and transforms wheel tasks. +// It is called by only "bundle deploy". +// Not having TransformWheelTask in plan phase might lead to jobs using these feature not +// being reported in plan as being updated. This is acceptable since TransformWheelTask is +// an experimental feature. +func uploadLibraries(ctx context.Context, b *bundle.Bundle) { + bundle.ApplySeqContext(ctx, b, + artifacts.CleanUp(), + libraries.Upload(), + trampoline.TransformWheelTask(), + ) +} + // deployPrepare is common set of mutators between "bundle plan" and "bundle deploy". -// Ideally it should not modify the remote, only in-memory bundle config. -// TODO: currently it does affect remote, via artifacts.CleanUp() and libraries.Upload(). -// We should refactor deployment so that it consists of two stages: -// 1. Preparation: only local config is changed. This will be used by both "bundle deploy" and "bundle plan" -// 2. Deployment: this does all the uploads. Only used by "deploy", not "plan". func deployPrepare(ctx context.Context, b *bundle.Bundle) { bundle.ApplySeqContext(ctx, b, statemgmt.StatePull(), @@ -158,9 +167,6 @@ func deployPrepare(ctx context.Context, b *bundle.Bundle) { mutator.ValidateGitDetails(), terraform.CheckRunningResource(), - // artifacts.CleanUp() is there because I'm not sure if it's safe to move to later stage. - artifacts.CleanUp(), - // libraries.CheckForSameNameLibraries() needs to be run after we expand glob references so we // know what are the actual library paths. // libraries.ExpandGlobReferences() has to be run after the libraries are built and thus this @@ -170,11 +176,6 @@ func deployPrepare(ctx context.Context, b *bundle.Bundle) { // SwitchToPatchedWheels must be run after ExpandGlobReferences and after build phase because it Artifact.Source and Artifact.Patched populated libraries.SwitchToPatchedWheels(), - // libraries.Upload() not just uploads but also replaces local paths with remote paths. - // TransformWheelTask depends on it and planning also depends on it. - libraries.Upload(), - trampoline.TransformWheelTask(), - mutator.ResolveVariableReferencesOnlyResources( "resources", ), @@ -214,6 +215,11 @@ func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHand return } + uploadLibraries(ctx, b) + if logdiag.HasError(ctx) { + return + } + bundle.ApplySeqContext(ctx, b, files.Upload(outputHandler), deploy.StateUpdate(), From b52e57fc292b9fc014dfe29948e9949fe9583a5a Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 19 Aug 2025 17:00:07 +0200 Subject: [PATCH 02/13] move ResolveVariableReferencesOnlyResources to after uploadLibraries --- bundle/phases/deploy.go | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index 4b86025608..40db3d54dc 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -175,17 +175,6 @@ func deployPrepare(ctx context.Context, b *bundle.Bundle) { libraries.CheckForSameNameLibraries(), // SwitchToPatchedWheels must be run after ExpandGlobReferences and after build phase because it Artifact.Source and Artifact.Patched populated libraries.SwitchToPatchedWheels(), - - mutator.ResolveVariableReferencesOnlyResources( - "resources", - ), - - // Reads (typed): resources.pipelines.*.{catalog,schema,target}, resources.volumes.*.{catalog_name,schema_name} (checks for schema references) - // Updates (typed): resources.pipelines.*.{schema,target}, resources.volumes.*.schema_name (converts implicit schema references to explicit ${resources.schemas..name} syntax) - // Translates implicit schema references in DLT pipelines or UC Volumes to explicit syntax to capture dependencies - // Needs to be run after ${resources} resolution since otherwise that undoes the change here. - // TODO: one we have depends_on support we should leverage that here and move this back to initialize phase. - resourcemutator.CaptureSchemaDependency(), ) } @@ -221,6 +210,16 @@ func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHand } bundle.ApplySeqContext(ctx, b, + mutator.ResolveVariableReferencesOnlyResources( + "resources", + ), + + // Reads (typed): resources.pipelines.*.{catalog,schema,target}, resources.volumes.*.{catalog_name,schema_name} (checks for schema references) + // Updates (typed): resources.pipelines.*.{schema,target}, resources.volumes.*.schema_name (converts implicit schema references to explicit ${resources.schemas..name} syntax) + // Translates implicit schema references in DLT pipelines or UC Volumes to explicit syntax to capture dependencies + // Needs to be run after ${resources} resolution since otherwise that undoes the change here. + // TODO: one we have depends_on support we should leverage that here and move this back to initialize phase. + resourcemutator.CaptureSchemaDependency(), files.Upload(outputHandler), deploy.StateUpdate(), deploy.StatePush(), From 6c3eaaace61c13bd3af2b475e42a6a97ecb36f20 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 20 Aug 2025 15:18:55 +0200 Subject: [PATCH 03/13] filter all non get requests --- acceptance/bundle/plan/no_upload/output.txt | 2 +- acceptance/bundle/plan/no_upload/script | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/acceptance/bundle/plan/no_upload/output.txt b/acceptance/bundle/plan/no_upload/output.txt index 977873ad00..1751f72207 100644 --- a/acceptance/bundle/plan/no_upload/output.txt +++ b/acceptance/bundle/plan/no_upload/output.txt @@ -2,6 +2,6 @@ >>> [CLI] bundle plan create jobs.my_job ->>> jq -s .[] | select(.path=="/api/2.0/workspace/delete") | select(.body.path | test(".*/artifacts/.internal")) out.requests.txt +>>> jq -s .[] | select(.path | contains("api/2.0/workspace")) | select(.method != "GET") out.requests.txt >>> jq -s .[] | select(.path | contains("/api/2.0/workspace-files/import-file")) out.requests.txt diff --git a/acceptance/bundle/plan/no_upload/script b/acceptance/bundle/plan/no_upload/script index 4329f6aee2..203bf2dc23 100644 --- a/acceptance/bundle/plan/no_upload/script +++ b/acceptance/bundle/plan/no_upload/script @@ -1,7 +1,7 @@ trace $CLI bundle plan # Expect no clean up requests -trace jq -s '.[] | select(.path=="/api/2.0/workspace/delete") | select(.body.path | test(".*/artifacts/.internal"))' out.requests.txt +trace jq -s '.[] | select(.path | contains("api/2.0/workspace")) | select(.method != "GET")' out.requests.txt # Expect no upload requests trace jq -s '.[] | select(.path | contains("/api/2.0/workspace-files/import-file"))' out.requests.txt From 22a6ea6016769b8f83f945b99fa1af8f72f9481d Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 20 Aug 2025 16:43:52 +0200 Subject: [PATCH 04/13] extracted remote path mutator --- bundle/libraries/remote_path.go | 65 +++++++++++++++++++++++++++++++++ bundle/libraries/upload.go | 28 +------------- bundle/phases/deploy.go | 8 ++-- 3 files changed, 70 insertions(+), 31 deletions(-) create mode 100644 bundle/libraries/remote_path.go diff --git a/bundle/libraries/remote_path.go b/bundle/libraries/remote_path.go new file mode 100644 index 0000000000..0c547af7bc --- /dev/null +++ b/bundle/libraries/remote_path.go @@ -0,0 +1,65 @@ +package libraries + +import ( + "context" + "path" + "path/filepath" + "strings" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/utils" +) + +type remotePath struct{} + +// ReplaceWithRemotePath updates all the libraries paths to point to the remote location +// where the libraries will be uploaded later. +func ReplaceWithRemotePath() bundle.Mutator { + return &remotePath{} +} + +func (r *remotePath) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + _, uploadPath, diags := GetFilerForLibraries(ctx, b) + if diags.HasError() { + return diags + } + + libs, err := collectLocalLibraries(b) + if err != nil { + return diag.FromErr(err) + } + + sources := utils.SortedKeys(libs) + + // Update all the config paths to point to the uploaded location + for _, source := range sources { + locations := libs[source] + err = b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + remotePath := path.Join(uploadPath, filepath.Base(source)) + + // If the remote path does not start with /Workspace or /Volumes, prepend /Workspace + if !strings.HasPrefix(remotePath, "/Workspace") && !strings.HasPrefix(remotePath, "/Volumes") { + remotePath = "/Workspace" + remotePath + } + for _, location := range locations { + v, err = dyn.SetByPath(v, location.configPath, dyn.NewValue(remotePath, []dyn.Location{location.location})) + if err != nil { + return v, err + } + } + + return v, nil + }) + if err != nil { + diags = diags.Extend(diag.FromErr(err)) + } + } + + return diags +} + +func (r *remotePath) Name() string { + return "libraries.ReplaceWithRemotePath" +} diff --git a/bundle/libraries/upload.go b/bundle/libraries/upload.go index ed69d27f88..2922790601 100644 --- a/bundle/libraries/upload.go +++ b/bundle/libraries/upload.go @@ -5,9 +5,7 @@ import ( "errors" "fmt" "os" - "path" "path/filepath" - "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/cmdio" @@ -135,7 +133,7 @@ func collectLocalLibraries(b *bundle.Bundle) (map[string][]configLocation, error } func (u *upload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - client, uploadPath, diags := GetFilerForLibraries(ctx, b) + client, _, diags := GetFilerForLibraries(ctx, b) if diags.HasError() { return diags } @@ -173,30 +171,6 @@ func (u *upload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { return diag.FromErr(err) } - // Update all the config paths to point to the uploaded location - for _, source := range sources { - locations := libs[source] - err = b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { - remotePath := path.Join(uploadPath, filepath.Base(source)) - - // If the remote path does not start with /Workspace or /Volumes, prepend /Workspace - if !strings.HasPrefix(remotePath, "/Workspace") && !strings.HasPrefix(remotePath, "/Volumes") { - remotePath = "/Workspace" + remotePath - } - for _, location := range locations { - v, err = dyn.SetByPath(v, location.configPath, dyn.NewValue(remotePath, []dyn.Location{location.location})) - if err != nil { - return v, err - } - } - - return v, nil - }) - if err != nil { - diags = diags.Extend(diag.FromErr(err)) - } - } - return diags } diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index 7d1442760e..6cea133d02 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -146,14 +146,10 @@ func deployCore(ctx context.Context, b *bundle.Bundle) { // uploadLibraries uploads libraries to the workspace. // It also cleans up the artifacts directory and transforms wheel tasks. // It is called by only "bundle deploy". -// Not having TransformWheelTask in plan phase might lead to jobs using these feature not -// being reported in plan as being updated. This is acceptable since TransformWheelTask is -// an experimental feature. func uploadLibraries(ctx context.Context, b *bundle.Bundle) { bundle.ApplySeqContext(ctx, b, artifacts.CleanUp(), libraries.Upload(), - trampoline.TransformWheelTask(), ) } @@ -174,6 +170,10 @@ func deployPrepare(ctx context.Context, b *bundle.Bundle) { libraries.CheckForSameNameLibraries(), // SwitchToPatchedWheels must be run after ExpandGlobReferences and after build phase because it Artifact.Source and Artifact.Patched populated libraries.SwitchToPatchedWheels(), + libraries.ReplaceWithRemotePath(), + // TransformWheelTask must be run after ReplaceWithRemotePath so we can use correct remote path in the + // transformed notebook + trampoline.TransformWheelTask(), ) } From e4e0f7853e57deb5f09e4bb0f862d74a9c189872 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 20 Aug 2025 17:15:08 +0200 Subject: [PATCH 05/13] export libs to upload from deployPrepare --- bundle/libraries/remote_path.go | 105 ++++++++-- bundle/libraries/upload.go | 109 +---------- bundle/libraries/upload_test.go | 331 -------------------------------- bundle/phases/deploy.go | 21 +- 4 files changed, 117 insertions(+), 449 deletions(-) delete mode 100644 bundle/libraries/upload_test.go diff --git a/bundle/libraries/remote_path.go b/bundle/libraries/remote_path.go index 0c547af7bc..31efeb3e0c 100644 --- a/bundle/libraries/remote_path.go +++ b/bundle/libraries/remote_path.go @@ -2,6 +2,7 @@ package libraries import ( "context" + "fmt" "path" "path/filepath" "strings" @@ -12,23 +13,17 @@ import ( "github.com/databricks/cli/libs/utils" ) -type remotePath struct{} - // ReplaceWithRemotePath updates all the libraries paths to point to the remote location // where the libraries will be uploaded later. -func ReplaceWithRemotePath() bundle.Mutator { - return &remotePath{} -} - -func (r *remotePath) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { +func ReplaceWithRemotePath(ctx context.Context, b *bundle.Bundle) (map[string][]ConfigLocation, diag.Diagnostics) { _, uploadPath, diags := GetFilerForLibraries(ctx, b) if diags.HasError() { - return diags + return nil, diags } libs, err := collectLocalLibraries(b) if err != nil { - return diag.FromErr(err) + return nil, diag.FromErr(err) } sources := utils.SortedKeys(libs) @@ -57,9 +52,95 @@ func (r *remotePath) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnosti } } - return diags + return libs, diags } -func (r *remotePath) Name() string { - return "libraries.ReplaceWithRemotePath" +// Collect all libraries from the bundle configuration and their config paths. +// By this stage all glob references are expanded and we have a list of all libraries that need to be uploaded. +// We collect them from task libraries, foreach task libraries, environment dependencies, and artifacts. +// We return a map of library source to a list of config paths and locations where the library is used. +// We use map so we don't upload the same library multiple times. +// Instead we upload it once and update all the config paths to point to the uploaded location. +func collectLocalLibraries(b *bundle.Bundle) (map[string][]ConfigLocation, error) { + libs := make(map[string]([]ConfigLocation)) + + patterns := []dyn.Pattern{ + taskLibrariesPattern.Append(dyn.AnyIndex(), dyn.Key("whl")), + taskLibrariesPattern.Append(dyn.AnyIndex(), dyn.Key("jar")), + forEachTaskLibrariesPattern.Append(dyn.AnyIndex(), dyn.Key("whl")), + forEachTaskLibrariesPattern.Append(dyn.AnyIndex(), dyn.Key("jar")), + envDepsPattern.Append(dyn.AnyIndex()), + pipelineEnvDepsPattern.Append(dyn.AnyIndex()), + } + + for _, pattern := range patterns { + err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + return dyn.MapByPattern(v, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + source, ok := v.AsString() + if !ok { + return v, fmt.Errorf("expected string, got %s", v.Kind()) + } + + if !IsLibraryLocal(source) { + return v, nil + } + + source = filepath.Join(b.SyncRootPath, source) + libs[source] = append(libs[source], ConfigLocation{ + configPath: p, + location: v.Location(), + }) + + return v, nil + }) + }) + if err != nil { + return nil, err + } + } + + artifactPattern := dyn.NewPattern( + dyn.Key("artifacts"), + dyn.AnyKey(), + dyn.Key("files"), + dyn.AnyIndex(), + ) + + err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + return dyn.MapByPattern(v, artifactPattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + file, ok := v.AsMap() + if !ok { + return v, fmt.Errorf("expected map, got %s", v.Kind()) + } + + sv, ok := file.GetByString("source") + if !ok { + return v, nil + } + + source, ok := sv.AsString() + if !ok { + return v, fmt.Errorf("expected string, got %s", v.Kind()) + } + + if sv, ok = file.GetByString("patched"); ok { + patched, ok := sv.AsString() + if ok && patched != "" { + source = patched + } + } + + libs[source] = append(libs[source], ConfigLocation{ + configPath: p.Append(dyn.Key("remote_path")), + location: v.Location(), + }) + + return v, nil + }) + }) + if err != nil { + return nil, err + } + + return libs, nil } diff --git a/bundle/libraries/upload.go b/bundle/libraries/upload.go index 2922790601..80df0b3c76 100644 --- a/bundle/libraries/upload.go +++ b/bundle/libraries/upload.go @@ -23,115 +23,29 @@ import ( // avoid hitting the rate limit. var maxFilesRequestsInFlight = 5 -func Upload() bundle.Mutator { - return &upload{} +func Upload(libs map[string][]ConfigLocation) bundle.Mutator { + return &upload{ + libs: libs, + } } -func UploadWithClient(client filer.Filer) bundle.Mutator { +func UploadWithClient(libs map[string][]ConfigLocation, client filer.Filer) bundle.Mutator { return &upload{ + libs: libs, client: client, } } type upload struct { client filer.Filer + libs map[string][]ConfigLocation } -type configLocation struct { +type ConfigLocation struct { configPath dyn.Path location dyn.Location } -// Collect all libraries from the bundle configuration and their config paths. -// By this stage all glob references are expanded and we have a list of all libraries that need to be uploaded. -// We collect them from task libraries, foreach task libraries, environment dependencies, and artifacts. -// We return a map of library source to a list of config paths and locations where the library is used. -// We use map so we don't upload the same library multiple times. -// Instead we upload it once and update all the config paths to point to the uploaded location. -func collectLocalLibraries(b *bundle.Bundle) (map[string][]configLocation, error) { - libs := make(map[string]([]configLocation)) - - patterns := []dyn.Pattern{ - taskLibrariesPattern.Append(dyn.AnyIndex(), dyn.Key("whl")), - taskLibrariesPattern.Append(dyn.AnyIndex(), dyn.Key("jar")), - forEachTaskLibrariesPattern.Append(dyn.AnyIndex(), dyn.Key("whl")), - forEachTaskLibrariesPattern.Append(dyn.AnyIndex(), dyn.Key("jar")), - envDepsPattern.Append(dyn.AnyIndex()), - pipelineEnvDepsPattern.Append(dyn.AnyIndex()), - } - - for _, pattern := range patterns { - err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { - return dyn.MapByPattern(v, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { - source, ok := v.AsString() - if !ok { - return v, fmt.Errorf("expected string, got %s", v.Kind()) - } - - if !IsLibraryLocal(source) { - return v, nil - } - - source = filepath.Join(b.SyncRootPath, source) - libs[source] = append(libs[source], configLocation{ - configPath: p, - location: v.Location(), - }) - - return v, nil - }) - }) - if err != nil { - return nil, err - } - } - - artifactPattern := dyn.NewPattern( - dyn.Key("artifacts"), - dyn.AnyKey(), - dyn.Key("files"), - dyn.AnyIndex(), - ) - - err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { - return dyn.MapByPattern(v, artifactPattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { - file, ok := v.AsMap() - if !ok { - return v, fmt.Errorf("expected map, got %s", v.Kind()) - } - - sv, ok := file.GetByString("source") - if !ok { - return v, nil - } - - source, ok := sv.AsString() - if !ok { - return v, fmt.Errorf("expected string, got %s", v.Kind()) - } - - if sv, ok = file.GetByString("patched"); ok { - patched, ok := sv.AsString() - if ok && patched != "" { - source = patched - } - } - - libs[source] = append(libs[source], configLocation{ - configPath: p.Append(dyn.Key("remote_path")), - location: v.Location(), - }) - - return v, nil - }) - }) - if err != nil { - return nil, err - } - - return libs, nil -} - func (u *upload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { client, _, diags := GetFilerForLibraries(ctx, b) if diags.HasError() { @@ -144,12 +58,7 @@ func (u *upload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { u.client = client } - libs, err := collectLocalLibraries(b) - if err != nil { - return diag.FromErr(err) - } - - sources := utils.SortedKeys(libs) + sources := utils.SortedKeys(u.libs) errs, errCtx := errgroup.WithContext(ctx) errs.SetLimit(maxFilesRequestsInFlight) diff --git a/bundle/libraries/upload_test.go b/bundle/libraries/upload_test.go deleted file mode 100644 index 00c8bef6de..0000000000 --- a/bundle/libraries/upload_test.go +++ /dev/null @@ -1,331 +0,0 @@ -package libraries - -import ( - "context" - "path/filepath" - "testing" - - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config" - "github.com/databricks/cli/bundle/config/resources" - mockfiler "github.com/databricks/cli/internal/mocks/libs/filer" - "github.com/databricks/cli/internal/testutil" - "github.com/databricks/cli/libs/filer" - "github.com/databricks/databricks-sdk-go/service/compute" - "github.com/databricks/databricks-sdk-go/service/jobs" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" -) - -func TestArtifactUploadForWorkspace(t *testing.T) { - tmpDir := t.TempDir() - whlFolder := filepath.Join(tmpDir, "whl") - testutil.Touch(t, whlFolder, "source.whl") - whlLocalPath := filepath.Join(whlFolder, "source.whl") - - b := &bundle.Bundle{ - SyncRootPath: tmpDir, - Config: config.Root{ - Workspace: config.Workspace{ - ArtifactPath: "/foo/bar/artifacts", - }, - Artifacts: config.Artifacts{ - "whl": { - Type: config.ArtifactPythonWheel, - Files: []config.ArtifactFile{ - {Source: whlLocalPath}, - }, - }, - }, - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "job": { - JobSettings: jobs.JobSettings{ - Tasks: []jobs.Task{ - { - Libraries: []compute.Library{ - { - Whl: filepath.Join("whl", "*.whl"), - }, - { - Whl: "/Workspace/Users/foo@bar.com/mywheel.whl", - }, - }, - }, - { - ForEachTask: &jobs.ForEachTask{ - Task: jobs.Task{ - Libraries: []compute.Library{ - { - Whl: filepath.Join("whl", "*.whl"), - }, - { - Whl: "/Workspace/Users/foo@bar.com/mywheel.whl", - }, - }, - }, - }, - }, - }, - Environments: []jobs.JobEnvironment{ - { - Spec: &compute.Environment{ - Dependencies: []string{ - filepath.Join("whl", "source.whl"), - "/Workspace/Users/foo@bar.com/mywheel.whl", - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - - mockFiler := mockfiler.NewMockFiler(t) - mockFiler.EXPECT().Write( - mock.Anything, - filepath.Join("source.whl"), - mock.AnythingOfType("*os.File"), - filer.OverwriteIfExists, - filer.CreateParentDirectories, - ).Return(nil) - - diags := bundle.ApplySeq(context.Background(), b, ExpandGlobReferences(), UploadWithClient(mockFiler)) - require.NoError(t, diags.Error()) - - // Test that libraries path is updated - require.Equal(t, "/Workspace/foo/bar/artifacts/.internal/source.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries[0].Whl) - require.Equal(t, "/Workspace/Users/foo@bar.com/mywheel.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries[1].Whl) - require.Equal(t, "/Workspace/foo/bar/artifacts/.internal/source.whl", b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies[0]) - require.Equal(t, "/Workspace/Users/foo@bar.com/mywheel.whl", b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies[1]) - require.Equal(t, "/Workspace/foo/bar/artifacts/.internal/source.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[1].ForEachTask.Task.Libraries[0].Whl) - require.Equal(t, "/Workspace/Users/foo@bar.com/mywheel.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[1].ForEachTask.Task.Libraries[1].Whl) -} - -func TestArtifactUploadForVolumes(t *testing.T) { - tmpDir := t.TempDir() - whlFolder := filepath.Join(tmpDir, "whl") - testutil.Touch(t, whlFolder, "source.whl") - whlLocalPath := filepath.Join(whlFolder, "source.whl") - - b := &bundle.Bundle{ - SyncRootPath: tmpDir, - Config: config.Root{ - Workspace: config.Workspace{ - ArtifactPath: "/Volumes/foo/bar/artifacts", - }, - Artifacts: config.Artifacts{ - "whl": { - Type: config.ArtifactPythonWheel, - Files: []config.ArtifactFile{ - {Source: whlLocalPath}, - }, - }, - }, - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "job": { - JobSettings: jobs.JobSettings{ - Tasks: []jobs.Task{ - { - Libraries: []compute.Library{ - { - Whl: filepath.Join("whl", "*.whl"), - }, - { - Whl: "/Volumes/some/path/mywheel.whl", - }, - }, - }, - { - ForEachTask: &jobs.ForEachTask{ - Task: jobs.Task{ - Libraries: []compute.Library{ - { - Whl: filepath.Join("whl", "*.whl"), - }, - { - Whl: "/Volumes/some/path/mywheel.whl", - }, - }, - }, - }, - }, - }, - Environments: []jobs.JobEnvironment{ - { - Spec: &compute.Environment{ - Dependencies: []string{ - filepath.Join("whl", "source.whl"), - "/Volumes/some/path/mywheel.whl", - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - - mockFiler := mockfiler.NewMockFiler(t) - mockFiler.EXPECT().Write( - mock.Anything, - filepath.Join("source.whl"), - mock.AnythingOfType("*os.File"), - filer.OverwriteIfExists, - filer.CreateParentDirectories, - ).Return(nil) - - diags := bundle.ApplySeq(context.Background(), b, ExpandGlobReferences(), UploadWithClient(mockFiler)) - require.NoError(t, diags.Error()) - - // Test that libraries path is updated - require.Equal(t, "/Volumes/foo/bar/artifacts/.internal/source.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries[0].Whl) - require.Equal(t, "/Volumes/some/path/mywheel.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries[1].Whl) - require.Equal(t, "/Volumes/foo/bar/artifacts/.internal/source.whl", b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies[0]) - require.Equal(t, "/Volumes/some/path/mywheel.whl", b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies[1]) - require.Equal(t, "/Volumes/foo/bar/artifacts/.internal/source.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[1].ForEachTask.Task.Libraries[0].Whl) - require.Equal(t, "/Volumes/some/path/mywheel.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[1].ForEachTask.Task.Libraries[1].Whl) -} - -func TestArtifactUploadWithNoLibraryReference(t *testing.T) { - tmpDir := t.TempDir() - whlFolder := filepath.Join(tmpDir, "whl") - testutil.Touch(t, whlFolder, "source.whl") - whlLocalPath := filepath.Join(whlFolder, "source.whl") - - b := &bundle.Bundle{ - SyncRootPath: tmpDir, - Config: config.Root{ - Workspace: config.Workspace{ - ArtifactPath: "/Workspace/foo/bar/artifacts", - }, - Artifacts: config.Artifacts{ - "whl": { - Type: config.ArtifactPythonWheel, - Files: []config.ArtifactFile{ - {Source: whlLocalPath}, - }, - }, - }, - }, - } - - mockFiler := mockfiler.NewMockFiler(t) - mockFiler.EXPECT().Write( - mock.Anything, - filepath.Join("source.whl"), - mock.AnythingOfType("*os.File"), - filer.OverwriteIfExists, - filer.CreateParentDirectories, - ).Return(nil) - - diags := bundle.ApplySeq(context.Background(), b, ExpandGlobReferences(), UploadWithClient(mockFiler)) - require.NoError(t, diags.Error()) - - require.Equal(t, "/Workspace/foo/bar/artifacts/.internal/source.whl", b.Config.Artifacts["whl"].Files[0].RemotePath) -} - -func TestUploadMultipleLibraries(t *testing.T) { - tmpDir := t.TempDir() - whlFolder := filepath.Join(tmpDir, "whl") - testutil.Touch(t, whlFolder, "source1.whl") - testutil.Touch(t, whlFolder, "source2.whl") - testutil.Touch(t, whlFolder, "source3.whl") - testutil.Touch(t, whlFolder, "source4.whl") - - b := &bundle.Bundle{ - SyncRootPath: tmpDir, - Config: config.Root{ - Workspace: config.Workspace{ - ArtifactPath: "/foo/bar/artifacts", - }, - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "job": { - JobSettings: jobs.JobSettings{ - Tasks: []jobs.Task{ - { - Libraries: []compute.Library{ - { - Whl: filepath.Join("whl", "*.whl"), - }, - { - Whl: "/Workspace/Users/foo@bar.com/mywheel.whl", - }, - }, - }, - }, - Environments: []jobs.JobEnvironment{ - { - Spec: &compute.Environment{ - Dependencies: []string{ - filepath.Join("whl", "*.whl"), - "/Workspace/Users/foo@bar.com/mywheel.whl", - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - - mockFiler := mockfiler.NewMockFiler(t) - mockFiler.EXPECT().Write( - mock.Anything, - filepath.Join("source1.whl"), - mock.AnythingOfType("*os.File"), - filer.OverwriteIfExists, - filer.CreateParentDirectories, - ).Return(nil).Once() - - mockFiler.EXPECT().Write( - mock.Anything, - filepath.Join("source2.whl"), - mock.AnythingOfType("*os.File"), - filer.OverwriteIfExists, - filer.CreateParentDirectories, - ).Return(nil).Once() - - mockFiler.EXPECT().Write( - mock.Anything, - filepath.Join("source3.whl"), - mock.AnythingOfType("*os.File"), - filer.OverwriteIfExists, - filer.CreateParentDirectories, - ).Return(nil).Once() - - mockFiler.EXPECT().Write( - mock.Anything, - filepath.Join("source4.whl"), - mock.AnythingOfType("*os.File"), - filer.OverwriteIfExists, - filer.CreateParentDirectories, - ).Return(nil).Once() - - diags := bundle.ApplySeq(context.Background(), b, ExpandGlobReferences(), UploadWithClient(mockFiler)) - require.NoError(t, diags.Error()) - - // Test that libraries path is updated - require.Len(t, b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries, 5) - require.Contains(t, b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries, compute.Library{Whl: "/Workspace/foo/bar/artifacts/.internal/source1.whl"}) - require.Contains(t, b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries, compute.Library{Whl: "/Workspace/foo/bar/artifacts/.internal/source2.whl"}) - require.Contains(t, b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries, compute.Library{Whl: "/Workspace/foo/bar/artifacts/.internal/source3.whl"}) - require.Contains(t, b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries, compute.Library{Whl: "/Workspace/foo/bar/artifacts/.internal/source4.whl"}) - require.Contains(t, b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries, compute.Library{Whl: "/Workspace/Users/foo@bar.com/mywheel.whl"}) - - require.Len(t, b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies, 5) - require.Contains(t, b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies, "/Workspace/foo/bar/artifacts/.internal/source1.whl") - require.Contains(t, b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies, "/Workspace/foo/bar/artifacts/.internal/source2.whl") - require.Contains(t, b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies, "/Workspace/foo/bar/artifacts/.internal/source3.whl") - require.Contains(t, b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies, "/Workspace/foo/bar/artifacts/.internal/source4.whl") - require.Contains(t, b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies, "/Workspace/Users/foo@bar.com/mywheel.whl") -} diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index 6cea133d02..683c9189d9 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -146,15 +146,15 @@ func deployCore(ctx context.Context, b *bundle.Bundle) { // uploadLibraries uploads libraries to the workspace. // It also cleans up the artifacts directory and transforms wheel tasks. // It is called by only "bundle deploy". -func uploadLibraries(ctx context.Context, b *bundle.Bundle) { +func uploadLibraries(ctx context.Context, b *bundle.Bundle, libs map[string][]libraries.ConfigLocation) { bundle.ApplySeqContext(ctx, b, artifacts.CleanUp(), - libraries.Upload(), + libraries.Upload(libs), ) } // deployPrepare is common set of mutators between "bundle plan" and "bundle deploy". -func deployPrepare(ctx context.Context, b *bundle.Bundle) { +func deployPrepare(ctx context.Context, b *bundle.Bundle) map[string][]libraries.ConfigLocation { bundle.ApplySeqContext(ctx, b, statemgmt.StatePull(), terraform.CheckDashboardsModifiedRemotely(), @@ -170,11 +170,20 @@ func deployPrepare(ctx context.Context, b *bundle.Bundle) { libraries.CheckForSameNameLibraries(), // SwitchToPatchedWheels must be run after ExpandGlobReferences and after build phase because it Artifact.Source and Artifact.Patched populated libraries.SwitchToPatchedWheels(), - libraries.ReplaceWithRemotePath(), + ) + + libs, diags := libraries.ReplaceWithRemotePath(ctx, b) + for _, diag := range diags { + logdiag.LogDiag(ctx, diag) + } + + bundle.ApplySeqContext(ctx, b, // TransformWheelTask must be run after ReplaceWithRemotePath so we can use correct remote path in the // transformed notebook trampoline.TransformWheelTask(), ) + + return libs } // The deploy phase deploys artifacts and resources. @@ -198,12 +207,12 @@ func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHand bundle.ApplyContext(ctx, b, lock.Release(lock.GoalDeploy)) }() - deployPrepare(ctx, b) + libs := deployPrepare(ctx, b) if logdiag.HasError(ctx) { return } - uploadLibraries(ctx, b) + uploadLibraries(ctx, b, libs) if logdiag.HasError(ctx) { return } From 616a9a6a2bfae6088b7a9386ead782365da30e52 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 20 Aug 2025 17:20:39 +0200 Subject: [PATCH 06/13] remove duplicate artifacts_test --- integration/bundle/artifacts_test.go | 223 --------------------------- 1 file changed, 223 deletions(-) delete mode 100644 integration/bundle/artifacts_test.go diff --git a/integration/bundle/artifacts_test.go b/integration/bundle/artifacts_test.go deleted file mode 100644 index cc4bb85e66..0000000000 --- a/integration/bundle/artifacts_test.go +++ /dev/null @@ -1,223 +0,0 @@ -package bundle_test - -import ( - "os" - "path" - "path/filepath" - "regexp" - "testing" - - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config" - "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/bundle/libraries" - "github.com/databricks/cli/integration/internal/acc" - "github.com/databricks/databricks-sdk-go/service/compute" - "github.com/databricks/databricks-sdk-go/service/jobs" - "github.com/stretchr/testify/require" -) - -func touchEmptyFile(t *testing.T, path string) { - err := os.MkdirAll(filepath.Dir(path), 0o700) - require.NoError(t, err) - f, err := os.Create(path) - require.NoError(t, err) - f.Close() -} - -func TestUploadArtifactFileToCorrectRemotePath(t *testing.T) { - ctx, wt := acc.WorkspaceTest(t) - dir := t.TempDir() - whlPath := filepath.Join(dir, "dist", "test.whl") - touchEmptyFile(t, whlPath) - - wsDir := acc.TemporaryWorkspaceDir(wt, "artifact-") - - b := &bundle.Bundle{ - BundleRootPath: dir, - SyncRootPath: dir, - Config: config.Root{ - Bundle: config.Bundle{ - Target: "whatever", - }, - Workspace: config.Workspace{ - ArtifactPath: wsDir, - }, - Artifacts: config.Artifacts{ - "test": &config.Artifact{ - Type: "whl", - Files: []config.ArtifactFile{ - { - Source: whlPath, - }, - }, - }, - }, - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "test": { - JobSettings: jobs.JobSettings{ - Tasks: []jobs.Task{ - { - Libraries: []compute.Library{ - { - Whl: "dist/test.whl", - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - - diags := bundle.ApplySeq(ctx, b, libraries.ExpandGlobReferences(), libraries.Upload()) - require.NoError(t, diags.Error()) - - // The remote path attribute on the artifact file should have been set. - require.Regexp(t, - path.Join(regexp.QuoteMeta(wsDir), `.internal/test\.whl`), - b.Config.Artifacts["test"].Files[0].RemotePath, - ) - - // The task library path should have been updated to the remote path. - require.Regexp(t, - path.Join("/Workspace", regexp.QuoteMeta(wsDir), `.internal/test\.whl`), - b.Config.Resources.Jobs["test"].JobSettings.Tasks[0].Libraries[0].Whl, - ) -} - -func TestUploadArtifactFileToCorrectRemotePathWithEnvironments(t *testing.T) { - ctx, wt := acc.WorkspaceTest(t) - dir := t.TempDir() - whlPath := filepath.Join(dir, "dist", "test.whl") - touchEmptyFile(t, whlPath) - - wsDir := acc.TemporaryWorkspaceDir(wt, "artifact-") - - b := &bundle.Bundle{ - BundleRootPath: dir, - SyncRootPath: dir, - Config: config.Root{ - Bundle: config.Bundle{ - Target: "whatever", - }, - Workspace: config.Workspace{ - ArtifactPath: wsDir, - }, - Artifacts: config.Artifacts{ - "test": &config.Artifact{ - Type: "whl", - Files: []config.ArtifactFile{ - { - Source: whlPath, - }, - }, - }, - }, - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "test": { - JobSettings: jobs.JobSettings{ - Environments: []jobs.JobEnvironment{ - { - Spec: &compute.Environment{ - Dependencies: []string{ - "dist/test.whl", - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - - diags := bundle.ApplySeq(ctx, b, libraries.ExpandGlobReferences(), libraries.Upload()) - require.NoError(t, diags.Error()) - - // The remote path attribute on the artifact file should have been set. - require.Regexp(t, - path.Join(regexp.QuoteMeta(wsDir), `.internal/test\.whl`), - b.Config.Artifacts["test"].Files[0].RemotePath, - ) - - // The job environment deps path should have been updated to the remote path. - require.Regexp(t, - path.Join("/Workspace", regexp.QuoteMeta(wsDir), `.internal/test\.whl`), - b.Config.Resources.Jobs["test"].Environments[0].Spec.Dependencies[0], - ) -} - -func TestUploadArtifactFileToCorrectRemotePathForVolumes(t *testing.T) { - ctx, wt := acc.WorkspaceTest(t) - - if os.Getenv("TEST_METASTORE_ID") == "" { - t.Skip("Skipping tests that require a UC Volume when metastore id is not set.") - } - - volumePath := acc.TemporaryVolume(wt) - - dir := t.TempDir() - whlPath := filepath.Join(dir, "dist", "test.whl") - touchEmptyFile(t, whlPath) - - b := &bundle.Bundle{ - BundleRootPath: dir, - SyncRootPath: dir, - Config: config.Root{ - Bundle: config.Bundle{ - Target: "whatever", - }, - Workspace: config.Workspace{ - ArtifactPath: volumePath, - }, - Artifacts: config.Artifacts{ - "test": &config.Artifact{ - Type: "whl", - Files: []config.ArtifactFile{ - { - Source: whlPath, - }, - }, - }, - }, - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "test": { - JobSettings: jobs.JobSettings{ - Tasks: []jobs.Task{ - { - Libraries: []compute.Library{ - { - Whl: "dist/test.whl", - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - - diags := bundle.ApplySeq(ctx, b, libraries.ExpandGlobReferences(), libraries.Upload()) - require.NoError(t, diags.Error()) - - // The remote path attribute on the artifact file should have been set. - require.Regexp(t, - path.Join(regexp.QuoteMeta(volumePath), `.internal/test\.whl`), - b.Config.Artifacts["test"].Files[0].RemotePath, - ) - - // The task library path should have been updated to the remote path. - require.Regexp(t, - path.Join(regexp.QuoteMeta(volumePath), `.internal/test\.whl`), - b.Config.Resources.Jobs["test"].JobSettings.Tasks[0].Libraries[0].Whl, - ) -} From 6d205055395646bc6eda7c3e15ff15d7fb0f3532 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 20 Aug 2025 17:23:13 +0200 Subject: [PATCH 07/13] fixed order of metrics in test output --- acceptance/bundle/paths/fallback_metric/output.txt | 8 ++++---- .../bundle/telemetry/deploy-compute-type/output.txt | 8 ++++---- .../bundle/telemetry/deploy-experimental/output.txt | 4 ++-- .../bundle/telemetry/deploy-name-prefix/custom/output.txt | 4 ++-- .../deploy-name-prefix/mode-development/output.txt | 4 ++-- .../bundle/telemetry/deploy-whl-artifacts/output.txt | 8 ++++---- acceptance/bundle/telemetry/deploy/out.telemetry.txt | 4 ++-- 7 files changed, 20 insertions(+), 20 deletions(-) diff --git a/acceptance/bundle/paths/fallback_metric/output.txt b/acceptance/bundle/paths/fallback_metric/output.txt index 6b8ef00e47..a98e275088 100644 --- a/acceptance/bundle/paths/fallback_metric/output.txt +++ b/acceptance/bundle/paths/fallback_metric/output.txt @@ -23,11 +23,11 @@ Deployment complete! "value": false }, { - "key": "skip_artifact_cleanup", + "key": "python_wheel_wrapper_is_set", "value": false }, { - "key": "python_wheel_wrapper_is_set", + "key": "skip_artifact_cleanup", "value": false }, { @@ -57,11 +57,11 @@ Deployment complete! "value": true }, { - "key": "skip_artifact_cleanup", + "key": "python_wheel_wrapper_is_set", "value": false }, { - "key": "python_wheel_wrapper_is_set", + "key": "skip_artifact_cleanup", "value": false }, { diff --git a/acceptance/bundle/telemetry/deploy-compute-type/output.txt b/acceptance/bundle/telemetry/deploy-compute-type/output.txt index 9cb418ea1d..0466986023 100644 --- a/acceptance/bundle/telemetry/deploy-compute-type/output.txt +++ b/acceptance/bundle/telemetry/deploy-compute-type/output.txt @@ -22,11 +22,11 @@ Deployment complete! "value": false }, { - "key": "skip_artifact_cleanup", + "key": "python_wheel_wrapper_is_set", "value": false }, { - "key": "python_wheel_wrapper_is_set", + "key": "skip_artifact_cleanup", "value": false }, { @@ -52,11 +52,11 @@ Deployment complete! "value": false }, { - "key": "skip_artifact_cleanup", + "key": "python_wheel_wrapper_is_set", "value": false }, { - "key": "python_wheel_wrapper_is_set", + "key": "skip_artifact_cleanup", "value": false }, { diff --git a/acceptance/bundle/telemetry/deploy-experimental/output.txt b/acceptance/bundle/telemetry/deploy-experimental/output.txt index e5e2ea254c..4a88862cd8 100644 --- a/acceptance/bundle/telemetry/deploy-experimental/output.txt +++ b/acceptance/bundle/telemetry/deploy-experimental/output.txt @@ -21,11 +21,11 @@ Deployment complete! "value": false }, { - "key": "skip_artifact_cleanup", + "key": "python_wheel_wrapper_is_set", "value": false }, { - "key": "python_wheel_wrapper_is_set", + "key": "skip_artifact_cleanup", "value": false }, { diff --git a/acceptance/bundle/telemetry/deploy-name-prefix/custom/output.txt b/acceptance/bundle/telemetry/deploy-name-prefix/custom/output.txt index 35c1c4070e..e79e408218 100644 --- a/acceptance/bundle/telemetry/deploy-name-prefix/custom/output.txt +++ b/acceptance/bundle/telemetry/deploy-name-prefix/custom/output.txt @@ -17,11 +17,11 @@ Deployment complete! "value": true }, { - "key": "skip_artifact_cleanup", + "key": "python_wheel_wrapper_is_set", "value": false }, { - "key": "python_wheel_wrapper_is_set", + "key": "skip_artifact_cleanup", "value": false }, { diff --git a/acceptance/bundle/telemetry/deploy-name-prefix/mode-development/output.txt b/acceptance/bundle/telemetry/deploy-name-prefix/mode-development/output.txt index c54466ab89..d28b22deea 100644 --- a/acceptance/bundle/telemetry/deploy-name-prefix/mode-development/output.txt +++ b/acceptance/bundle/telemetry/deploy-name-prefix/mode-development/output.txt @@ -17,11 +17,11 @@ Deployment complete! "value": true }, { - "key": "skip_artifact_cleanup", + "key": "python_wheel_wrapper_is_set", "value": false }, { - "key": "python_wheel_wrapper_is_set", + "key": "skip_artifact_cleanup", "value": false }, { diff --git a/acceptance/bundle/telemetry/deploy-whl-artifacts/output.txt b/acceptance/bundle/telemetry/deploy-whl-artifacts/output.txt index 941b1aa988..207ee71d24 100644 --- a/acceptance/bundle/telemetry/deploy-whl-artifacts/output.txt +++ b/acceptance/bundle/telemetry/deploy-whl-artifacts/output.txt @@ -25,11 +25,11 @@ Deployment complete! "value": false }, { - "key": "skip_artifact_cleanup", + "key": "python_wheel_wrapper_is_set", "value": false }, { - "key": "python_wheel_wrapper_is_set", + "key": "skip_artifact_cleanup", "value": false }, { @@ -61,11 +61,11 @@ Deployment complete! "value": true }, { - "key": "skip_artifact_cleanup", + "key": "python_wheel_wrapper_is_set", "value": true }, { - "key": "python_wheel_wrapper_is_set", + "key": "skip_artifact_cleanup", "value": true }, { diff --git a/acceptance/bundle/telemetry/deploy/out.telemetry.txt b/acceptance/bundle/telemetry/deploy/out.telemetry.txt index 8d4afad759..0c4a1d5d62 100644 --- a/acceptance/bundle/telemetry/deploy/out.telemetry.txt +++ b/acceptance/bundle/telemetry/deploy/out.telemetry.txt @@ -51,11 +51,11 @@ "value": false }, { - "key": "skip_artifact_cleanup", + "key": "python_wheel_wrapper_is_set", "value": false }, { - "key": "python_wheel_wrapper_is_set", + "key": "skip_artifact_cleanup", "value": false }, { From 74aa4345f14046205fdf7a1adb977388d56db272 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 25 Aug 2025 13:44:17 +0200 Subject: [PATCH 08/13] fixes + brought back tests --- acceptance/bundle/plan/no_upload/output.txt | 4 +- acceptance/bundle/plan/no_upload/script | 7 +- bundle/libraries/remote_path.go | 2 +- bundle/libraries/upload_test.go | 351 ++++++++++++++++++++ integration/bundle/artifacts_test.go | 238 +++++++++++++ 5 files changed, 593 insertions(+), 9 deletions(-) create mode 100644 bundle/libraries/upload_test.go create mode 100644 integration/bundle/artifacts_test.go diff --git a/acceptance/bundle/plan/no_upload/output.txt b/acceptance/bundle/plan/no_upload/output.txt index 1751f72207..8997a8b2f2 100644 --- a/acceptance/bundle/plan/no_upload/output.txt +++ b/acceptance/bundle/plan/no_upload/output.txt @@ -2,6 +2,4 @@ >>> [CLI] bundle plan create jobs.my_job ->>> jq -s .[] | select(.path | contains("api/2.0/workspace")) | select(.method != "GET") out.requests.txt - ->>> jq -s .[] | select(.path | contains("/api/2.0/workspace-files/import-file")) out.requests.txt +>>> jq -s .[] | select(.method != "GET") out.requests.txt diff --git a/acceptance/bundle/plan/no_upload/script b/acceptance/bundle/plan/no_upload/script index 203bf2dc23..02d44d657b 100644 --- a/acceptance/bundle/plan/no_upload/script +++ b/acceptance/bundle/plan/no_upload/script @@ -1,9 +1,6 @@ trace $CLI bundle plan -# Expect no clean up requests -trace jq -s '.[] | select(.path | contains("api/2.0/workspace")) | select(.method != "GET")' out.requests.txt - -# Expect no upload requests -trace jq -s '.[] | select(.path | contains("/api/2.0/workspace-files/import-file"))' out.requests.txt +# Expect no non-GET requests +trace jq -s '.[] | select(.method != "GET")' out.requests.txt rm out.requests.txt diff --git a/bundle/libraries/remote_path.go b/bundle/libraries/remote_path.go index 31efeb3e0c..4d0988e628 100644 --- a/bundle/libraries/remote_path.go +++ b/bundle/libraries/remote_path.go @@ -41,7 +41,7 @@ func ReplaceWithRemotePath(ctx context.Context, b *bundle.Bundle) (map[string][] for _, location := range locations { v, err = dyn.SetByPath(v, location.configPath, dyn.NewValue(remotePath, []dyn.Location{location.location})) if err != nil { - return v, err + return v, fmt.Errorf("internal error: failed to update path %#v to %#v: %w", source, remotePath, err) } } diff --git a/bundle/libraries/upload_test.go b/bundle/libraries/upload_test.go new file mode 100644 index 0000000000..93b7d39bcd --- /dev/null +++ b/bundle/libraries/upload_test.go @@ -0,0 +1,351 @@ +package libraries + +import ( + "context" + "path/filepath" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" + mockfiler "github.com/databricks/cli/internal/mocks/libs/filer" + "github.com/databricks/cli/internal/testutil" + "github.com/databricks/cli/libs/filer" + "github.com/databricks/databricks-sdk-go/service/compute" + "github.com/databricks/databricks-sdk-go/service/jobs" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func TestArtifactUploadForWorkspace(t *testing.T) { + tmpDir := t.TempDir() + whlFolder := filepath.Join(tmpDir, "whl") + testutil.Touch(t, whlFolder, "source.whl") + whlLocalPath := filepath.Join(whlFolder, "source.whl") + + b := &bundle.Bundle{ + SyncRootPath: tmpDir, + Config: config.Root{ + Workspace: config.Workspace{ + ArtifactPath: "/foo/bar/artifacts", + }, + Artifacts: config.Artifacts{ + "whl": { + Type: config.ArtifactPythonWheel, + Files: []config.ArtifactFile{ + {Source: whlLocalPath}, + }, + }, + }, + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job": { + JobSettings: jobs.JobSettings{ + Tasks: []jobs.Task{ + { + Libraries: []compute.Library{ + { + Whl: filepath.Join("whl", "*.whl"), + }, + { + Whl: "/Workspace/Users/foo@bar.com/mywheel.whl", + }, + }, + }, + { + ForEachTask: &jobs.ForEachTask{ + Task: jobs.Task{ + Libraries: []compute.Library{ + { + Whl: filepath.Join("whl", "*.whl"), + }, + { + Whl: "/Workspace/Users/foo@bar.com/mywheel.whl", + }, + }, + }, + }, + }, + }, + Environments: []jobs.JobEnvironment{ + { + Spec: &compute.Environment{ + Dependencies: []string{ + filepath.Join("whl", "source.whl"), + "/Workspace/Users/foo@bar.com/mywheel.whl", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + mockFiler := mockfiler.NewMockFiler(t) + mockFiler.EXPECT().Write( + mock.Anything, + filepath.Join("source.whl"), + mock.AnythingOfType("*os.File"), + filer.OverwriteIfExists, + filer.CreateParentDirectories, + ).Return(nil) + + diags := bundle.ApplySeq(context.Background(), b, ExpandGlobReferences()) + require.NoError(t, diags.Error()) + + libs, diags := ReplaceWithRemotePath(context.Background(), b) + require.NoError(t, diags.Error()) + diags = bundle.ApplySeq(context.Background(), b, UploadWithClient(libs, mockFiler)) + require.NoError(t, diags.Error()) + + // Test that libraries path is updated + require.Equal(t, "/Workspace/foo/bar/artifacts/.internal/source.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries[0].Whl) + require.Equal(t, "/Workspace/Users/foo@bar.com/mywheel.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries[1].Whl) + require.Equal(t, "/Workspace/foo/bar/artifacts/.internal/source.whl", b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies[0]) + require.Equal(t, "/Workspace/Users/foo@bar.com/mywheel.whl", b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies[1]) + require.Equal(t, "/Workspace/foo/bar/artifacts/.internal/source.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[1].ForEachTask.Task.Libraries[0].Whl) + require.Equal(t, "/Workspace/Users/foo@bar.com/mywheel.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[1].ForEachTask.Task.Libraries[1].Whl) +} + +func TestArtifactUploadForVolumes(t *testing.T) { + tmpDir := t.TempDir() + whlFolder := filepath.Join(tmpDir, "whl") + testutil.Touch(t, whlFolder, "source.whl") + whlLocalPath := filepath.Join(whlFolder, "source.whl") + + b := &bundle.Bundle{ + SyncRootPath: tmpDir, + Config: config.Root{ + Workspace: config.Workspace{ + ArtifactPath: "/Volumes/foo/bar/artifacts", + }, + Artifacts: config.Artifacts{ + "whl": { + Type: config.ArtifactPythonWheel, + Files: []config.ArtifactFile{ + {Source: whlLocalPath}, + }, + }, + }, + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job": { + JobSettings: jobs.JobSettings{ + Tasks: []jobs.Task{ + { + Libraries: []compute.Library{ + { + Whl: filepath.Join("whl", "*.whl"), + }, + { + Whl: "/Volumes/some/path/mywheel.whl", + }, + }, + }, + { + ForEachTask: &jobs.ForEachTask{ + Task: jobs.Task{ + Libraries: []compute.Library{ + { + Whl: filepath.Join("whl", "*.whl"), + }, + { + Whl: "/Volumes/some/path/mywheel.whl", + }, + }, + }, + }, + }, + }, + Environments: []jobs.JobEnvironment{ + { + Spec: &compute.Environment{ + Dependencies: []string{ + filepath.Join("whl", "source.whl"), + "/Volumes/some/path/mywheel.whl", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + mockFiler := mockfiler.NewMockFiler(t) + mockFiler.EXPECT().Write( + mock.Anything, + filepath.Join("source.whl"), + mock.AnythingOfType("*os.File"), + filer.OverwriteIfExists, + filer.CreateParentDirectories, + ).Return(nil) + + diags := bundle.ApplySeq(context.Background(), b, ExpandGlobReferences()) + require.NoError(t, diags.Error()) + + libs, diags := ReplaceWithRemotePath(context.Background(), b) + require.NoError(t, diags.Error()) + diags = bundle.ApplySeq(context.Background(), b, UploadWithClient(libs, mockFiler)) + require.NoError(t, diags.Error()) + + // Test that libraries path is updated + require.Equal(t, "/Volumes/foo/bar/artifacts/.internal/source.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries[0].Whl) + require.Equal(t, "/Volumes/some/path/mywheel.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries[1].Whl) + require.Equal(t, "/Volumes/foo/bar/artifacts/.internal/source.whl", b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies[0]) + require.Equal(t, "/Volumes/some/path/mywheel.whl", b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies[1]) + require.Equal(t, "/Volumes/foo/bar/artifacts/.internal/source.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[1].ForEachTask.Task.Libraries[0].Whl) + require.Equal(t, "/Volumes/some/path/mywheel.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[1].ForEachTask.Task.Libraries[1].Whl) +} + +func TestArtifactUploadWithNoLibraryReference(t *testing.T) { + tmpDir := t.TempDir() + whlFolder := filepath.Join(tmpDir, "whl") + testutil.Touch(t, whlFolder, "source.whl") + whlLocalPath := filepath.Join(whlFolder, "source.whl") + + b := &bundle.Bundle{ + SyncRootPath: tmpDir, + Config: config.Root{ + Workspace: config.Workspace{ + ArtifactPath: "/Workspace/foo/bar/artifacts", + }, + Artifacts: config.Artifacts{ + "whl": { + Type: config.ArtifactPythonWheel, + Files: []config.ArtifactFile{ + {Source: whlLocalPath}, + }, + }, + }, + }, + } + + mockFiler := mockfiler.NewMockFiler(t) + mockFiler.EXPECT().Write( + mock.Anything, + filepath.Join("source.whl"), + mock.AnythingOfType("*os.File"), + filer.OverwriteIfExists, + filer.CreateParentDirectories, + ).Return(nil) + + diags := bundle.ApplySeq(context.Background(), b, ExpandGlobReferences()) + require.NoError(t, diags.Error()) + + libs, diags := ReplaceWithRemotePath(context.Background(), b) + require.NoError(t, diags.Error()) + diags = bundle.ApplySeq(context.Background(), b, UploadWithClient(libs, mockFiler)) + require.NoError(t, diags.Error()) + + require.Equal(t, "/Workspace/foo/bar/artifacts/.internal/source.whl", b.Config.Artifacts["whl"].Files[0].RemotePath) +} + +func TestUploadMultipleLibraries(t *testing.T) { + tmpDir := t.TempDir() + whlFolder := filepath.Join(tmpDir, "whl") + testutil.Touch(t, whlFolder, "source1.whl") + testutil.Touch(t, whlFolder, "source2.whl") + testutil.Touch(t, whlFolder, "source3.whl") + testutil.Touch(t, whlFolder, "source4.whl") + + b := &bundle.Bundle{ + SyncRootPath: tmpDir, + Config: config.Root{ + Workspace: config.Workspace{ + ArtifactPath: "/foo/bar/artifacts", + }, + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job": { + JobSettings: jobs.JobSettings{ + Tasks: []jobs.Task{ + { + Libraries: []compute.Library{ + { + Whl: filepath.Join("whl", "*.whl"), + }, + { + Whl: "/Workspace/Users/foo@bar.com/mywheel.whl", + }, + }, + }, + }, + Environments: []jobs.JobEnvironment{ + { + Spec: &compute.Environment{ + Dependencies: []string{ + filepath.Join("whl", "*.whl"), + "/Workspace/Users/foo@bar.com/mywheel.whl", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + mockFiler := mockfiler.NewMockFiler(t) + mockFiler.EXPECT().Write( + mock.Anything, + filepath.Join("source1.whl"), + mock.AnythingOfType("*os.File"), + filer.OverwriteIfExists, + filer.CreateParentDirectories, + ).Return(nil).Once() + + mockFiler.EXPECT().Write( + mock.Anything, + filepath.Join("source2.whl"), + mock.AnythingOfType("*os.File"), + filer.OverwriteIfExists, + filer.CreateParentDirectories, + ).Return(nil).Once() + + mockFiler.EXPECT().Write( + mock.Anything, + filepath.Join("source3.whl"), + mock.AnythingOfType("*os.File"), + filer.OverwriteIfExists, + filer.CreateParentDirectories, + ).Return(nil).Once() + + mockFiler.EXPECT().Write( + mock.Anything, + filepath.Join("source4.whl"), + mock.AnythingOfType("*os.File"), + filer.OverwriteIfExists, + filer.CreateParentDirectories, + ).Return(nil).Once() + + diags := bundle.ApplySeq(context.Background(), b, ExpandGlobReferences()) + require.NoError(t, diags.Error()) + + libs, diags := ReplaceWithRemotePath(context.Background(), b) + require.NoError(t, diags.Error()) + diags = bundle.ApplySeq(context.Background(), b, UploadWithClient(libs, mockFiler)) + require.NoError(t, diags.Error()) + + // Test that libraries path is updated + require.Len(t, b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries, 5) + require.Contains(t, b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries, compute.Library{Whl: "/Workspace/foo/bar/artifacts/.internal/source1.whl"}) + require.Contains(t, b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries, compute.Library{Whl: "/Workspace/foo/bar/artifacts/.internal/source2.whl"}) + require.Contains(t, b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries, compute.Library{Whl: "/Workspace/foo/bar/artifacts/.internal/source3.whl"}) + require.Contains(t, b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries, compute.Library{Whl: "/Workspace/foo/bar/artifacts/.internal/source4.whl"}) + require.Contains(t, b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries, compute.Library{Whl: "/Workspace/Users/foo@bar.com/mywheel.whl"}) + + require.Len(t, b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies, 5) + require.Contains(t, b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies, "/Workspace/foo/bar/artifacts/.internal/source1.whl") + require.Contains(t, b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies, "/Workspace/foo/bar/artifacts/.internal/source2.whl") + require.Contains(t, b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies, "/Workspace/foo/bar/artifacts/.internal/source3.whl") + require.Contains(t, b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies, "/Workspace/foo/bar/artifacts/.internal/source4.whl") + require.Contains(t, b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies, "/Workspace/Users/foo@bar.com/mywheel.whl") +} diff --git a/integration/bundle/artifacts_test.go b/integration/bundle/artifacts_test.go new file mode 100644 index 0000000000..58f14693b1 --- /dev/null +++ b/integration/bundle/artifacts_test.go @@ -0,0 +1,238 @@ +package bundle_test + +import ( + "os" + "path" + "path/filepath" + "regexp" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/libraries" + "github.com/databricks/cli/integration/internal/acc" + "github.com/databricks/databricks-sdk-go/service/compute" + "github.com/databricks/databricks-sdk-go/service/jobs" + "github.com/stretchr/testify/require" +) + +func touchEmptyFile(t *testing.T, path string) { + err := os.MkdirAll(filepath.Dir(path), 0o700) + require.NoError(t, err) + f, err := os.Create(path) + require.NoError(t, err) + f.Close() +} + +func TestUploadArtifactFileToCorrectRemotePath(t *testing.T) { + ctx, wt := acc.WorkspaceTest(t) + dir := t.TempDir() + whlPath := filepath.Join(dir, "dist", "test.whl") + touchEmptyFile(t, whlPath) + + wsDir := acc.TemporaryWorkspaceDir(wt, "artifact-") + + b := &bundle.Bundle{ + BundleRootPath: dir, + SyncRootPath: dir, + Config: config.Root{ + Bundle: config.Bundle{ + Target: "whatever", + }, + Workspace: config.Workspace{ + ArtifactPath: wsDir, + }, + Artifacts: config.Artifacts{ + "test": &config.Artifact{ + Type: "whl", + Files: []config.ArtifactFile{ + { + Source: whlPath, + }, + }, + }, + }, + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "test": { + JobSettings: jobs.JobSettings{ + Tasks: []jobs.Task{ + { + Libraries: []compute.Library{ + { + Whl: "dist/test.whl", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + diags := bundle.ApplySeq(ctx, b, libraries.ExpandGlobReferences()) + require.NoError(t, diags.Error()) + + libs, diags := libraries.ReplaceWithRemotePath(ctx, b) + require.NoError(t, diags.Error()) + diags = bundle.ApplySeq(ctx, b, libraries.Upload(libs)) + require.NoError(t, diags.Error()) + + // The remote path attribute on the artifact file should have been set. + require.Regexp(t, + path.Join(regexp.QuoteMeta(wsDir), `.internal/test\.whl`), + b.Config.Artifacts["test"].Files[0].RemotePath, + ) + + // The task library path should have been updated to the remote path. + require.Regexp(t, + path.Join("/Workspace", regexp.QuoteMeta(wsDir), `.internal/test\.whl`), + b.Config.Resources.Jobs["test"].JobSettings.Tasks[0].Libraries[0].Whl, + ) +} + +func TestUploadArtifactFileToCorrectRemotePathWithEnvironments(t *testing.T) { + ctx, wt := acc.WorkspaceTest(t) + dir := t.TempDir() + whlPath := filepath.Join(dir, "dist", "test.whl") + touchEmptyFile(t, whlPath) + + wsDir := acc.TemporaryWorkspaceDir(wt, "artifact-") + + b := &bundle.Bundle{ + BundleRootPath: dir, + SyncRootPath: dir, + Config: config.Root{ + Bundle: config.Bundle{ + Target: "whatever", + }, + Workspace: config.Workspace{ + ArtifactPath: wsDir, + }, + Artifacts: config.Artifacts{ + "test": &config.Artifact{ + Type: "whl", + Files: []config.ArtifactFile{ + { + Source: whlPath, + }, + }, + }, + }, + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "test": { + JobSettings: jobs.JobSettings{ + Environments: []jobs.JobEnvironment{ + { + Spec: &compute.Environment{ + Dependencies: []string{ + "dist/test.whl", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + diags := bundle.ApplySeq(ctx, b, libraries.ExpandGlobReferences()) + require.NoError(t, diags.Error()) + + libs, diags := libraries.ReplaceWithRemotePath(ctx, b) + require.NoError(t, diags.Error()) + diags = bundle.ApplySeq(ctx, b, libraries.Upload(libs)) + require.NoError(t, diags.Error()) + + // The remote path attribute on the artifact file should have been set. + require.Regexp(t, + path.Join(regexp.QuoteMeta(wsDir), `.internal/test\.whl`), + b.Config.Artifacts["test"].Files[0].RemotePath, + ) + + // The job environment deps path should have been updated to the remote path. + require.Regexp(t, + path.Join("/Workspace", regexp.QuoteMeta(wsDir), `.internal/test\.whl`), + b.Config.Resources.Jobs["test"].Environments[0].Spec.Dependencies[0], + ) +} + +func TestUploadArtifactFileToCorrectRemotePathForVolumes(t *testing.T) { + ctx, wt := acc.WorkspaceTest(t) + + if os.Getenv("TEST_METASTORE_ID") == "" { + t.Skip("Skipping tests that require a UC Volume when metastore id is not set.") + } + + volumePath := acc.TemporaryVolume(wt) + + dir := t.TempDir() + whlPath := filepath.Join(dir, "dist", "test.whl") + touchEmptyFile(t, whlPath) + + b := &bundle.Bundle{ + BundleRootPath: dir, + SyncRootPath: dir, + Config: config.Root{ + Bundle: config.Bundle{ + Target: "whatever", + }, + Workspace: config.Workspace{ + ArtifactPath: volumePath, + }, + Artifacts: config.Artifacts{ + "test": &config.Artifact{ + Type: "whl", + Files: []config.ArtifactFile{ + { + Source: whlPath, + }, + }, + }, + }, + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "test": { + JobSettings: jobs.JobSettings{ + Tasks: []jobs.Task{ + { + Libraries: []compute.Library{ + { + Whl: "dist/test.whl", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + diags := bundle.ApplySeq(ctx, b, libraries.ExpandGlobReferences()) + require.NoError(t, diags.Error()) + + libs, diags := libraries.ReplaceWithRemotePath(ctx, b) + require.NoError(t, diags.Error()) + diags = bundle.ApplySeq(ctx, b, libraries.Upload(libs)) + require.NoError(t, diags.Error()) + + // The remote path attribute on the artifact file should have been set. + require.Regexp(t, + path.Join(regexp.QuoteMeta(volumePath), `.internal/test\.whl`), + b.Config.Artifacts["test"].Files[0].RemotePath, + ) + + // The task library path should have been updated to the remote path. + require.Regexp(t, + path.Join(regexp.QuoteMeta(volumePath), `.internal/test\.whl`), + b.Config.Resources.Jobs["test"].JobSettings.Tasks[0].Libraries[0].Whl, + ) +} From 3a8e018ba48e80fd7c0faee1309b436e6b0d6f84 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 26 Aug 2025 12:24:49 +0200 Subject: [PATCH 09/13] remove old tests --- bundle/libraries/upload_test.go | 351 --------------------------- integration/bundle/artifacts_test.go | 238 ------------------ 2 files changed, 589 deletions(-) delete mode 100644 bundle/libraries/upload_test.go delete mode 100644 integration/bundle/artifacts_test.go diff --git a/bundle/libraries/upload_test.go b/bundle/libraries/upload_test.go deleted file mode 100644 index 93b7d39bcd..0000000000 --- a/bundle/libraries/upload_test.go +++ /dev/null @@ -1,351 +0,0 @@ -package libraries - -import ( - "context" - "path/filepath" - "testing" - - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config" - "github.com/databricks/cli/bundle/config/resources" - mockfiler "github.com/databricks/cli/internal/mocks/libs/filer" - "github.com/databricks/cli/internal/testutil" - "github.com/databricks/cli/libs/filer" - "github.com/databricks/databricks-sdk-go/service/compute" - "github.com/databricks/databricks-sdk-go/service/jobs" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" -) - -func TestArtifactUploadForWorkspace(t *testing.T) { - tmpDir := t.TempDir() - whlFolder := filepath.Join(tmpDir, "whl") - testutil.Touch(t, whlFolder, "source.whl") - whlLocalPath := filepath.Join(whlFolder, "source.whl") - - b := &bundle.Bundle{ - SyncRootPath: tmpDir, - Config: config.Root{ - Workspace: config.Workspace{ - ArtifactPath: "/foo/bar/artifacts", - }, - Artifacts: config.Artifacts{ - "whl": { - Type: config.ArtifactPythonWheel, - Files: []config.ArtifactFile{ - {Source: whlLocalPath}, - }, - }, - }, - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "job": { - JobSettings: jobs.JobSettings{ - Tasks: []jobs.Task{ - { - Libraries: []compute.Library{ - { - Whl: filepath.Join("whl", "*.whl"), - }, - { - Whl: "/Workspace/Users/foo@bar.com/mywheel.whl", - }, - }, - }, - { - ForEachTask: &jobs.ForEachTask{ - Task: jobs.Task{ - Libraries: []compute.Library{ - { - Whl: filepath.Join("whl", "*.whl"), - }, - { - Whl: "/Workspace/Users/foo@bar.com/mywheel.whl", - }, - }, - }, - }, - }, - }, - Environments: []jobs.JobEnvironment{ - { - Spec: &compute.Environment{ - Dependencies: []string{ - filepath.Join("whl", "source.whl"), - "/Workspace/Users/foo@bar.com/mywheel.whl", - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - - mockFiler := mockfiler.NewMockFiler(t) - mockFiler.EXPECT().Write( - mock.Anything, - filepath.Join("source.whl"), - mock.AnythingOfType("*os.File"), - filer.OverwriteIfExists, - filer.CreateParentDirectories, - ).Return(nil) - - diags := bundle.ApplySeq(context.Background(), b, ExpandGlobReferences()) - require.NoError(t, diags.Error()) - - libs, diags := ReplaceWithRemotePath(context.Background(), b) - require.NoError(t, diags.Error()) - diags = bundle.ApplySeq(context.Background(), b, UploadWithClient(libs, mockFiler)) - require.NoError(t, diags.Error()) - - // Test that libraries path is updated - require.Equal(t, "/Workspace/foo/bar/artifacts/.internal/source.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries[0].Whl) - require.Equal(t, "/Workspace/Users/foo@bar.com/mywheel.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries[1].Whl) - require.Equal(t, "/Workspace/foo/bar/artifacts/.internal/source.whl", b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies[0]) - require.Equal(t, "/Workspace/Users/foo@bar.com/mywheel.whl", b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies[1]) - require.Equal(t, "/Workspace/foo/bar/artifacts/.internal/source.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[1].ForEachTask.Task.Libraries[0].Whl) - require.Equal(t, "/Workspace/Users/foo@bar.com/mywheel.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[1].ForEachTask.Task.Libraries[1].Whl) -} - -func TestArtifactUploadForVolumes(t *testing.T) { - tmpDir := t.TempDir() - whlFolder := filepath.Join(tmpDir, "whl") - testutil.Touch(t, whlFolder, "source.whl") - whlLocalPath := filepath.Join(whlFolder, "source.whl") - - b := &bundle.Bundle{ - SyncRootPath: tmpDir, - Config: config.Root{ - Workspace: config.Workspace{ - ArtifactPath: "/Volumes/foo/bar/artifacts", - }, - Artifacts: config.Artifacts{ - "whl": { - Type: config.ArtifactPythonWheel, - Files: []config.ArtifactFile{ - {Source: whlLocalPath}, - }, - }, - }, - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "job": { - JobSettings: jobs.JobSettings{ - Tasks: []jobs.Task{ - { - Libraries: []compute.Library{ - { - Whl: filepath.Join("whl", "*.whl"), - }, - { - Whl: "/Volumes/some/path/mywheel.whl", - }, - }, - }, - { - ForEachTask: &jobs.ForEachTask{ - Task: jobs.Task{ - Libraries: []compute.Library{ - { - Whl: filepath.Join("whl", "*.whl"), - }, - { - Whl: "/Volumes/some/path/mywheel.whl", - }, - }, - }, - }, - }, - }, - Environments: []jobs.JobEnvironment{ - { - Spec: &compute.Environment{ - Dependencies: []string{ - filepath.Join("whl", "source.whl"), - "/Volumes/some/path/mywheel.whl", - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - - mockFiler := mockfiler.NewMockFiler(t) - mockFiler.EXPECT().Write( - mock.Anything, - filepath.Join("source.whl"), - mock.AnythingOfType("*os.File"), - filer.OverwriteIfExists, - filer.CreateParentDirectories, - ).Return(nil) - - diags := bundle.ApplySeq(context.Background(), b, ExpandGlobReferences()) - require.NoError(t, diags.Error()) - - libs, diags := ReplaceWithRemotePath(context.Background(), b) - require.NoError(t, diags.Error()) - diags = bundle.ApplySeq(context.Background(), b, UploadWithClient(libs, mockFiler)) - require.NoError(t, diags.Error()) - - // Test that libraries path is updated - require.Equal(t, "/Volumes/foo/bar/artifacts/.internal/source.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries[0].Whl) - require.Equal(t, "/Volumes/some/path/mywheel.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries[1].Whl) - require.Equal(t, "/Volumes/foo/bar/artifacts/.internal/source.whl", b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies[0]) - require.Equal(t, "/Volumes/some/path/mywheel.whl", b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies[1]) - require.Equal(t, "/Volumes/foo/bar/artifacts/.internal/source.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[1].ForEachTask.Task.Libraries[0].Whl) - require.Equal(t, "/Volumes/some/path/mywheel.whl", b.Config.Resources.Jobs["job"].JobSettings.Tasks[1].ForEachTask.Task.Libraries[1].Whl) -} - -func TestArtifactUploadWithNoLibraryReference(t *testing.T) { - tmpDir := t.TempDir() - whlFolder := filepath.Join(tmpDir, "whl") - testutil.Touch(t, whlFolder, "source.whl") - whlLocalPath := filepath.Join(whlFolder, "source.whl") - - b := &bundle.Bundle{ - SyncRootPath: tmpDir, - Config: config.Root{ - Workspace: config.Workspace{ - ArtifactPath: "/Workspace/foo/bar/artifacts", - }, - Artifacts: config.Artifacts{ - "whl": { - Type: config.ArtifactPythonWheel, - Files: []config.ArtifactFile{ - {Source: whlLocalPath}, - }, - }, - }, - }, - } - - mockFiler := mockfiler.NewMockFiler(t) - mockFiler.EXPECT().Write( - mock.Anything, - filepath.Join("source.whl"), - mock.AnythingOfType("*os.File"), - filer.OverwriteIfExists, - filer.CreateParentDirectories, - ).Return(nil) - - diags := bundle.ApplySeq(context.Background(), b, ExpandGlobReferences()) - require.NoError(t, diags.Error()) - - libs, diags := ReplaceWithRemotePath(context.Background(), b) - require.NoError(t, diags.Error()) - diags = bundle.ApplySeq(context.Background(), b, UploadWithClient(libs, mockFiler)) - require.NoError(t, diags.Error()) - - require.Equal(t, "/Workspace/foo/bar/artifacts/.internal/source.whl", b.Config.Artifacts["whl"].Files[0].RemotePath) -} - -func TestUploadMultipleLibraries(t *testing.T) { - tmpDir := t.TempDir() - whlFolder := filepath.Join(tmpDir, "whl") - testutil.Touch(t, whlFolder, "source1.whl") - testutil.Touch(t, whlFolder, "source2.whl") - testutil.Touch(t, whlFolder, "source3.whl") - testutil.Touch(t, whlFolder, "source4.whl") - - b := &bundle.Bundle{ - SyncRootPath: tmpDir, - Config: config.Root{ - Workspace: config.Workspace{ - ArtifactPath: "/foo/bar/artifacts", - }, - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "job": { - JobSettings: jobs.JobSettings{ - Tasks: []jobs.Task{ - { - Libraries: []compute.Library{ - { - Whl: filepath.Join("whl", "*.whl"), - }, - { - Whl: "/Workspace/Users/foo@bar.com/mywheel.whl", - }, - }, - }, - }, - Environments: []jobs.JobEnvironment{ - { - Spec: &compute.Environment{ - Dependencies: []string{ - filepath.Join("whl", "*.whl"), - "/Workspace/Users/foo@bar.com/mywheel.whl", - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - - mockFiler := mockfiler.NewMockFiler(t) - mockFiler.EXPECT().Write( - mock.Anything, - filepath.Join("source1.whl"), - mock.AnythingOfType("*os.File"), - filer.OverwriteIfExists, - filer.CreateParentDirectories, - ).Return(nil).Once() - - mockFiler.EXPECT().Write( - mock.Anything, - filepath.Join("source2.whl"), - mock.AnythingOfType("*os.File"), - filer.OverwriteIfExists, - filer.CreateParentDirectories, - ).Return(nil).Once() - - mockFiler.EXPECT().Write( - mock.Anything, - filepath.Join("source3.whl"), - mock.AnythingOfType("*os.File"), - filer.OverwriteIfExists, - filer.CreateParentDirectories, - ).Return(nil).Once() - - mockFiler.EXPECT().Write( - mock.Anything, - filepath.Join("source4.whl"), - mock.AnythingOfType("*os.File"), - filer.OverwriteIfExists, - filer.CreateParentDirectories, - ).Return(nil).Once() - - diags := bundle.ApplySeq(context.Background(), b, ExpandGlobReferences()) - require.NoError(t, diags.Error()) - - libs, diags := ReplaceWithRemotePath(context.Background(), b) - require.NoError(t, diags.Error()) - diags = bundle.ApplySeq(context.Background(), b, UploadWithClient(libs, mockFiler)) - require.NoError(t, diags.Error()) - - // Test that libraries path is updated - require.Len(t, b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries, 5) - require.Contains(t, b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries, compute.Library{Whl: "/Workspace/foo/bar/artifacts/.internal/source1.whl"}) - require.Contains(t, b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries, compute.Library{Whl: "/Workspace/foo/bar/artifacts/.internal/source2.whl"}) - require.Contains(t, b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries, compute.Library{Whl: "/Workspace/foo/bar/artifacts/.internal/source3.whl"}) - require.Contains(t, b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries, compute.Library{Whl: "/Workspace/foo/bar/artifacts/.internal/source4.whl"}) - require.Contains(t, b.Config.Resources.Jobs["job"].JobSettings.Tasks[0].Libraries, compute.Library{Whl: "/Workspace/Users/foo@bar.com/mywheel.whl"}) - - require.Len(t, b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies, 5) - require.Contains(t, b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies, "/Workspace/foo/bar/artifacts/.internal/source1.whl") - require.Contains(t, b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies, "/Workspace/foo/bar/artifacts/.internal/source2.whl") - require.Contains(t, b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies, "/Workspace/foo/bar/artifacts/.internal/source3.whl") - require.Contains(t, b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies, "/Workspace/foo/bar/artifacts/.internal/source4.whl") - require.Contains(t, b.Config.Resources.Jobs["job"].Environments[0].Spec.Dependencies, "/Workspace/Users/foo@bar.com/mywheel.whl") -} diff --git a/integration/bundle/artifacts_test.go b/integration/bundle/artifacts_test.go deleted file mode 100644 index 58f14693b1..0000000000 --- a/integration/bundle/artifacts_test.go +++ /dev/null @@ -1,238 +0,0 @@ -package bundle_test - -import ( - "os" - "path" - "path/filepath" - "regexp" - "testing" - - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config" - "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/bundle/libraries" - "github.com/databricks/cli/integration/internal/acc" - "github.com/databricks/databricks-sdk-go/service/compute" - "github.com/databricks/databricks-sdk-go/service/jobs" - "github.com/stretchr/testify/require" -) - -func touchEmptyFile(t *testing.T, path string) { - err := os.MkdirAll(filepath.Dir(path), 0o700) - require.NoError(t, err) - f, err := os.Create(path) - require.NoError(t, err) - f.Close() -} - -func TestUploadArtifactFileToCorrectRemotePath(t *testing.T) { - ctx, wt := acc.WorkspaceTest(t) - dir := t.TempDir() - whlPath := filepath.Join(dir, "dist", "test.whl") - touchEmptyFile(t, whlPath) - - wsDir := acc.TemporaryWorkspaceDir(wt, "artifact-") - - b := &bundle.Bundle{ - BundleRootPath: dir, - SyncRootPath: dir, - Config: config.Root{ - Bundle: config.Bundle{ - Target: "whatever", - }, - Workspace: config.Workspace{ - ArtifactPath: wsDir, - }, - Artifacts: config.Artifacts{ - "test": &config.Artifact{ - Type: "whl", - Files: []config.ArtifactFile{ - { - Source: whlPath, - }, - }, - }, - }, - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "test": { - JobSettings: jobs.JobSettings{ - Tasks: []jobs.Task{ - { - Libraries: []compute.Library{ - { - Whl: "dist/test.whl", - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - - diags := bundle.ApplySeq(ctx, b, libraries.ExpandGlobReferences()) - require.NoError(t, diags.Error()) - - libs, diags := libraries.ReplaceWithRemotePath(ctx, b) - require.NoError(t, diags.Error()) - diags = bundle.ApplySeq(ctx, b, libraries.Upload(libs)) - require.NoError(t, diags.Error()) - - // The remote path attribute on the artifact file should have been set. - require.Regexp(t, - path.Join(regexp.QuoteMeta(wsDir), `.internal/test\.whl`), - b.Config.Artifacts["test"].Files[0].RemotePath, - ) - - // The task library path should have been updated to the remote path. - require.Regexp(t, - path.Join("/Workspace", regexp.QuoteMeta(wsDir), `.internal/test\.whl`), - b.Config.Resources.Jobs["test"].JobSettings.Tasks[0].Libraries[0].Whl, - ) -} - -func TestUploadArtifactFileToCorrectRemotePathWithEnvironments(t *testing.T) { - ctx, wt := acc.WorkspaceTest(t) - dir := t.TempDir() - whlPath := filepath.Join(dir, "dist", "test.whl") - touchEmptyFile(t, whlPath) - - wsDir := acc.TemporaryWorkspaceDir(wt, "artifact-") - - b := &bundle.Bundle{ - BundleRootPath: dir, - SyncRootPath: dir, - Config: config.Root{ - Bundle: config.Bundle{ - Target: "whatever", - }, - Workspace: config.Workspace{ - ArtifactPath: wsDir, - }, - Artifacts: config.Artifacts{ - "test": &config.Artifact{ - Type: "whl", - Files: []config.ArtifactFile{ - { - Source: whlPath, - }, - }, - }, - }, - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "test": { - JobSettings: jobs.JobSettings{ - Environments: []jobs.JobEnvironment{ - { - Spec: &compute.Environment{ - Dependencies: []string{ - "dist/test.whl", - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - - diags := bundle.ApplySeq(ctx, b, libraries.ExpandGlobReferences()) - require.NoError(t, diags.Error()) - - libs, diags := libraries.ReplaceWithRemotePath(ctx, b) - require.NoError(t, diags.Error()) - diags = bundle.ApplySeq(ctx, b, libraries.Upload(libs)) - require.NoError(t, diags.Error()) - - // The remote path attribute on the artifact file should have been set. - require.Regexp(t, - path.Join(regexp.QuoteMeta(wsDir), `.internal/test\.whl`), - b.Config.Artifacts["test"].Files[0].RemotePath, - ) - - // The job environment deps path should have been updated to the remote path. - require.Regexp(t, - path.Join("/Workspace", regexp.QuoteMeta(wsDir), `.internal/test\.whl`), - b.Config.Resources.Jobs["test"].Environments[0].Spec.Dependencies[0], - ) -} - -func TestUploadArtifactFileToCorrectRemotePathForVolumes(t *testing.T) { - ctx, wt := acc.WorkspaceTest(t) - - if os.Getenv("TEST_METASTORE_ID") == "" { - t.Skip("Skipping tests that require a UC Volume when metastore id is not set.") - } - - volumePath := acc.TemporaryVolume(wt) - - dir := t.TempDir() - whlPath := filepath.Join(dir, "dist", "test.whl") - touchEmptyFile(t, whlPath) - - b := &bundle.Bundle{ - BundleRootPath: dir, - SyncRootPath: dir, - Config: config.Root{ - Bundle: config.Bundle{ - Target: "whatever", - }, - Workspace: config.Workspace{ - ArtifactPath: volumePath, - }, - Artifacts: config.Artifacts{ - "test": &config.Artifact{ - Type: "whl", - Files: []config.ArtifactFile{ - { - Source: whlPath, - }, - }, - }, - }, - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "test": { - JobSettings: jobs.JobSettings{ - Tasks: []jobs.Task{ - { - Libraries: []compute.Library{ - { - Whl: "dist/test.whl", - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - - diags := bundle.ApplySeq(ctx, b, libraries.ExpandGlobReferences()) - require.NoError(t, diags.Error()) - - libs, diags := libraries.ReplaceWithRemotePath(ctx, b) - require.NoError(t, diags.Error()) - diags = bundle.ApplySeq(ctx, b, libraries.Upload(libs)) - require.NoError(t, diags.Error()) - - // The remote path attribute on the artifact file should have been set. - require.Regexp(t, - path.Join(regexp.QuoteMeta(volumePath), `.internal/test\.whl`), - b.Config.Artifacts["test"].Files[0].RemotePath, - ) - - // The task library path should have been updated to the remote path. - require.Regexp(t, - path.Join(regexp.QuoteMeta(volumePath), `.internal/test\.whl`), - b.Config.Resources.Jobs["test"].JobSettings.Tasks[0].Libraries[0].Whl, - ) -} From 7fdf47a6363281e8ed3142deac686cf25123fbbf Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 1 Sep 2025 11:25:59 +0200 Subject: [PATCH 10/13] fixes --- acceptance/bundle/plan/no_upload/.gitignore | 1 - .../bundle/plan/no_upload/databricks.yml | 17 ++++--- bundle/libraries/filer.go | 16 ++++++- bundle/libraries/remote_path.go | 33 ++++++------- bundle/libraries/upload.go | 8 ++-- bundle/phases/deploy.go | 37 +------------- bundle/phases/plan.go | 48 +++++++++++++++++++ 7 files changed, 89 insertions(+), 71 deletions(-) delete mode 100644 acceptance/bundle/plan/no_upload/.gitignore create mode 100644 bundle/phases/plan.go diff --git a/acceptance/bundle/plan/no_upload/.gitignore b/acceptance/bundle/plan/no_upload/.gitignore deleted file mode 100644 index 15bcc6dd09..0000000000 --- a/acceptance/bundle/plan/no_upload/.gitignore +++ /dev/null @@ -1 +0,0 @@ -.databricks diff --git a/acceptance/bundle/plan/no_upload/databricks.yml b/acceptance/bundle/plan/no_upload/databricks.yml index 9b036d72f1..76eed50823 100644 --- a/acceptance/bundle/plan/no_upload/databricks.yml +++ b/acceptance/bundle/plan/no_upload/databricks.yml @@ -9,12 +9,11 @@ resources: - task_key: task1 spark_python_task: python_file: "./my_script.py" - new_cluster: - num_workers: 1 - spark_version: 13.3.x-scala2.12 - node_type_id: i3.xlarge - driver_node_type_id: i3.xlarge - custom_tags: - "ResourceClass": "SingleNode" - libraries: - - whl: "*.whl" + environment_key: "env" + + environments: + - environment_key: "env" + spec: + client: "1" + dependencies: + - "*.whl" diff --git a/bundle/libraries/filer.go b/bundle/libraries/filer.go index 73ca442735..a2b75d1a9e 100644 --- a/bundle/libraries/filer.go +++ b/bundle/libraries/filer.go @@ -3,6 +3,7 @@ package libraries import ( "context" "path" + "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/diag" @@ -24,6 +25,7 @@ func GetFilerForLibraries(ctx context.Context, b *bundle.Bundle) (filer.Filer, s } uploadPath := path.Join(b.Config.Workspace.ArtifactPath, InternalDirName) + uploadPath = ensureWorkspaceOrVolumesPrefix(uploadPath) switch { case IsVolumesPath(artifactPath): @@ -40,11 +42,21 @@ func GetFilerForLibrariesCleanup(ctx context.Context, b *bundle.Bundle) (filer.F return nil, "", diag.Errorf("remote artifact path not configured") } + artifactPath = ensureWorkspaceOrVolumesPrefix(artifactPath) + switch { case IsVolumesPath(artifactPath): - return filerForVolume(b, b.Config.Workspace.ArtifactPath) + return filerForVolume(b, artifactPath) default: - return filerForWorkspace(b, b.Config.Workspace.ArtifactPath) + return filerForWorkspace(b, artifactPath) + } +} + +// If the remote path does not start with /Workspace or /Volumes, prepend /Workspace +func ensureWorkspaceOrVolumesPrefix(path string) string { + if !strings.HasPrefix(path, "/Workspace") && !strings.HasPrefix(path, "/Volumes") { + path = "/Workspace" + path } + return path } diff --git a/bundle/libraries/remote_path.go b/bundle/libraries/remote_path.go index 4d0988e628..d24387653a 100644 --- a/bundle/libraries/remote_path.go +++ b/bundle/libraries/remote_path.go @@ -5,7 +5,6 @@ import ( "fmt" "path" "path/filepath" - "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/diag" @@ -15,7 +14,7 @@ import ( // ReplaceWithRemotePath updates all the libraries paths to point to the remote location // where the libraries will be uploaded later. -func ReplaceWithRemotePath(ctx context.Context, b *bundle.Bundle) (map[string][]ConfigLocation, diag.Diagnostics) { +func ReplaceWithRemotePath(ctx context.Context, b *bundle.Bundle) (map[string][]LocationToUpdate, diag.Diagnostics) { _, uploadPath, diags := GetFilerForLibraries(ctx, b) if diags.HasError() { return nil, diags @@ -29,27 +28,23 @@ func ReplaceWithRemotePath(ctx context.Context, b *bundle.Bundle) (map[string][] sources := utils.SortedKeys(libs) // Update all the config paths to point to the uploaded location - for _, source := range sources { - locations := libs[source] - err = b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + err = b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + for _, source := range sources { + locations := libs[source] remotePath := path.Join(uploadPath, filepath.Base(source)) - // If the remote path does not start with /Workspace or /Volumes, prepend /Workspace - if !strings.HasPrefix(remotePath, "/Workspace") && !strings.HasPrefix(remotePath, "/Volumes") { - remotePath = "/Workspace" + remotePath - } for _, location := range locations { v, err = dyn.SetByPath(v, location.configPath, dyn.NewValue(remotePath, []dyn.Location{location.location})) if err != nil { return v, fmt.Errorf("internal error: failed to update path %#v to %#v: %w", source, remotePath, err) } } - - return v, nil - }) - if err != nil { - diags = diags.Extend(diag.FromErr(err)) } + + return v, nil + }) + if err != nil { + diags = diags.Extend(diag.FromErr(err)) } return libs, diags @@ -60,9 +55,9 @@ func ReplaceWithRemotePath(ctx context.Context, b *bundle.Bundle) (map[string][] // We collect them from task libraries, foreach task libraries, environment dependencies, and artifacts. // We return a map of library source to a list of config paths and locations where the library is used. // We use map so we don't upload the same library multiple times. -// Instead we upload it once and update all the config paths to point to the uploaded location. -func collectLocalLibraries(b *bundle.Bundle) (map[string][]ConfigLocation, error) { - libs := make(map[string]([]ConfigLocation)) +// This map is later used to upload the libraries to the remote location and update the config paths to point to the uploaded location. +func collectLocalLibraries(b *bundle.Bundle) (map[string][]LocationToUpdate, error) { + libs := make(map[string]([]LocationToUpdate)) patterns := []dyn.Pattern{ taskLibrariesPattern.Append(dyn.AnyIndex(), dyn.Key("whl")), @@ -86,7 +81,7 @@ func collectLocalLibraries(b *bundle.Bundle) (map[string][]ConfigLocation, error } source = filepath.Join(b.SyncRootPath, source) - libs[source] = append(libs[source], ConfigLocation{ + libs[source] = append(libs[source], LocationToUpdate{ configPath: p, location: v.Location(), }) @@ -130,7 +125,7 @@ func collectLocalLibraries(b *bundle.Bundle) (map[string][]ConfigLocation, error } } - libs[source] = append(libs[source], ConfigLocation{ + libs[source] = append(libs[source], LocationToUpdate{ configPath: p.Append(dyn.Key("remote_path")), location: v.Location(), }) diff --git a/bundle/libraries/upload.go b/bundle/libraries/upload.go index 80df0b3c76..590adda4ff 100644 --- a/bundle/libraries/upload.go +++ b/bundle/libraries/upload.go @@ -23,13 +23,13 @@ import ( // avoid hitting the rate limit. var maxFilesRequestsInFlight = 5 -func Upload(libs map[string][]ConfigLocation) bundle.Mutator { +func Upload(libs map[string][]LocationToUpdate) bundle.Mutator { return &upload{ libs: libs, } } -func UploadWithClient(libs map[string][]ConfigLocation, client filer.Filer) bundle.Mutator { +func UploadWithClient(libs map[string][]LocationToUpdate, client filer.Filer) bundle.Mutator { return &upload{ libs: libs, client: client, @@ -38,10 +38,10 @@ func UploadWithClient(libs map[string][]ConfigLocation, client filer.Filer) bund type upload struct { client filer.Filer - libs map[string][]ConfigLocation + libs map[string][]LocationToUpdate } -type ConfigLocation struct { +type LocationToUpdate struct { configPath dyn.Path location dyn.Location } diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index d2181fd0f8..e23505fb29 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -8,7 +8,6 @@ import ( "github.com/databricks/cli/bundle/apps" "github.com/databricks/cli/bundle/artifacts" "github.com/databricks/cli/bundle/config" - "github.com/databricks/cli/bundle/config/mutator" "github.com/databricks/cli/bundle/deploy" "github.com/databricks/cli/bundle/deploy/files" "github.com/databricks/cli/bundle/deploy/lock" @@ -21,7 +20,6 @@ import ( "github.com/databricks/cli/bundle/scripts" "github.com/databricks/cli/bundle/statemgmt" "github.com/databricks/cli/bundle/terranova" - "github.com/databricks/cli/bundle/trampoline" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/logdiag" @@ -146,46 +144,13 @@ func deployCore(ctx context.Context, b *bundle.Bundle) { // uploadLibraries uploads libraries to the workspace. // It also cleans up the artifacts directory and transforms wheel tasks. // It is called by only "bundle deploy". -func uploadLibraries(ctx context.Context, b *bundle.Bundle, libs map[string][]libraries.ConfigLocation) { +func uploadLibraries(ctx context.Context, b *bundle.Bundle, libs map[string][]libraries.LocationToUpdate) { bundle.ApplySeqContext(ctx, b, artifacts.CleanUp(), libraries.Upload(libs), ) } -// deployPrepare is common set of mutators between "bundle plan" and "bundle deploy". -func deployPrepare(ctx context.Context, b *bundle.Bundle) map[string][]libraries.ConfigLocation { - bundle.ApplySeqContext(ctx, b, - statemgmt.StatePull(), - terraform.CheckDashboardsModifiedRemotely(), - deploy.StatePull(), - mutator.ValidateGitDetails(), - terraform.CheckRunningResource(), - - // libraries.CheckForSameNameLibraries() needs to be run after we expand glob references so we - // know what are the actual library paths. - // libraries.ExpandGlobReferences() has to be run after the libraries are built and thus this - // mutator is part of the deploy step rather than validate. - libraries.ExpandGlobReferences(), - libraries.CheckForSameNameLibraries(), - // SwitchToPatchedWheels must be run after ExpandGlobReferences and after build phase because it Artifact.Source and Artifact.Patched populated - libraries.SwitchToPatchedWheels(), - ) - - libs, diags := libraries.ReplaceWithRemotePath(ctx, b) - for _, diag := range diags { - logdiag.LogDiag(ctx, diag) - } - - bundle.ApplySeqContext(ctx, b, - // TransformWheelTask must be run after ReplaceWithRemotePath so we can use correct remote path in the - // transformed notebook - trampoline.TransformWheelTask(), - ) - - return libs -} - // The deploy phase deploys artifacts and resources. func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHandler) { log.Info(ctx, "Phase: deploy") diff --git a/bundle/phases/plan.go b/bundle/phases/plan.go new file mode 100644 index 0000000000..2b0f09e2f1 --- /dev/null +++ b/bundle/phases/plan.go @@ -0,0 +1,48 @@ +package phases + +import ( + "context" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/bundle/deploy" + "github.com/databricks/cli/bundle/deploy/terraform" + "github.com/databricks/cli/bundle/libraries" + "github.com/databricks/cli/bundle/statemgmt" + "github.com/databricks/cli/bundle/trampoline" + "github.com/databricks/cli/libs/logdiag" +) + +// deployPrepare is common set of mutators between "bundle plan" and "bundle deploy". +// This function does not make any mutations in the workspace remotely, only in-memory bundle config mutations +func deployPrepare(ctx context.Context, b *bundle.Bundle) map[string][]libraries.LocationToUpdate { + bundle.ApplySeqContext(ctx, b, + statemgmt.StatePull(), + terraform.CheckDashboardsModifiedRemotely(), + deploy.StatePull(), + mutator.ValidateGitDetails(), + terraform.CheckRunningResource(), + + // libraries.CheckForSameNameLibraries() needs to be run after we expand glob references so we + // know what are the actual library paths. + // libraries.ExpandGlobReferences() has to be run after the libraries are built and thus this + // mutator is part of the deploy step rather than validate. + libraries.ExpandGlobReferences(), + libraries.CheckForSameNameLibraries(), + // SwitchToPatchedWheels must be run after ExpandGlobReferences and after build phase because it Artifact.Source and Artifact.Patched populated + libraries.SwitchToPatchedWheels(), + ) + + libs, diags := libraries.ReplaceWithRemotePath(ctx, b) + for _, diag := range diags { + logdiag.LogDiag(ctx, diag) + } + + bundle.ApplySeqContext(ctx, b, + // TransformWheelTask must be run after ReplaceWithRemotePath so we can use correct remote path in the + // transformed notebook + trampoline.TransformWheelTask(), + ) + + return libs +} From cb062a6eaf41980c2529426a4165536d70f0917e Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 1 Sep 2025 13:20:35 +0200 Subject: [PATCH 11/13] comment --- bundle/libraries/filer.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bundle/libraries/filer.go b/bundle/libraries/filer.go index a2b75d1a9e..4b62da5935 100644 --- a/bundle/libraries/filer.go +++ b/bundle/libraries/filer.go @@ -54,6 +54,8 @@ func GetFilerForLibrariesCleanup(ctx context.Context, b *bundle.Bundle) (filer.F } // If the remote path does not start with /Workspace or /Volumes, prepend /Workspace +// Some of the bundle configuration might use workspace paths like /Users or /Shared. +// While this is still a valid workspace path, the backend converts it to /Workspace/Users or /Workspace/Shared. func ensureWorkspaceOrVolumesPrefix(path string) string { if !strings.HasPrefix(path, "/Workspace") && !strings.HasPrefix(path, "/Volumes") { path = "/Workspace" + path From 233ecd5494bb4295be3dfd1463cf2eb54321ebc1 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 1 Sep 2025 13:22:32 +0200 Subject: [PATCH 12/13] fix test --- bundle/libraries/filer_test.go | 4 ++-- bundle/phases/deploy.go | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/bundle/libraries/filer_test.go b/bundle/libraries/filer_test.go index b4799fcf36..258a88dad1 100644 --- a/bundle/libraries/filer_test.go +++ b/bundle/libraries/filer_test.go @@ -28,7 +28,7 @@ func TestGetFilerForLibrariesValidWsfs(t *testing.T) { client, uploadPath, diags := GetFilerForLibraries(context.Background(), b) require.NoError(t, diags.Error()) - assert.Equal(t, "/foo/bar/artifacts/.internal", uploadPath) + assert.Equal(t, "/Workspace/foo/bar/artifacts/.internal", uploadPath) assert.IsType(t, &filer.WorkspaceFilesClient{}, client) } @@ -48,7 +48,7 @@ func TestGetFilerForLibrariesCleanupValidWsfs(t *testing.T) { client, uploadPath, diags := GetFilerForLibrariesCleanup(context.Background(), b) require.NoError(t, diags.Error()) - assert.Equal(t, "/foo/bar/artifacts", uploadPath) + assert.Equal(t, "/Workspace/foo/bar/artifacts", uploadPath) assert.IsType(t, &filer.WorkspaceFilesClient{}, client) } diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index 4cd373666c..ad8dda876c 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -19,7 +19,6 @@ import ( "github.com/databricks/cli/bundle/permissions" "github.com/databricks/cli/bundle/scripts" "github.com/databricks/cli/bundle/statemgmt" - "github.com/databricks/cli/bundle/terranova" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/logdiag" From 7db75a57165dbedb7af91fe88b3808cac80b4a18 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 1 Sep 2025 13:31:23 +0200 Subject: [PATCH 13/13] udpated output --- .../missing_ingestion_definition/out.requests.txt | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/acceptance/bundle/resource_deps/missing_ingestion_definition/out.requests.txt b/acceptance/bundle/resource_deps/missing_ingestion_definition/out.requests.txt index 9d772ac786..08f0d3dcd4 100644 --- a/acceptance/bundle/resource_deps/missing_ingestion_definition/out.requests.txt +++ b/acceptance/bundle/resource_deps/missing_ingestion_definition/out.requests.txt @@ -29,18 +29,3 @@ "method": "GET", "path": "/api/2.0/workspace/get-status" } -{ - "method": "POST", - "path": "/api/2.0/workspace/delete", - "body": { - "path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/artifacts/.internal", - "recursive": true - } -} -{ - "method": "POST", - "path": "/api/2.0/workspace/mkdirs", - "body": { - "path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/artifacts/.internal" - } -}