Skip to content

Commit fb2b646

Browse files
authored
Move generate utils to bundle/generate package (#3088)
## Changes * Move the downloader and the configuration conversion functions to `bundle/generate`. * Export necessary functions. * Add a simple unit test. ## Why While looking into #2528, I wanted to add a unit test to the downloader and found it doesn't have any direct unit tests yet, and was colocated with the command implementations. This consolidates the downloader and the configuration conversion functions from `bundle/config/generate` into `bundle/generate` and adds a simple unit test for the downloader. A follow-up PR will address path normalization. ## Tests Unit tests pass.
1 parent c5b1d72 commit fb2b646

File tree

10 files changed

+60
-18
lines changed

10 files changed

+60
-18
lines changed
Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,23 +18,23 @@ import (
1818
"golang.org/x/sync/errgroup"
1919
)
2020

21-
type downloader struct {
21+
type Downloader struct {
2222
files map[string]string
2323
w *databricks.WorkspaceClient
2424
sourceDir string
2525
configDir string
2626
basePath string
2727
}
2828

29-
func (n *downloader) MarkTaskForDownload(ctx context.Context, task *jobs.Task) error {
29+
func (n *Downloader) MarkTaskForDownload(ctx context.Context, task *jobs.Task) error {
3030
if task.NotebookTask == nil {
3131
return nil
3232
}
3333

3434
return n.markNotebookForDownload(ctx, &task.NotebookTask.NotebookPath)
3535
}
3636

37-
func (n *downloader) MarkPipelineLibraryForDownload(ctx context.Context, lib *pipelines.PipelineLibrary) error {
37+
func (n *Downloader) MarkPipelineLibraryForDownload(ctx context.Context, lib *pipelines.PipelineLibrary) error {
3838
if lib.Notebook != nil {
3939
return n.markNotebookForDownload(ctx, &lib.Notebook.Path)
4040
}
@@ -46,7 +46,7 @@ func (n *downloader) MarkPipelineLibraryForDownload(ctx context.Context, lib *pi
4646
return nil
4747
}
4848

49-
func (n *downloader) markFileForDownload(ctx context.Context, filePath *string) error {
49+
func (n *Downloader) markFileForDownload(ctx context.Context, filePath *string) error {
5050
_, err := n.w.Workspace.GetStatusByPath(ctx, *filePath)
5151
if err != nil {
5252
return err
@@ -66,7 +66,7 @@ func (n *downloader) markFileForDownload(ctx context.Context, filePath *string)
6666
return nil
6767
}
6868

69-
func (n *downloader) markDirectoryForDownload(ctx context.Context, dirPath *string) error {
69+
func (n *Downloader) MarkDirectoryForDownload(ctx context.Context, dirPath *string) error {
7070
_, err := n.w.Workspace.GetStatusByPath(ctx, *dirPath)
7171
if err != nil {
7272
return err
@@ -102,7 +102,7 @@ func (n *downloader) markDirectoryForDownload(ctx context.Context, dirPath *stri
102102
return nil
103103
}
104104

105-
func (n *downloader) markNotebookForDownload(ctx context.Context, notebookPath *string) error {
105+
func (n *Downloader) markNotebookForDownload(ctx context.Context, notebookPath *string) error {
106106
info, err := n.w.Workspace.GetStatusByPath(ctx, *notebookPath)
107107
if err != nil {
108108
return err
@@ -123,7 +123,7 @@ func (n *downloader) markNotebookForDownload(ctx context.Context, notebookPath *
123123
return nil
124124
}
125125

126-
func (n *downloader) relativePath(fullPath string) string {
126+
func (n *Downloader) relativePath(fullPath string) string {
127127
basePath := path.Dir(fullPath)
128128
if n.basePath != "" {
129129
basePath = n.basePath
@@ -138,7 +138,7 @@ func (n *downloader) relativePath(fullPath string) string {
138138
return relPath
139139
}
140140

141-
func (n *downloader) FlushToDisk(ctx context.Context, force bool) error {
141+
func (n *Downloader) FlushToDisk(ctx context.Context, force bool) error {
142142
// First check that all files can be written
143143
for targetPath := range n.files {
144144
info, err := os.Stat(targetPath)
@@ -185,8 +185,8 @@ func (n *downloader) FlushToDisk(ctx context.Context, force bool) error {
185185
return errs.Wait()
186186
}
187187

188-
func newDownloader(w *databricks.WorkspaceClient, sourceDir, configDir string) *downloader {
189-
return &downloader{
188+
func NewDownloader(w *databricks.WorkspaceClient, sourceDir, configDir string) *Downloader {
189+
return &Downloader{
190190
files: make(map[string]string),
191191
w: w,
192192
sourceDir: sourceDir,

bundle/generate/downloader_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
package generate
2+
3+
import (
4+
"context"
5+
"path/filepath"
6+
"testing"
7+
8+
"github.com/databricks/databricks-sdk-go/experimental/mocks"
9+
"github.com/databricks/databricks-sdk-go/service/workspace"
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
func TestDownloader_MarkFileReturnsRelativePath(t *testing.T) {
15+
ctx := context.Background()
16+
m := mocks.NewMockWorkspaceClient(t)
17+
18+
dir := "base/dir/doesnt/matter"
19+
sourceDir := filepath.Join(dir, "source")
20+
configDir := filepath.Join(dir, "config")
21+
downloader := NewDownloader(m.WorkspaceClient, sourceDir, configDir)
22+
23+
var err error
24+
25+
// Test that the path is normalized to be relative to the config directory.
26+
f1 := "/a/b/c"
27+
m.GetMockWorkspaceAPI().EXPECT().GetStatusByPath(ctx, f1).Return(&workspace.ObjectInfo{
28+
Path: f1,
29+
}, nil)
30+
err = downloader.markFileForDownload(ctx, &f1)
31+
require.NoError(t, err)
32+
assert.Equal(t, filepath.FromSlash("../source/c"), f1)
33+
34+
// Test that the previous path doesn't influence the next path.
35+
f2 := "/a/b/c/d"
36+
m.GetMockWorkspaceAPI().EXPECT().GetStatusByPath(ctx, f2).Return(&workspace.ObjectInfo{
37+
Path: f2,
38+
}, nil)
39+
err = downloader.markFileForDownload(ctx, &f2)
40+
require.NoError(t, err)
41+
assert.Equal(t, filepath.FromSlash("../source/d"), f2)
42+
}

cmd/bundle/generate/app.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import (
44
"fmt"
55
"path/filepath"
66

7-
"github.com/databricks/cli/bundle/config/generate"
7+
"github.com/databricks/cli/bundle/generate"
88
"github.com/databricks/cli/cmd/root"
99
"github.com/databricks/cli/libs/cmdio"
1010
"github.com/databricks/cli/libs/dyn"
@@ -46,10 +46,10 @@ func NewGenerateAppCommand() *cobra.Command {
4646
return err
4747
}
4848

49-
downloader := newDownloader(w, sourceDir, configDir)
49+
downloader := generate.NewDownloader(w, sourceDir, configDir)
5050

5151
sourceCodePath := app.DefaultSourceCodePath
52-
err = downloader.markDirectoryForDownload(ctx, &sourceCodePath)
52+
err = downloader.MarkDirectoryForDownload(ctx, &sourceCodePath)
5353
if err != nil {
5454
return err
5555
}

cmd/bundle/generate/dashboard.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ import (
1313
"time"
1414

1515
"github.com/databricks/cli/bundle"
16-
"github.com/databricks/cli/bundle/config/generate"
1716
"github.com/databricks/cli/bundle/deploy/terraform"
17+
"github.com/databricks/cli/bundle/generate"
1818
"github.com/databricks/cli/bundle/phases"
1919
"github.com/databricks/cli/bundle/render"
2020
"github.com/databricks/cli/bundle/resources"

cmd/bundle/generate/job.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import (
77
"os"
88
"path/filepath"
99

10-
"github.com/databricks/cli/bundle/config/generate"
10+
"github.com/databricks/cli/bundle/generate"
1111
"github.com/databricks/cli/cmd/root"
1212
"github.com/databricks/cli/libs/cmdio"
1313
"github.com/databricks/cli/libs/dyn"
@@ -49,7 +49,7 @@ func NewGenerateJobCommand() *cobra.Command {
4949
return err
5050
}
5151

52-
downloader := newDownloader(w, sourceDir, configDir)
52+
downloader := generate.NewDownloader(w, sourceDir, configDir)
5353

5454
// Don't download files if the job is using Git source
5555
// When Git source is used, the job will be using the files from the Git repository

cmd/bundle/generate/pipeline.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import (
77
"os"
88
"path/filepath"
99

10-
"github.com/databricks/cli/bundle/config/generate"
10+
"github.com/databricks/cli/bundle/generate"
1111
"github.com/databricks/cli/cmd/root"
1212
"github.com/databricks/cli/libs/cmdio"
1313
"github.com/databricks/cli/libs/dyn"
@@ -49,7 +49,7 @@ func NewGeneratePipelineCommand() *cobra.Command {
4949
return err
5050
}
5151

52-
downloader := newDownloader(w, sourceDir, configDir)
52+
downloader := generate.NewDownloader(w, sourceDir, configDir)
5353
for _, lib := range pipeline.Spec.Libraries {
5454
err := downloader.MarkPipelineLibraryForDownload(ctx, &lib)
5555
if err != nil {

0 commit comments

Comments
 (0)