Skip to content

Commit

Permalink
Move path field to bundle type (#1316)
Browse files Browse the repository at this point in the history
## Changes

The bundle path was previously stored on the `config.Root` type under
the assumption that the first configuration file being loaded would set
it. This is slightly counterintuitive and we know what the path is upon
construction of the bundle. The new location for this property reflects
this.

## Tests

Unit tests pass.
  • Loading branch information
pietern authored Mar 27, 2024
1 parent b503804 commit 00d76d5
Show file tree
Hide file tree
Showing 39 changed files with 104 additions and 124 deletions.
2 changes: 1 addition & 1 deletion bundle/artifacts/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (m *build) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {

// If artifact path is not provided, use bundle root dir
if artifact.Path == "" {
artifact.Path = b.Config.Path
artifact.Path = b.RootPath
}

if !filepath.IsAbs(artifact.Path) {
Expand Down
4 changes: 2 additions & 2 deletions bundle/artifacts/upload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ func TestExpandGlobFilesSource(t *testing.T) {
t2.Close(t)

b := &bundle.Bundle{
RootPath: rootPath,
Config: config.Root{
Path: rootPath,
Artifacts: map[string]*config.Artifact{
"test": {
Type: "custom",
Expand Down Expand Up @@ -72,8 +72,8 @@ func TestExpandGlobFilesSourceWithNoMatches(t *testing.T) {
require.NoError(t, err)

b := &bundle.Bundle{
RootPath: rootPath,
Config: config.Root{
Path: rootPath,
Artifacts: map[string]*config.Artifact{
"test": {
Type: "custom",
Expand Down
6 changes: 3 additions & 3 deletions bundle/artifacts/whl/autodetect.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,21 @@ func (m *detectPkg) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostic
log.Infof(ctx, "Detecting Python wheel project...")

// checking if there is setup.py in the bundle root
setupPy := filepath.Join(b.Config.Path, "setup.py")
setupPy := filepath.Join(b.RootPath, "setup.py")
_, err := os.Stat(setupPy)
if err != nil {
log.Infof(ctx, "No Python wheel project found at bundle root folder")
return nil
}

log.Infof(ctx, fmt.Sprintf("Found Python wheel project at %s", b.Config.Path))
log.Infof(ctx, fmt.Sprintf("Found Python wheel project at %s", b.RootPath))
module := extractModuleName(setupPy)

if b.Config.Artifacts == nil {
b.Config.Artifacts = make(map[string]*config.Artifact)
}

pkgPath, err := filepath.Abs(b.Config.Path)
pkgPath, err := filepath.Abs(b.RootPath)
if err != nil {
return diag.FromErr(err)
}
Expand Down
2 changes: 1 addition & 1 deletion bundle/artifacts/whl/from_libraries.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func (*fromLibraries) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnost
tasks := libraries.FindAllWheelTasksWithLocalLibraries(b)
for _, task := range tasks {
for _, lib := range task.Libraries {
matches, err := filepath.Glob(filepath.Join(b.Config.Path, lib.Whl))
matches, err := filepath.Glob(filepath.Join(b.RootPath, lib.Whl))
// File referenced from libraries section does not exists, skipping
if err != nil {
continue
Expand Down
15 changes: 10 additions & 5 deletions bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ import (
const internalFolder = ".internal"

type Bundle struct {
// RootPath contains the directory path to the root of the bundle.
// It is set when we instantiate a new bundle instance.
RootPath string

Config config.Root

// Metadata about the bundle deployment. This is the interface Databricks services
Expand Down Expand Up @@ -63,7 +67,9 @@ type Bundle struct {
}

func Load(ctx context.Context, path string) (*Bundle, error) {
b := &Bundle{}
b := &Bundle{
RootPath: filepath.Clean(path),
}
stat, err := os.Stat(path)
if err != nil {
return nil, err
Expand All @@ -75,7 +81,6 @@ func Load(ctx context.Context, path string) (*Bundle, error) {
if hasRootEnv && hasIncludesEnv && stat.IsDir() {
log.Debugf(ctx, "No bundle configuration; using bundle root: %s", path)
b.Config = config.Root{
Path: path,
Bundle: config.Bundle{
Name: filepath.Base(path),
},
Expand Down Expand Up @@ -158,7 +163,7 @@ func (b *Bundle) CacheDir(ctx context.Context, paths ...string) (string, error)
if !exists || cacheDirName == "" {
cacheDirName = filepath.Join(
// Anchor at bundle root directory.
b.Config.Path,
b.RootPath,
// Static cache directory.
".databricks",
"bundle",
Expand Down Expand Up @@ -210,15 +215,15 @@ func (b *Bundle) GetSyncIncludePatterns(ctx context.Context) ([]string, error) {
if err != nil {
return nil, err
}
internalDirRel, err := filepath.Rel(b.Config.Path, internalDir)
internalDirRel, err := filepath.Rel(b.RootPath, internalDir)
if err != nil {
return nil, err
}
return append(b.Config.Sync.Include, filepath.ToSlash(filepath.Join(internalDirRel, "*.*"))), nil
}

func (b *Bundle) GitRepository() (*git.Repository, error) {
rootPath, err := folders.FindDirWithLeaf(b.Config.Path, ".git")
rootPath, err := folders.FindDirWithLeaf(b.RootPath, ".git")
if err != nil {
return nil, fmt.Errorf("unable to locate repository root: %w", err)
}
Expand Down
4 changes: 2 additions & 2 deletions bundle/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func TestBundleMustLoadSuccess(t *testing.T) {
t.Setenv(env.RootVariable, "./tests/basic")
b, err := MustLoad(context.Background())
require.NoError(t, err)
assert.Equal(t, "tests/basic", filepath.ToSlash(b.Config.Path))
assert.Equal(t, "tests/basic", filepath.ToSlash(b.RootPath))
}

func TestBundleMustLoadFailureWithEnv(t *testing.T) {
Expand All @@ -96,7 +96,7 @@ func TestBundleTryLoadSuccess(t *testing.T) {
t.Setenv(env.RootVariable, "./tests/basic")
b, err := TryLoad(context.Background())
require.NoError(t, err)
assert.Equal(t, "tests/basic", filepath.ToSlash(b.Config.Path))
assert.Equal(t, "tests/basic", filepath.ToSlash(b.RootPath))
}

func TestBundleTryLoadFailureWithEnv(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion bundle/config/mutator/expand_pipeline_glob_paths_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ func TestExpandGlobPathsInPipelines(t *testing.T) {
touchEmptyFile(t, filepath.Join(dir, "skip/test7.py"))

b := &bundle.Bundle{
RootPath: dir,
Config: config.Root{
Path: dir,
Resources: config.Resources{
Pipelines: map[string]*resources.Pipeline{
"pipeline": {
Expand Down
4 changes: 2 additions & 2 deletions bundle/config/mutator/load_git_details.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func (m *loadGitDetails) Name() string {

func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
// Load relevant git repository
repo, err := git.NewRepository(b.Config.Path)
repo, err := git.NewRepository(b.RootPath)
if err != nil {
return diag.FromErr(err)
}
Expand Down Expand Up @@ -56,7 +56,7 @@ func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagn
}

// Compute relative path of the bundle root from the Git repo root.
absBundlePath, err := filepath.Abs(b.Config.Path)
absBundlePath, err := filepath.Abs(b.RootPath)
if err != nil {
return diag.FromErr(err)
}
Expand Down
4 changes: 2 additions & 2 deletions bundle/config/mutator/process_include_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,16 @@ import (

func TestProcessInclude(t *testing.T) {
b := &bundle.Bundle{
RootPath: t.TempDir(),
Config: config.Root{
Path: t.TempDir(),
Workspace: config.Workspace{
Host: "foo",
},
},
}

relPath := "./file.yml"
fullPath := filepath.Join(b.Config.Path, relPath)
fullPath := filepath.Join(b.RootPath, relPath)
f, err := os.Create(fullPath)
require.NoError(t, err)
fmt.Fprint(f, "workspace:\n host: bar\n")
Expand Down
8 changes: 4 additions & 4 deletions bundle/config/mutator/process_root_includes.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag.
// Converts extra include paths from environment variable to relative paths
for _, extraIncludePath := range getExtraIncludePaths(ctx) {
if filepath.IsAbs(extraIncludePath) {
rel, err := filepath.Rel(b.Config.Path, extraIncludePath)
rel, err := filepath.Rel(b.RootPath, extraIncludePath)
if err != nil {
return diag.Errorf("unable to include file '%s': %v", extraIncludePath, err)
}
Expand All @@ -70,7 +70,7 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag.
}

// Anchor includes to the bundle root path.
matches, err := filepath.Glob(filepath.Join(b.Config.Path, entry))
matches, err := filepath.Glob(filepath.Join(b.RootPath, entry))
if err != nil {
return diag.FromErr(err)
}
Expand All @@ -84,7 +84,7 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag.
// Filter matches to ones we haven't seen yet.
var includes []string
for _, match := range matches {
rel, err := filepath.Rel(b.Config.Path, match)
rel, err := filepath.Rel(b.RootPath, match)
if err != nil {
return diag.FromErr(err)
}
Expand All @@ -99,7 +99,7 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag.
slices.Sort(includes)
files = append(files, includes...)
for _, include := range includes {
out = append(out, ProcessInclude(filepath.Join(b.Config.Path, include), include))
out = append(out, ProcessInclude(filepath.Join(b.RootPath, include), include))
}
}

Expand Down
34 changes: 14 additions & 20 deletions bundle/config/mutator/process_root_includes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ import (

func TestProcessRootIncludesEmpty(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Path: ".",
},
RootPath: ".",
}
diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes())
require.NoError(t, diags.Error())
Expand All @@ -36,8 +34,8 @@ func TestProcessRootIncludesAbs(t *testing.T) {
}

b := &bundle.Bundle{
RootPath: ".",
Config: config.Root{
Path: ".",
Include: []string{
"/tmp/*.yml",
},
Expand All @@ -50,17 +48,17 @@ func TestProcessRootIncludesAbs(t *testing.T) {

func TestProcessRootIncludesSingleGlob(t *testing.T) {
b := &bundle.Bundle{
RootPath: t.TempDir(),
Config: config.Root{
Path: t.TempDir(),
Include: []string{
"*.yml",
},
},
}

testutil.Touch(t, b.Config.Path, "databricks.yml")
testutil.Touch(t, b.Config.Path, "a.yml")
testutil.Touch(t, b.Config.Path, "b.yml")
testutil.Touch(t, b.RootPath, "databricks.yml")
testutil.Touch(t, b.RootPath, "a.yml")
testutil.Touch(t, b.RootPath, "b.yml")

diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes())
require.NoError(t, diags.Error())
Expand All @@ -69,17 +67,17 @@ func TestProcessRootIncludesSingleGlob(t *testing.T) {

func TestProcessRootIncludesMultiGlob(t *testing.T) {
b := &bundle.Bundle{
RootPath: t.TempDir(),
Config: config.Root{
Path: t.TempDir(),
Include: []string{
"a*.yml",
"b*.yml",
},
},
}

testutil.Touch(t, b.Config.Path, "a1.yml")
testutil.Touch(t, b.Config.Path, "b1.yml")
testutil.Touch(t, b.RootPath, "a1.yml")
testutil.Touch(t, b.RootPath, "b1.yml")

diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes())
require.NoError(t, diags.Error())
Expand All @@ -88,16 +86,16 @@ func TestProcessRootIncludesMultiGlob(t *testing.T) {

func TestProcessRootIncludesRemoveDups(t *testing.T) {
b := &bundle.Bundle{
RootPath: t.TempDir(),
Config: config.Root{
Path: t.TempDir(),
Include: []string{
"*.yml",
"*.yml",
},
},
}

testutil.Touch(t, b.Config.Path, "a.yml")
testutil.Touch(t, b.RootPath, "a.yml")

diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes())
require.NoError(t, diags.Error())
Expand All @@ -106,8 +104,8 @@ func TestProcessRootIncludesRemoveDups(t *testing.T) {

func TestProcessRootIncludesNotExists(t *testing.T) {
b := &bundle.Bundle{
RootPath: t.TempDir(),
Config: config.Root{
Path: t.TempDir(),
Include: []string{
"notexist.yml",
},
Expand All @@ -125,9 +123,7 @@ func TestProcessRootIncludesExtrasFromEnvVar(t *testing.T) {
t.Setenv(env.IncludesVariable, path.Join(rootPath, testYamlName))

b := &bundle.Bundle{
Config: config.Root{
Path: rootPath,
},
RootPath: rootPath,
}

diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes())
Expand All @@ -148,9 +144,7 @@ func TestProcessRootIncludesDedupExtrasFromEnvVar(t *testing.T) {
))

b := &bundle.Bundle{
Config: config.Root{
Path: rootPath,
},
RootPath: rootPath,
}

diags := bundle.Apply(context.Background(), b, mutator.ProcessRootIncludes())
Expand Down
4 changes: 2 additions & 2 deletions bundle/config/mutator/rewrite_sync_paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@ func (m *rewriteSyncPaths) makeRelativeTo(root string) dyn.MapFunc {
func (m *rewriteSyncPaths) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) {
return dyn.Map(v, "sync", func(_ dyn.Path, v dyn.Value) (nv dyn.Value, err error) {
v, err = dyn.Map(v, "include", dyn.Foreach(m.makeRelativeTo(b.Config.Path)))
v, err = dyn.Map(v, "include", dyn.Foreach(m.makeRelativeTo(b.RootPath)))
if err != nil {
return dyn.NilValue, err
}
v, err = dyn.Map(v, "exclude", dyn.Foreach(m.makeRelativeTo(b.Config.Path)))
v, err = dyn.Map(v, "exclude", dyn.Foreach(m.makeRelativeTo(b.RootPath)))
if err != nil {
return dyn.NilValue, err
}
Expand Down
10 changes: 4 additions & 6 deletions bundle/config/mutator/rewrite_sync_paths_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import (

func TestRewriteSyncPathsRelative(t *testing.T) {
b := &bundle.Bundle{
RootPath: ".",
Config: config.Root{
Path: ".",
Sync: config.Sync{
Include: []string{
"foo",
Expand Down Expand Up @@ -45,8 +45,8 @@ func TestRewriteSyncPathsRelative(t *testing.T) {

func TestRewriteSyncPathsAbsolute(t *testing.T) {
b := &bundle.Bundle{
RootPath: "/tmp/dir",
Config: config.Root{
Path: "/tmp/dir",
Sync: config.Sync{
Include: []string{
"foo",
Expand Down Expand Up @@ -77,9 +77,7 @@ func TestRewriteSyncPathsAbsolute(t *testing.T) {
func TestRewriteSyncPathsErrorPaths(t *testing.T) {
t.Run("no sync block", func(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Path: ".",
},
RootPath: ".",
}

diags := bundle.Apply(context.Background(), b, mutator.RewriteSyncPaths())
Expand All @@ -88,8 +86,8 @@ func TestRewriteSyncPathsErrorPaths(t *testing.T) {

t.Run("empty include/exclude blocks", func(t *testing.T) {
b := &bundle.Bundle{
RootPath: ".",
Config: config.Root{
Path: ".",
Sync: config.Sync{
Include: []string{},
Exclude: []string{},
Expand Down
2 changes: 1 addition & 1 deletion bundle/config/mutator/trampoline.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func (m *trampoline) generateNotebookWrapper(ctx context.Context, b *bundle.Bund
return err
}

internalDirRel, err := filepath.Rel(b.Config.Path, internalDir)
internalDirRel, err := filepath.Rel(b.RootPath, internalDir)
if err != nil {
return err
}
Expand Down
Loading

0 comments on commit 00d76d5

Please sign in to comment.