Skip to content

Commit

Permalink
Rename RootPath -> BundleRootPath (#1792)
Browse files Browse the repository at this point in the history
## Changes

After introducing the `SyncRootPath` field on the bundle (#1694), the
previous `RootPath` became ambiguous. Does it mean the bundle root path
or the sync root path? This PR renames to field to `BundleRootPath` to
remove the ambiguity.

## Tests

n/a

---------

Co-authored-by: shreyas-goenka <[email protected]>
  • Loading branch information
pietern and shreyas-goenka authored Sep 27, 2024
1 parent 56cd96c commit 1d1aa0a
Show file tree
Hide file tree
Showing 37 changed files with 112 additions and 108 deletions.
6 changes: 3 additions & 3 deletions bundle/artifacts/expand_globs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func TestExpandGlobs_Nominal(t *testing.T) {
testutil.Touch(t, tmpDir, "bc.txt")

b := &bundle.Bundle{
RootPath: tmpDir,
BundleRootPath: tmpDir,
Config: config.Root{
Artifacts: config.Artifacts{
"test": {
Expand Down Expand Up @@ -63,7 +63,7 @@ func TestExpandGlobs_InvalidPattern(t *testing.T) {
tmpDir := t.TempDir()

b := &bundle.Bundle{
RootPath: tmpDir,
BundleRootPath: tmpDir,
Config: config.Root{
Artifacts: config.Artifacts{
"test": {
Expand Down Expand Up @@ -111,7 +111,7 @@ func TestExpandGlobs_NoMatches(t *testing.T) {
testutil.Touch(t, tmpDir, "b2.txt")

b := &bundle.Bundle{
RootPath: tmpDir,
BundleRootPath: tmpDir,
Config: config.Root{
Artifacts: config.Artifacts{
"test": {
Expand Down
2 changes: 1 addition & 1 deletion bundle/artifacts/prepare.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (m *prepare) 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.RootPath
artifact.Path = b.BundleRootPath
}

if !filepath.IsAbs(artifact.Path) {
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.RootPath, "setup.py")
setupPy := filepath.Join(b.BundleRootPath, "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.RootPath))
log.Infof(ctx, fmt.Sprintf("Found Python wheel project at %s", b.BundleRootPath))
module := extractModuleName(setupPy)

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

pkgPath, err := filepath.Abs(b.RootPath)
pkgPath, err := filepath.Abs(b.BundleRootPath)
if err != nil {
return diag.FromErr(err)
}
Expand Down
30 changes: 17 additions & 13 deletions bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,22 +31,26 @@ import (
const internalFolder = ".internal"

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

// BundleRoot is a virtual filesystem path to the root of the bundle.
// BundleRoot is a virtual filesystem path to [BundleRootPath].
// Exclusively use this field for filesystem operations.
BundleRoot vfs.Path

// SyncRoot is a virtual filesystem path to the root directory of the files that are synchronized to the workspace.
// It can be an ancestor to [BundleRoot], but not a descendant; that is, [SyncRoot] must contain [BundleRoot].
SyncRoot vfs.Path

// SyncRootPath is the local path to the root directory of files that are synchronized to the workspace.
// It is equal to `SyncRoot.Native()` and included as dedicated field for convenient access.
// By default, it is the same as [BundleRootPath].
// If it is different, it must be an ancestor to [BundleRootPath].
// That is, [SyncRootPath] must contain [BundleRootPath].
SyncRootPath string

// SyncRoot is a virtual filesystem path to [SyncRootPath].
// Exclusively use this field for filesystem operations.
SyncRoot vfs.Path

// Config contains the bundle configuration.
// It is loaded from the bundle configuration files and mutators may update it.
Config config.Root

// Metadata about the bundle deployment. This is the interface Databricks services
Expand Down Expand Up @@ -84,14 +88,14 @@ type Bundle struct {

func Load(ctx context.Context, path string) (*Bundle, error) {
b := &Bundle{
RootPath: filepath.Clean(path),
BundleRoot: vfs.MustNew(path),
BundleRootPath: filepath.Clean(path),
BundleRoot: vfs.MustNew(path),
}
configFile, err := config.FileNames.FindInPath(path)
if err != nil {
return nil, err
}
log.Debugf(ctx, "Found bundle root at %s (file %s)", b.RootPath, configFile)
log.Debugf(ctx, "Found bundle root at %s (file %s)", b.BundleRootPath, configFile)
return b, nil
}

Expand Down Expand Up @@ -160,7 +164,7 @@ func (b *Bundle) CacheDir(ctx context.Context, paths ...string) (string, error)
if !exists || cacheDirName == "" {
cacheDirName = filepath.Join(
// Anchor at bundle root directory.
b.RootPath,
b.BundleRootPath,
// Static cache directory.
".databricks",
"bundle",
Expand Down Expand Up @@ -212,7 +216,7 @@ func (b *Bundle) GetSyncIncludePatterns(ctx context.Context) ([]string, error) {
if err != nil {
return nil, err
}
internalDirRel, err := filepath.Rel(b.RootPath, internalDir)
internalDirRel, err := filepath.Rel(b.BundleRootPath, internalDir)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion bundle/bundle_read_only.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func (r ReadOnlyBundle) Config() config.Root {
}

func (r ReadOnlyBundle) RootPath() string {
return r.b.RootPath
return r.b.BundleRootPath
}

func (r ReadOnlyBundle) BundleRoot() vfs.Path {
Expand Down
4 changes: 2 additions & 2 deletions bundle/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,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.RootPath))
assert.Equal(t, "tests/basic", filepath.ToSlash(b.BundleRootPath))
}

func TestBundleMustLoadFailureWithEnv(t *testing.T) {
Expand All @@ -98,7 +98,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.RootPath))
assert.Equal(t, "tests/basic", filepath.ToSlash(b.BundleRootPath))
}

func TestBundleTryLoadFailureWithEnv(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion bundle/config/loader/entry_point.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func (m *entryPoint) Name() string {
}

func (m *entryPoint) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics {
path, err := config.FileNames.FindInPath(b.RootPath)
path, err := config.FileNames.FindInPath(b.BundleRootPath)
if err != nil {
return diag.FromErr(err)
}
Expand Down
2 changes: 1 addition & 1 deletion bundle/config/loader/entry_point_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func TestEntryPointNoRootPath(t *testing.T) {

func TestEntryPoint(t *testing.T) {
b := &bundle.Bundle{
RootPath: "testdata",
BundleRootPath: "testdata",
}
diags := bundle.Apply(context.Background(), b, loader.EntryPoint())
require.NoError(t, diags.Error())
Expand Down
4 changes: 2 additions & 2 deletions bundle/config/loader/process_include_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@ import (

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

m := loader.ProcessInclude(filepath.Join(b.RootPath, "host.yml"), "host.yml")
m := loader.ProcessInclude(filepath.Join(b.BundleRootPath, "host.yml"), "host.yml")
assert.Equal(t, "ProcessInclude(host.yml)", m.Name())

// Assert the host value prior to applying the mutator
Expand Down
6 changes: 3 additions & 3 deletions bundle/config/loader/process_root_includes.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,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.RootPath, entry))
matches, err := filepath.Glob(filepath.Join(b.BundleRootPath, entry))
if err != nil {
return diag.FromErr(err)
}
Expand All @@ -61,7 +61,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.RootPath, match)
rel, err := filepath.Rel(b.BundleRootPath, match)
if err != nil {
return diag.FromErr(err)
}
Expand All @@ -76,7 +76,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.RootPath, include), include))
out = append(out, ProcessInclude(filepath.Join(b.BundleRootPath, include), include))
}
}

Expand Down
24 changes: 12 additions & 12 deletions bundle/config/loader/process_root_includes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (

func TestProcessRootIncludesEmpty(t *testing.T) {
b := &bundle.Bundle{
RootPath: ".",
BundleRootPath: ".",
}
diags := bundle.Apply(context.Background(), b, loader.ProcessRootIncludes())
require.NoError(t, diags.Error())
Expand All @@ -30,7 +30,7 @@ func TestProcessRootIncludesAbs(t *testing.T) {
}

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

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

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

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

func TestProcessRootIncludesMultiGlob(t *testing.T) {
b := &bundle.Bundle{
RootPath: t.TempDir(),
BundleRootPath: t.TempDir(),
Config: config.Root{
Include: []string{
"a*.yml",
Expand All @@ -72,8 +72,8 @@ func TestProcessRootIncludesMultiGlob(t *testing.T) {
},
}

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

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

func TestProcessRootIncludesRemoveDups(t *testing.T) {
b := &bundle.Bundle{
RootPath: t.TempDir(),
BundleRootPath: t.TempDir(),
Config: config.Root{
Include: []string{
"*.yml",
Expand All @@ -91,7 +91,7 @@ func TestProcessRootIncludesRemoveDups(t *testing.T) {
},
}

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

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

func TestProcessRootIncludesNotExists(t *testing.T) {
b := &bundle.Bundle{
RootPath: t.TempDir(),
BundleRootPath: t.TempDir(),
Config: config.Root{
Include: []string{
"notexist.yml",
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 @@ -42,7 +42,7 @@ func TestExpandGlobPathsInPipelines(t *testing.T) {
touchEmptyFile(t, filepath.Join(dir, "skip/test7.py"))

b := &bundle.Bundle{
RootPath: dir,
BundleRootPath: dir,
Config: config.Root{
Resources: config.Resources{
Pipelines: map[string]*resources.Pipeline{
Expand Down
2 changes: 1 addition & 1 deletion bundle/config/mutator/load_git_details.go
Original file line number Diff line number Diff line change
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.RootPath)
absBundlePath, err := filepath.Abs(b.BundleRootPath)
if err != nil {
return diag.FromErr(err)
}
Expand Down
2 changes: 1 addition & 1 deletion bundle/config/mutator/python/python_mutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func (m *pythonMutator) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagno
return dyn.InvalidValue, fmt.Errorf("failed to create cache dir: %w", err)
}

rightRoot, diags := m.runPythonMutator(ctx, cacheDir, b.RootPath, pythonPath, leftRoot)
rightRoot, diags := m.runPythonMutator(ctx, cacheDir, b.BundleRootPath, pythonPath, leftRoot)
mutateDiags = diags
if diags.HasError() {
return dyn.InvalidValue, mutateDiagsHasError
Expand Down
6 changes: 3 additions & 3 deletions bundle/config/mutator/rewrite_sync_paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,15 @@ 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, "paths", dyn.Foreach(m.makeRelativeTo(b.RootPath)))
v, err = dyn.Map(v, "paths", dyn.Foreach(m.makeRelativeTo(b.BundleRootPath)))
if err != nil {
return dyn.InvalidValue, err
}
v, err = dyn.Map(v, "include", dyn.Foreach(m.makeRelativeTo(b.RootPath)))
v, err = dyn.Map(v, "include", dyn.Foreach(m.makeRelativeTo(b.BundleRootPath)))
if err != nil {
return dyn.InvalidValue, err
}
v, err = dyn.Map(v, "exclude", dyn.Foreach(m.makeRelativeTo(b.RootPath)))
v, err = dyn.Map(v, "exclude", dyn.Foreach(m.makeRelativeTo(b.BundleRootPath)))
if err != nil {
return dyn.InvalidValue, err
}
Expand Down
8 changes: 4 additions & 4 deletions bundle/config/mutator/rewrite_sync_paths_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (

func TestRewriteSyncPathsRelative(t *testing.T) {
b := &bundle.Bundle{
RootPath: ".",
BundleRootPath: ".",
Config: config.Root{
Sync: config.Sync{
Paths: []string{
Expand Down Expand Up @@ -54,7 +54,7 @@ func TestRewriteSyncPathsRelative(t *testing.T) {

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

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

t.Run("empty include/exclude blocks", func(t *testing.T) {
b := &bundle.Bundle{
RootPath: ".",
BundleRootPath: ".",
Config: config.Root{
Sync: config.Sync{
Include: []string{},
Expand Down
Loading

0 comments on commit 1d1aa0a

Please sign in to comment.