From 0767f9b86e5ba09da23f02ad38fa5c48992aa7b5 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Fri, 1 Mar 2024 12:03:51 +0100 Subject: [PATCH 1/5] Support relative paths in artifact files source section and always upload all artifact files --- bundle/artifacts/artifacts.go | 14 ++--- bundle/artifacts/upload.go | 28 ++++++++++ bundle/artifacts/upload_test.go | 92 +++++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 7 deletions(-) create mode 100644 bundle/artifacts/upload_test.go diff --git a/bundle/artifacts/artifacts.go b/bundle/artifacts/artifacts.go index e474240ded..ce2e165b7f 100644 --- a/bundle/artifacts/artifacts.go +++ b/bundle/artifacts/artifacts.go @@ -121,13 +121,6 @@ func uploadArtifact(ctx context.Context, b *bundle.Bundle, a *config.Artifact, u for i := range a.Files { f := &a.Files[i] - // Lookup all tasks that reference this file. - libs, ok := filesToLibraries[f.Source] - if !ok { - log.Debugf(ctx, "No tasks reference %s. Skipping upload.", f.Source) - continue - } - filename := filepath.Base(f.Source) cmdio.LogString(ctx, fmt.Sprintf("Uploading %s...", filename)) @@ -139,6 +132,13 @@ func uploadArtifact(ctx context.Context, b *bundle.Bundle, a *config.Artifact, u log.Infof(ctx, "Upload succeeded") f.RemotePath = path.Join(uploadPath, filepath.Base(f.Source)) + // Lookup all tasks that reference this file. + libs, ok := filesToLibraries[f.Source] + if !ok { + log.Debugf(ctx, "No tasks reference %s", f.Source) + continue + } + // Update all tasks that reference this file. for _, lib := range libs { wsfsBase := "/Workspace" diff --git a/bundle/artifacts/upload.go b/bundle/artifacts/upload.go index 990718aa46..e99e0231a4 100644 --- a/bundle/artifacts/upload.go +++ b/bundle/artifacts/upload.go @@ -3,8 +3,10 @@ package artifacts import ( "context" "fmt" + "path/filepath" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" "github.com/databricks/databricks-sdk-go/service/workspace" ) @@ -41,6 +43,32 @@ func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) error { return fmt.Errorf("artifact source is not configured: %s", m.name) } + // Check if source paths are absolute, if not, make them absolute + for k := range artifact.Files { + f := &artifact.Files[k] + if !filepath.IsAbs(f.Source) { + f.Source = filepath.Join(b.Config.Path, f.Source) + } + } + + // Expand any glob reference in files source path + files := make([]config.ArtifactFile, 0, len(artifact.Files)) + for _, f := range artifact.Files { + matches, err := filepath.Glob(f.Source) + // If the source is not a glob pattern or no matches, just add it to the list + if err != nil || len(matches) == 0 { + files = append(files, f) + continue + } + + for _, match := range matches { + files = append(files, config.ArtifactFile{ + Source: match, + }) + } + } + + artifact.Files = files return bundle.Apply(ctx, b, getUploadMutator(artifact.Type, m.name)) } diff --git a/bundle/artifacts/upload_test.go b/bundle/artifacts/upload_test.go new file mode 100644 index 0000000000..c4b4570be0 --- /dev/null +++ b/bundle/artifacts/upload_test.go @@ -0,0 +1,92 @@ +package artifacts + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/libs/testfile" + "github.com/stretchr/testify/require" +) + +type noop struct{} + +func (n *noop) Apply(context.Context, *bundle.Bundle) error { + return nil +} + +func (n *noop) Name() string { + return "noop" +} + +func TestExpandGlobFilesSource(t *testing.T) { + rootPath := t.TempDir() + err := os.Mkdir(filepath.Join(rootPath, "test"), 0755) + require.NoError(t, err) + + testfile.CreateFile(t, filepath.Join(rootPath, "test", "myjar1.jar")) + testfile.CreateFile(t, filepath.Join(rootPath, "test", "myjar2.jar")) + + b := &bundle.Bundle{ + Config: config.Root{ + Path: rootPath, + Artifacts: map[string]*config.Artifact{ + "test": { + Type: "custom", + Files: []config.ArtifactFile{ + { + Source: filepath.Join(".", "test", "*.jar"), + }, + }, + }, + }, + }, + } + u := &upload{"test"} + uploadMutators[config.ArtifactType("custom")] = func(name string) bundle.Mutator { + return &noop{} + } + + err = bundle.Apply(context.Background(), b, u) + require.NoError(t, err) + + require.Equal(t, 2, len(b.Config.Artifacts["test"].Files)) + require.Equal(t, filepath.Join(rootPath, "test", "myjar1.jar"), b.Config.Artifacts["test"].Files[0].Source) + require.Equal(t, filepath.Join(rootPath, "test", "myjar2.jar"), b.Config.Artifacts["test"].Files[1].Source) +} + +func TestExpandGlobFilesSourceWithNoMatches(t *testing.T) { + rootPath := t.TempDir() + err := os.Mkdir(filepath.Join(rootPath, "test"), 0755) + require.NoError(t, err) + + b := &bundle.Bundle{ + Config: config.Root{ + Path: rootPath, + Artifacts: map[string]*config.Artifact{ + "test": { + Type: "custom", + Files: []config.ArtifactFile{ + { + Source: filepath.Join(".", "test", "myjar.jar"), + }, + }, + }, + }, + }, + } + u := &upload{"test"} + uploadMutators[config.ArtifactType("custom")] = func(name string) bundle.Mutator { + return &noop{} + } + + err = bundle.Apply(context.Background(), b, u) + require.NoError(t, err) + + // We expect to have the same path as it was provided in the source + require.Equal(t, 1, len(b.Config.Artifacts["test"].Files)) + require.Equal(t, filepath.Join(rootPath, "test", "myjar.jar"), b.Config.Artifacts["test"].Files[0].Source) +} From d836b3f680c6938850012bd9e78d9de47ce455b6 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Fri, 1 Mar 2024 12:26:39 +0100 Subject: [PATCH 2/5] fix tests --- bundle/artifacts/upload_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/bundle/artifacts/upload_test.go b/bundle/artifacts/upload_test.go index c4b4570be0..4841f3e935 100644 --- a/bundle/artifacts/upload_test.go +++ b/bundle/artifacts/upload_test.go @@ -27,8 +27,11 @@ func TestExpandGlobFilesSource(t *testing.T) { err := os.Mkdir(filepath.Join(rootPath, "test"), 0755) require.NoError(t, err) - testfile.CreateFile(t, filepath.Join(rootPath, "test", "myjar1.jar")) - testfile.CreateFile(t, filepath.Join(rootPath, "test", "myjar2.jar")) + t1 := testfile.CreateFile(t, filepath.Join(rootPath, "test", "myjar1.jar")) + t1.Close(t) + + t2 := testfile.CreateFile(t, filepath.Join(rootPath, "test", "myjar2.jar")) + t2.Close(t) b := &bundle.Bundle{ Config: config.Root{ From 21a69e4817d4ad44bd3dae2680e1cf9360255f21 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 4 Mar 2024 16:38:37 +0100 Subject: [PATCH 3/5] fix --- bundle/artifacts/build.go | 2 +- bundle/artifacts/upload.go | 13 ++++++++----- bundle/artifacts/upload_test.go | 6 +++++- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/bundle/artifacts/build.go b/bundle/artifacts/build.go index 6b1aac8226..871eaa1839 100644 --- a/bundle/artifacts/build.go +++ b/bundle/artifacts/build.go @@ -49,7 +49,7 @@ func (m *build) Apply(ctx context.Context, b *bundle.Bundle) error { } if !filepath.IsAbs(artifact.Path) { - artifact.Path = filepath.Join(b.Config.Path, artifact.Path) + artifact.Path = filepath.Join(artifact.ConfigFilePath, artifact.Path) } return bundle.Apply(ctx, b, getBuildMutator(artifact.Type, m.name)) diff --git a/bundle/artifacts/upload.go b/bundle/artifacts/upload.go index e99e0231a4..61e6520866 100644 --- a/bundle/artifacts/upload.go +++ b/bundle/artifacts/upload.go @@ -47,7 +47,8 @@ func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) error { for k := range artifact.Files { f := &artifact.Files[k] if !filepath.IsAbs(f.Source) { - f.Source = filepath.Join(b.Config.Path, f.Source) + dirPath := filepath.Dir(artifact.ConfigFilePath) + f.Source = filepath.Join(dirPath, f.Source) } } @@ -55,10 +56,12 @@ func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) error { files := make([]config.ArtifactFile, 0, len(artifact.Files)) for _, f := range artifact.Files { matches, err := filepath.Glob(f.Source) - // If the source is not a glob pattern or no matches, just add it to the list - if err != nil || len(matches) == 0 { - files = append(files, f) - continue + if err != nil { + return fmt.Errorf("unable to find files for %s: %w", f.Source, err) + } + + if len(matches) == 0 { + return fmt.Errorf("no files found for %s", f.Source) } for _, match := range matches { diff --git a/bundle/artifacts/upload_test.go b/bundle/artifacts/upload_test.go index 4841f3e935..02a24766eb 100644 --- a/bundle/artifacts/upload_test.go +++ b/bundle/artifacts/upload_test.go @@ -8,6 +8,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/internal/bundletest" "github.com/databricks/cli/libs/testfile" "github.com/stretchr/testify/require" ) @@ -41,13 +42,16 @@ func TestExpandGlobFilesSource(t *testing.T) { Type: "custom", Files: []config.ArtifactFile{ { - Source: filepath.Join(".", "test", "*.jar"), + Source: filepath.Join("..", "test", "*.jar"), }, }, }, }, }, } + + bundletest.SetLocation(b, ".", filepath.Join(rootPath, "resources", "artifacts.yml")) + u := &upload{"test"} uploadMutators[config.ArtifactType("custom")] = func(name string) bundle.Mutator { return &noop{} From 67af01e51cb33d79c0b793f91fba545f99c8be83 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 4 Mar 2024 16:43:21 +0100 Subject: [PATCH 4/5] fixed test --- bundle/artifacts/build.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bundle/artifacts/build.go b/bundle/artifacts/build.go index 871eaa1839..a78958e60b 100644 --- a/bundle/artifacts/build.go +++ b/bundle/artifacts/build.go @@ -49,7 +49,8 @@ func (m *build) Apply(ctx context.Context, b *bundle.Bundle) error { } if !filepath.IsAbs(artifact.Path) { - artifact.Path = filepath.Join(artifact.ConfigFilePath, artifact.Path) + dirPath := filepath.Dir(artifact.ConfigFilePath) + artifact.Path = filepath.Join(dirPath, artifact.Path) } return bundle.Apply(ctx, b, getBuildMutator(artifact.Type, m.name)) From 33c2182881fd3e6aea34eb0c1644a91f0ec24d46 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 4 Mar 2024 16:49:17 +0100 Subject: [PATCH 5/5] fix --- bundle/artifacts/upload_test.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/bundle/artifacts/upload_test.go b/bundle/artifacts/upload_test.go index 02a24766eb..6dea1c1457 100644 --- a/bundle/artifacts/upload_test.go +++ b/bundle/artifacts/upload_test.go @@ -78,22 +78,21 @@ func TestExpandGlobFilesSourceWithNoMatches(t *testing.T) { Type: "custom", Files: []config.ArtifactFile{ { - Source: filepath.Join(".", "test", "myjar.jar"), + Source: filepath.Join("..", "test", "myjar.jar"), }, }, }, }, }, } + + bundletest.SetLocation(b, ".", filepath.Join(rootPath, "resources", "artifacts.yml")) + u := &upload{"test"} uploadMutators[config.ArtifactType("custom")] = func(name string) bundle.Mutator { return &noop{} } err = bundle.Apply(context.Background(), b, u) - require.NoError(t, err) - - // We expect to have the same path as it was provided in the source - require.Equal(t, 1, len(b.Config.Artifacts["test"].Files)) - require.Equal(t, filepath.Join(rootPath, "test", "myjar.jar"), b.Config.Artifacts["test"].Files[0].Source) + require.ErrorContains(t, err, "no files found for") }