From 84b47745e451f6552465243665ad6c897c55ae5e Mon Sep 17 00:00:00 2001 From: "Lennart Kats (databricks)" Date: Fri, 23 Aug 2024 12:13:21 +0200 Subject: [PATCH 1/2] Ignore CLI version check on development builds of the CLI (#1714) ## Changes This changes makes sure we ignore CLI version check on development builds of the CLI. Before: ``` $ cat databricks.yml | grep cli_version databricks_cli_version: ">= 0.223.1" $ cli bundle deploy Error: Databricks CLI version constraint not satisfied. Required: >= 0.223.1, current: 0.0.0-dev+06b169284737 ``` after ``` ... $ cli bundle deploy ... Warning: Ignoring Databricks CLI version constraint for development build. Required: >= 0.223.1, current: 0.0.0-dev+d52d6f08fcd5 ``` ## Tests --- bundle/config/mutator/verify_cli_version.go | 4 ++++ bundle/config/mutator/verify_cli_version_test.go | 7 ++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/bundle/config/mutator/verify_cli_version.go b/bundle/config/mutator/verify_cli_version.go index 9c32fcc9d..279af44e6 100644 --- a/bundle/config/mutator/verify_cli_version.go +++ b/bundle/config/mutator/verify_cli_version.go @@ -40,6 +40,10 @@ func (v *verifyCliVersion) Apply(ctx context.Context, b *bundle.Bundle) diag.Dia } if !c.Check(version) { + if version.Prerelease() == "dev" && version.Major() == 0 { + return diag.Warningf("Ignoring Databricks CLI version constraint for development build. Required: %s, current: %s", constraint, currentVersion) + } + return diag.Errorf("Databricks CLI version constraint not satisfied. Required: %s, current: %s", constraint, currentVersion) } diff --git a/bundle/config/mutator/verify_cli_version_test.go b/bundle/config/mutator/verify_cli_version_test.go index 24f656745..025461292 100644 --- a/bundle/config/mutator/verify_cli_version_test.go +++ b/bundle/config/mutator/verify_cli_version_test.go @@ -107,6 +107,11 @@ func TestVerifyCliVersion(t *testing.T) { constraint: "^0.100", expectedError: "invalid version constraint \"^0.100\" specified. Please specify the version constraint in the format (>=) 0.0.0(, <= 1.0.0)", }, + { + currentVersion: "0.0.0-dev+06b169284737", + constraint: ">= 0.100.0", + expectedError: "Ignoring Databricks CLI version constraint for development build. Required: >= 0.100.0", + }, } t.Cleanup(func() { @@ -130,7 +135,7 @@ func TestVerifyCliVersion(t *testing.T) { diags := bundle.Apply(context.Background(), b, VerifyCliVersion()) if tc.expectedError != "" { require.NotEmpty(t, diags) - require.Equal(t, tc.expectedError, diags.Error().Error()) + require.Contains(t, diags[0].Summary, tc.expectedError) } else { require.Empty(t, diags) } From 783e05c939a694fe722e52ddea9c48f0ea077181 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 26 Aug 2024 12:03:56 +0200 Subject: [PATCH 2/2] Do not treat empty path as a local path (#1717) ## Changes Fixes issue introduced here https://github.com/databricks/cli/pull/1699 where PyPi packages were treated as local library. The reason is that `libraryPath` returns an empty string as a path for PyPi packages and then `IsLibraryLocal` treated empty string as local path. Both of these functions are fixed in this PR. ## Tests Added regression test --- bundle/libraries/helpers.go | 19 +++++++---- bundle/libraries/helpers_test.go | 28 +++++++++++++--- bundle/libraries/libraries.go | 7 +++- bundle/libraries/local_path.go | 4 +++ bundle/libraries/local_path_test.go | 1 + bundle/libraries/workspace_path.go | 4 +-- bundle/python/warning_test.go | 51 +++++++++++++++++++++++++++++ 7 files changed, 99 insertions(+), 15 deletions(-) diff --git a/bundle/libraries/helpers.go b/bundle/libraries/helpers.go index b7e707ccf..2149e5885 100644 --- a/bundle/libraries/helpers.go +++ b/bundle/libraries/helpers.go @@ -1,19 +1,24 @@ package libraries -import "github.com/databricks/databricks-sdk-go/service/compute" +import ( + "fmt" -func libraryPath(library *compute.Library) string { + "github.com/databricks/databricks-sdk-go/service/compute" +) + +func libraryPath(library *compute.Library) (string, error) { if library.Whl != "" { - return library.Whl + return library.Whl, nil } if library.Jar != "" { - return library.Jar + return library.Jar, nil } if library.Egg != "" { - return library.Egg + return library.Egg, nil } if library.Requirements != "" { - return library.Requirements + return library.Requirements, nil } - return "" + + return "", fmt.Errorf("not supported library type") } diff --git a/bundle/libraries/helpers_test.go b/bundle/libraries/helpers_test.go index e4bd32770..9d7e12ee5 100644 --- a/bundle/libraries/helpers_test.go +++ b/bundle/libraries/helpers_test.go @@ -10,9 +10,27 @@ import ( func TestLibraryPath(t *testing.T) { path := "/some/path" - assert.Equal(t, path, libraryPath(&compute.Library{Whl: path})) - assert.Equal(t, path, libraryPath(&compute.Library{Jar: path})) - assert.Equal(t, path, libraryPath(&compute.Library{Egg: path})) - assert.Equal(t, path, libraryPath(&compute.Library{Requirements: path})) - assert.Equal(t, "", libraryPath(&compute.Library{})) + p, err := libraryPath(&compute.Library{Whl: path}) + assert.Equal(t, path, p) + assert.Nil(t, err) + + p, err = libraryPath(&compute.Library{Jar: path}) + assert.Equal(t, path, p) + assert.Nil(t, err) + + p, err = libraryPath(&compute.Library{Egg: path}) + assert.Equal(t, path, p) + assert.Nil(t, err) + + p, err = libraryPath(&compute.Library{Requirements: path}) + assert.Equal(t, path, p) + assert.Nil(t, err) + + p, err = libraryPath(&compute.Library{}) + assert.Equal(t, "", p) + assert.NotNil(t, err) + + p, err = libraryPath(&compute.Library{Pypi: &compute.PythonPyPiLibrary{Package: "pypipackage"}}) + assert.Equal(t, "", p) + assert.NotNil(t, err) } diff --git a/bundle/libraries/libraries.go b/bundle/libraries/libraries.go index 33b848dd9..f75e23a8c 100644 --- a/bundle/libraries/libraries.go +++ b/bundle/libraries/libraries.go @@ -67,7 +67,12 @@ func FindTasksWithLocalLibraries(b *bundle.Bundle) []jobs.Task { func isTaskWithLocalLibraries(task jobs.Task) bool { for _, l := range task.Libraries { - if IsLibraryLocal(libraryPath(&l)) { + p, err := libraryPath(&l) + // If there's an error, skip the library because it's not of supported type + if err != nil { + continue + } + if IsLibraryLocal(p) { return true } } diff --git a/bundle/libraries/local_path.go b/bundle/libraries/local_path.go index 417bce10e..e49562405 100644 --- a/bundle/libraries/local_path.go +++ b/bundle/libraries/local_path.go @@ -43,6 +43,10 @@ func IsLocalPath(p string) bool { // We can't use IsLocalPath beacuse environment dependencies can be // a pypi package name which can be misinterpreted as a local path by IsLocalPath. func IsLibraryLocal(dep string) bool { + if dep == "" { + return false + } + possiblePrefixes := []string{ ".", } diff --git a/bundle/libraries/local_path_test.go b/bundle/libraries/local_path_test.go index 7f84b3244..667d64ec8 100644 --- a/bundle/libraries/local_path_test.go +++ b/bundle/libraries/local_path_test.go @@ -48,6 +48,7 @@ func TestIsLibraryLocal(t *testing.T) { {path: "../../local/*.whl", expected: true}, {path: "..\\..\\local\\*.whl", expected: true}, {path: "file://path/to/package/whl.whl", expected: true}, + {path: "", expected: false}, {path: "pypipackage", expected: false}, {path: "/Volumes/catalog/schema/volume/path.whl", expected: false}, {path: "/Workspace/my_project/dist.whl", expected: false}, diff --git a/bundle/libraries/workspace_path.go b/bundle/libraries/workspace_path.go index b08ca1616..126ad3f13 100644 --- a/bundle/libraries/workspace_path.go +++ b/bundle/libraries/workspace_path.go @@ -29,8 +29,8 @@ func IsWorkspacePath(path string) bool { // IsWorkspaceLibrary returns true if the specified library refers to a workspace path. func IsWorkspaceLibrary(library *compute.Library) bool { - path := libraryPath(library) - if path == "" { + path, err := libraryPath(library) + if err != nil { return false } diff --git a/bundle/python/warning_test.go b/bundle/python/warning_test.go index dd6397f78..b2296392b 100644 --- a/bundle/python/warning_test.go +++ b/bundle/python/warning_test.go @@ -223,6 +223,17 @@ func TestNoIncompatibleWheelTasks(t *testing.T) { {Whl: "./dist/test.whl"}, }, }, + { + TaskKey: "key7", + PythonWheelTask: &jobs.PythonWheelTask{}, + ExistingClusterId: "test-key-2", + Libraries: []compute.Library{ + {Whl: "signol_lib-0.4.4-20240822+prod-py3-none-any.whl"}, + {Pypi: &compute.PythonPyPiLibrary{ + Package: "requests==2.25.1", + }}, + }, + }, }, }, }, @@ -241,6 +252,46 @@ func TestNoIncompatibleWheelTasks(t *testing.T) { require.False(t, hasIncompatibleWheelTasks(context.Background(), b)) } +func TestTasksWithPyPiPackageAreCompatible(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job1": { + JobSettings: &jobs.JobSettings{ + JobClusters: []jobs.JobCluster{ + { + JobClusterKey: "cluster1", + NewCluster: compute.ClusterSpec{ + SparkVersion: "12.2.x-scala2.12", + }, + }, + }, + Tasks: []jobs.Task{ + { + TaskKey: "key1", + PythonWheelTask: &jobs.PythonWheelTask{}, + ExistingClusterId: "test-key-2", + Libraries: []compute.Library{ + {Pypi: &compute.PythonPyPiLibrary{ + Package: "requests==2.25.1", + }}, + }, + }, + }, + }, + }, + }, + }, + }, + } + + m := mocks.NewMockWorkspaceClient(t) + b.SetWorkpaceClient(m.WorkspaceClient) + + require.False(t, hasIncompatibleWheelTasks(context.Background(), b)) +} + func TestNoWarningWhenPythonWheelWrapperIsOn(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{