From 11b7b210b9625815bd41ec4d3b5bb66d118e1e1c Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 9 Oct 2023 11:13:09 +0200 Subject: [PATCH 1/3] Fixed using repo files as pipeline libraries --- bundle/config/mutator/expand_pipeline_glob_paths.go | 8 +++++++- bundle/config/mutator/expand_pipeline_glob_paths_test.go | 8 +++++++- bundle/libraries/libraries.go | 6 +++++- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/bundle/config/mutator/expand_pipeline_glob_paths.go b/bundle/config/mutator/expand_pipeline_glob_paths.go index 5fa203a000..f1adc220c6 100644 --- a/bundle/config/mutator/expand_pipeline_glob_paths.go +++ b/bundle/config/mutator/expand_pipeline_glob_paths.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "path/filepath" + "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/libraries" @@ -28,7 +29,7 @@ func (m *expandPipelineGlobPaths) Apply(_ context.Context, b *bundle.Bundle) err library := &pipeline.Libraries[i] path := getGlobPatternToExpand(library) - if path == "" || !libraries.IsLocalPath(path) { + if path == "" || isAbsoluteOrRemotePath(path) { expandedLibraries = append(expandedLibraries, *library) continue } @@ -52,6 +53,11 @@ func (m *expandPipelineGlobPaths) Apply(_ context.Context, b *bundle.Bundle) err return nil } +func isAbsoluteOrRemotePath(path string) bool { + // If path for library starts with /, it's a remote absolute path + return strings.HasPrefix(path, "/") || !libraries.IsLocalPath(path) +} + func getGlobPatternToExpand(library *pipelines.PipelineLibrary) string { if library.File != nil { return library.File.Path diff --git a/bundle/config/mutator/expand_pipeline_glob_paths_test.go b/bundle/config/mutator/expand_pipeline_glob_paths_test.go index ef99e71606..48cd52a094 100644 --- a/bundle/config/mutator/expand_pipeline_glob_paths_test.go +++ b/bundle/config/mutator/expand_pipeline_glob_paths_test.go @@ -80,6 +80,11 @@ func TestExpandGlobPathsInPipelines(t *testing.T) { Path: "dbfs:/me@company.com/test.ipynb", }, }, + { + Notebook: &pipelines.NotebookLibrary{ + Path: "/Repos/somerepo/test.ipynb", + }, + }, }, }, }, @@ -93,7 +98,7 @@ func TestExpandGlobPathsInPipelines(t *testing.T) { require.NoError(t, err) libraries := b.Config.Resources.Pipelines["pipeline"].Libraries - require.Len(t, libraries, 9) + require.Len(t, libraries, 10) // Making sure glob patterns are expanded correctly require.True(t, containsNotebook(libraries, filepath.Join("test", "test2.ipynb"))) @@ -107,6 +112,7 @@ func TestExpandGlobPathsInPipelines(t *testing.T) { // Making sure absolute pass to remote FS file references work as well require.True(t, containsNotebook(libraries, "/Workspace/Users/me@company.com/test.ipynb")) require.True(t, containsNotebook(libraries, "dbfs:/me@company.com/test.ipynb")) + require.True(t, containsNotebook(libraries, "/Repos/somerepo/test.ipynb")) // Making sure other libraries are not replaced require.True(t, containsJar(libraries, "./*.jar")) diff --git a/bundle/libraries/libraries.go b/bundle/libraries/libraries.go index f973642f80..c78f79353f 100644 --- a/bundle/libraries/libraries.go +++ b/bundle/libraries/libraries.go @@ -173,7 +173,7 @@ func IsLocalPath(path string) bool { return false } - return !isWorkspacePath(path) + return !isWorkspacePath(path) && !isReposPath(path) } func isExplicitFileScheme(path string) bool { @@ -200,3 +200,7 @@ func isWorkspacePath(path string) bool { strings.HasPrefix(path, "/Users/") || strings.HasPrefix(path, "/Shared/") } + +func isReposPath(path string) bool { + return strings.HasPrefix(path, "/Repos/") +} From 2ee7a2ad4d1aab8fc8f9f00b27f484354abbbb5f Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 9 Oct 2023 11:46:15 +0200 Subject: [PATCH 2/3] fix --- bundle/config/mutator/expand_pipeline_glob_paths.go | 8 +------- bundle/libraries/libraries.go | 10 ++++++++++ bundle/libraries/libraries_test.go | 2 +- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/bundle/config/mutator/expand_pipeline_glob_paths.go b/bundle/config/mutator/expand_pipeline_glob_paths.go index f1adc220c6..5fa203a000 100644 --- a/bundle/config/mutator/expand_pipeline_glob_paths.go +++ b/bundle/config/mutator/expand_pipeline_glob_paths.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "path/filepath" - "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/libraries" @@ -29,7 +28,7 @@ func (m *expandPipelineGlobPaths) Apply(_ context.Context, b *bundle.Bundle) err library := &pipeline.Libraries[i] path := getGlobPatternToExpand(library) - if path == "" || isAbsoluteOrRemotePath(path) { + if path == "" || !libraries.IsLocalPath(path) { expandedLibraries = append(expandedLibraries, *library) continue } @@ -53,11 +52,6 @@ func (m *expandPipelineGlobPaths) Apply(_ context.Context, b *bundle.Bundle) err return nil } -func isAbsoluteOrRemotePath(path string) bool { - // If path for library starts with /, it's a remote absolute path - return strings.HasPrefix(path, "/") || !libraries.IsLocalPath(path) -} - func getGlobPatternToExpand(library *pipelines.PipelineLibrary) string { if library.File != nil { return library.File.Path diff --git a/bundle/libraries/libraries.go b/bundle/libraries/libraries.go index c78f79353f..f2698dd054 100644 --- a/bundle/libraries/libraries.go +++ b/bundle/libraries/libraries.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/url" + "path" "path/filepath" "strings" @@ -173,6 +174,10 @@ func IsLocalPath(path string) bool { return false } + if isAbsoluteRemotePath(path) { + return false + } + return !isWorkspacePath(path) && !isReposPath(path) } @@ -204,3 +209,8 @@ func isWorkspacePath(path string) bool { func isReposPath(path string) bool { return strings.HasPrefix(path, "/Repos/") } + +func isAbsoluteRemotePath(p string) bool { + // If path for library starts with /, it's a remote absolute path + return path.IsAbs(p) +} diff --git a/bundle/libraries/libraries_test.go b/bundle/libraries/libraries_test.go index 7ff1609ab0..41609bd4e1 100644 --- a/bundle/libraries/libraries_test.go +++ b/bundle/libraries/libraries_test.go @@ -10,7 +10,7 @@ import ( var testCases map[string]bool = map[string]bool{ "./some/local/path": true, - "/some/full/path": true, + "/some/full/path": false, "/Workspace/path/to/package": false, "/Users/path/to/package": false, "file://path/to/package": true, From 74af538433c0c5a766e992abf23cf8e6cec4a3d6 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 9 Oct 2023 12:03:43 +0200 Subject: [PATCH 3/3] fix --- bundle/libraries/libraries.go | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/bundle/libraries/libraries.go b/bundle/libraries/libraries.go index f2698dd054..548d5ef1b6 100644 --- a/bundle/libraries/libraries.go +++ b/bundle/libraries/libraries.go @@ -174,11 +174,7 @@ func IsLocalPath(path string) bool { return false } - if isAbsoluteRemotePath(path) { - return false - } - - return !isWorkspacePath(path) && !isReposPath(path) + return !isAbsoluteRemotePath(path) } func isExplicitFileScheme(path string) bool { @@ -206,10 +202,6 @@ func isWorkspacePath(path string) bool { strings.HasPrefix(path, "/Shared/") } -func isReposPath(path string) bool { - return strings.HasPrefix(path, "/Repos/") -} - func isAbsoluteRemotePath(p string) bool { // If path for library starts with /, it's a remote absolute path return path.IsAbs(p)