Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use vfs.Path for filesystem interaction #1554

Merged
merged 4 commits into from
Jul 3, 2024
Merged

Use vfs.Path for filesystem interaction #1554

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3367bd0
Use vfs.Path for filesystem interaction
pietern Jun 24, 2024
3f50244
Fix
pietern Jul 3, 2024
26114a5
Use existing vfs.Path instead of constructing a new one
pietern Jul 3, 2024
4227a9e
Include ToSlash
pietern Jul 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions bundle/bundle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"github.com/databricks/cli/bundle/env"
"github.com/databricks/cli/bundle/metadata"
"github.com/databricks/cli/libs/fileset"
"github.com/databricks/cli/libs/folders"
"github.com/databricks/cli/libs/git"
"github.com/databricks/cli/libs/locker"
"github.com/databricks/cli/libs/log"
Expand All @@ -36,6 +35,10 @@ type Bundle struct {
// It is set when we instantiate a new bundle instance.
RootPath string

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

Config config.Root

// Metadata about the bundle deployment. This is the interface Databricks services
Expand Down Expand Up @@ -73,7 +76,8 @@ type Bundle struct {

func Load(ctx context.Context, path string) (*Bundle, error) {
b := &Bundle{
RootPath: filepath.Clean(path),
RootPath: filepath.Clean(path),
BundleRoot: vfs.MustNew(path),
}
configFile, err := config.FileNames.FindInPath(path)
if err != nil {
Expand Down Expand Up @@ -208,12 +212,12 @@ func (b *Bundle) GetSyncIncludePatterns(ctx context.Context) ([]string, error) {
}

func (b *Bundle) GitRepository() (*git.Repository, error) {
rootPath, err := folders.FindDirWithLeaf(b.RootPath, ".git")
_, err := vfs.FindLeafInTree(b.BundleRoot, ".git")
if err != nil {
return nil, fmt.Errorf("unable to locate repository root: %w", err)
}

return git.NewRepository(vfs.MustNew(rootPath))
return git.NewRepository(b.BundleRoot)
}

// AuthEnv returns a map with environment variables and their values
Expand Down
5 changes: 5 additions & 0 deletions bundle/bundle_read_only.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"

"github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/libs/vfs"
"github.com/databricks/databricks-sdk-go"
)

Expand All @@ -23,6 +24,10 @@ func (r ReadOnlyBundle) RootPath() string {
return r.b.RootPath
}

func (r ReadOnlyBundle) BundleRoot() vfs.Path {
return r.b.BundleRoot
}

func (r ReadOnlyBundle) WorkspaceClient() *databricks.WorkspaceClient {
return r.b.WorkspaceClient()
}
Expand Down
3 changes: 1 addition & 2 deletions bundle/config/mutator/load_git_details.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/git"
"github.com/databricks/cli/libs/log"
"github.com/databricks/cli/libs/vfs"
)

type loadGitDetails struct{}
Expand All @@ -23,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(vfs.MustNew(b.RootPath))
repo, err := git.NewRepository(b.BundleRoot)
if err != nil {
return diag.FromErr(err)
}
Expand Down
7 changes: 3 additions & 4 deletions bundle/config/mutator/translate_paths.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"fmt"
"io/fs"
"net/url"
"os"
"path"
"path/filepath"
"strings"
Expand Down Expand Up @@ -119,7 +118,7 @@ func (t *translateContext) rewritePath(
}

func (t *translateContext) translateNotebookPath(literal, localFullPath, localRelPath, remotePath string) (string, error) {
nb, _, err := notebook.Detect(localFullPath)
nb, _, err := notebook.DetectWithFS(t.b.BundleRoot, filepath.ToSlash(localRelPath))
if errors.Is(err, fs.ErrNotExist) {
return "", fmt.Errorf("notebook %s not found", literal)
}
Expand All @@ -135,7 +134,7 @@ func (t *translateContext) translateNotebookPath(literal, localFullPath, localRe
}

func (t *translateContext) translateFilePath(literal, localFullPath, localRelPath, remotePath string) (string, error) {
nb, _, err := notebook.Detect(localFullPath)
nb, _, err := notebook.DetectWithFS(t.b.BundleRoot, filepath.ToSlash(localRelPath))
if errors.Is(err, fs.ErrNotExist) {
return "", fmt.Errorf("file %s not found", literal)
}
Expand All @@ -149,7 +148,7 @@ func (t *translateContext) translateFilePath(literal, localFullPath, localRelPat
}

func (t *translateContext) translateDirectoryPath(literal, localFullPath, localRelPath, remotePath string) (string, error) {
info, err := os.Stat(localFullPath)
info, err := t.b.BundleRoot.Stat(filepath.ToSlash(localRelPath))
if err != nil {
return "", err
}
Expand Down
40 changes: 27 additions & 13 deletions bundle/config/mutator/translate_paths_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/databricks/cli/bundle/config/mutator"
"github.com/databricks/cli/bundle/config/resources"
"github.com/databricks/cli/bundle/internal/bundletest"
"github.com/databricks/cli/libs/vfs"
"github.com/databricks/databricks-sdk-go/service/compute"
"github.com/databricks/databricks-sdk-go/service/jobs"
"github.com/databricks/databricks-sdk-go/service/pipelines"
Expand All @@ -37,7 +38,8 @@ func touchEmptyFile(t *testing.T, path string) {
func TestTranslatePathsSkippedWithGitSource(t *testing.T) {
dir := t.TempDir()
b := &bundle.Bundle{
RootPath: dir,
RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{
Workspace: config.Workspace{
FilePath: "/bundle",
Expand Down Expand Up @@ -107,7 +109,8 @@ func TestTranslatePaths(t *testing.T) {
touchEmptyFile(t, filepath.Join(dir, "dist", "task.jar"))

b := &bundle.Bundle{
RootPath: dir,
RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{
Workspace: config.Workspace{
FilePath: "/bundle",
Expand Down Expand Up @@ -274,7 +277,8 @@ func TestTranslatePathsInSubdirectories(t *testing.T) {
touchEmptyFile(t, filepath.Join(dir, "job", "my_dbt_project", "dbt_project.yml"))

b := &bundle.Bundle{
RootPath: dir,
RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{
Workspace: config.Workspace{
FilePath: "/bundle",
Expand Down Expand Up @@ -368,7 +372,8 @@ func TestTranslatePathsOutsideBundleRoot(t *testing.T) {
dir := t.TempDir()

b := &bundle.Bundle{
RootPath: dir,
RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{
Workspace: config.Workspace{
FilePath: "/bundle",
Expand Down Expand Up @@ -401,7 +406,8 @@ func TestJobNotebookDoesNotExistError(t *testing.T) {
dir := t.TempDir()

b := &bundle.Bundle{
RootPath: dir,
RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{
Resources: config.Resources{
Jobs: map[string]*resources.Job{
Expand Down Expand Up @@ -431,7 +437,8 @@ func TestJobFileDoesNotExistError(t *testing.T) {
dir := t.TempDir()

b := &bundle.Bundle{
RootPath: dir,
RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{
Resources: config.Resources{
Jobs: map[string]*resources.Job{
Expand Down Expand Up @@ -461,7 +468,8 @@ func TestPipelineNotebookDoesNotExistError(t *testing.T) {
dir := t.TempDir()

b := &bundle.Bundle{
RootPath: dir,
RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{
Resources: config.Resources{
Pipelines: map[string]*resources.Pipeline{
Expand Down Expand Up @@ -491,7 +499,8 @@ func TestPipelineFileDoesNotExistError(t *testing.T) {
dir := t.TempDir()

b := &bundle.Bundle{
RootPath: dir,
RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{
Resources: config.Resources{
Pipelines: map[string]*resources.Pipeline{
Expand Down Expand Up @@ -522,7 +531,8 @@ func TestJobSparkPythonTaskWithNotebookSourceError(t *testing.T) {
touchNotebookFile(t, filepath.Join(dir, "my_notebook.py"))

b := &bundle.Bundle{
RootPath: dir,
RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{
Workspace: config.Workspace{
FilePath: "/bundle",
Expand Down Expand Up @@ -556,7 +566,8 @@ func TestJobNotebookTaskWithFileSourceError(t *testing.T) {
touchEmptyFile(t, filepath.Join(dir, "my_file.py"))

b := &bundle.Bundle{
RootPath: dir,
RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{
Workspace: config.Workspace{
FilePath: "/bundle",
Expand Down Expand Up @@ -590,7 +601,8 @@ func TestPipelineNotebookLibraryWithFileSourceError(t *testing.T) {
touchEmptyFile(t, filepath.Join(dir, "my_file.py"))

b := &bundle.Bundle{
RootPath: dir,
RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{
Workspace: config.Workspace{
FilePath: "/bundle",
Expand Down Expand Up @@ -624,7 +636,8 @@ func TestPipelineFileLibraryWithNotebookSourceError(t *testing.T) {
touchNotebookFile(t, filepath.Join(dir, "my_notebook.py"))

b := &bundle.Bundle{
RootPath: dir,
RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{
Workspace: config.Workspace{
FilePath: "/bundle",
Expand Down Expand Up @@ -659,7 +672,8 @@ func TestTranslatePathJobEnvironments(t *testing.T) {
touchEmptyFile(t, filepath.Join(dir, "env2.py"))

b := &bundle.Bundle{
RootPath: dir,
RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{
Resources: config.Resources{
Jobs: map[string]*resources.Job{
Expand Down
3 changes: 1 addition & 2 deletions bundle/config/validate/validate_sync_patterns.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"github.com/databricks/cli/bundle"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/fileset"
"github.com/databricks/cli/libs/vfs"
"golang.org/x/sync/errgroup"
)

Expand Down Expand Up @@ -51,7 +50,7 @@ func checkPatterns(patterns []string, path string, rb bundle.ReadOnlyBundle) (di
index := i
p := pattern
errs.Go(func() error {
fs, err := fileset.NewGlobSet(vfs.MustNew(rb.RootPath()), []string{p})
fs, err := fileset.NewGlobSet(rb.BundleRoot(), []string{p})
if err != nil {
return err
}
Expand Down
3 changes: 1 addition & 2 deletions bundle/deploy/files/sync.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/libs/sync"
"github.com/databricks/cli/libs/vfs"
)

func GetSync(ctx context.Context, rb bundle.ReadOnlyBundle) (*sync.Sync, error) {
Expand All @@ -29,7 +28,7 @@ func GetSyncOptions(ctx context.Context, rb bundle.ReadOnlyBundle) (*sync.SyncOp
}

opts := &sync.SyncOptions{
LocalPath: vfs.MustNew(rb.RootPath()),
LocalPath: rb.BundleRoot(),
RemotePath: rb.Config().Workspace.FilePath,
Include: includes,
Exclude: rb.Config().Sync.Exclude,
Expand Down
10 changes: 4 additions & 6 deletions bundle/deploy/state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"fmt"
"io"
"io/fs"
"os"
"path/filepath"
"time"

Expand Down Expand Up @@ -59,8 +58,8 @@ type entry struct {
info fs.FileInfo
}

func newEntry(path string) *entry {
info, err := os.Stat(path)
func newEntry(root vfs.Path, path string) *entry {
info, err := root.Stat(path)
if err != nil {
return &entry{path, nil}
}
Expand Down Expand Up @@ -111,11 +110,10 @@ func FromSlice(files []fileset.File) (Filelist, error) {
return f, nil
}

func (f Filelist) ToSlice(basePath string) []fileset.File {
func (f Filelist) ToSlice(root vfs.Path) []fileset.File {
var files []fileset.File
root := vfs.MustNew(basePath)
for _, file := range f {
entry := newEntry(filepath.Join(basePath, file.LocalPath))
entry := newEntry(root, filepath.ToSlash(file.LocalPath))

// Snapshots created with versions <= v0.220.0 use platform-specific
// paths (i.e. with backslashes). Files returned by [libs/fileset] always
Expand Down
2 changes: 1 addition & 1 deletion bundle/deploy/state_pull.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func (s *statePull) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostic
}

log.Infof(ctx, "Creating new snapshot")
snapshot, err := sync.NewSnapshot(state.Files.ToSlice(b.RootPath), opts)
snapshot, err := sync.NewSnapshot(state.Files.ToSlice(b.BundleRoot), opts)
if err != nil {
return diag.FromErr(err)
}
Expand Down
5 changes: 4 additions & 1 deletion bundle/deploy/state_pull_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/databricks/cli/internal/testutil"
"github.com/databricks/cli/libs/filer"
"github.com/databricks/cli/libs/sync"
"github.com/databricks/cli/libs/vfs"
"github.com/databricks/databricks-sdk-go/service/iam"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -59,8 +60,10 @@ func testStatePull(t *testing.T, opts statePullOpts) {
return f, nil
}}

tmpDir := t.TempDir()
b := &bundle.Bundle{
RootPath: t.TempDir(),
RootPath: tmpDir,
BundleRoot: vfs.MustNew(tmpDir),
Config: config.Root{
Bundle: config.Bundle{
Target: "default",
Expand Down
5 changes: 3 additions & 2 deletions bundle/deploy/state_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ func TestFromSlice(t *testing.T) {

func TestToSlice(t *testing.T) {
tmpDir := t.TempDir()
fileset := fileset.New(vfs.MustNew(tmpDir))
root := vfs.MustNew(tmpDir)
fileset := fileset.New(root)
testutil.Touch(t, tmpDir, "test1.py")
testutil.Touch(t, tmpDir, "test2.py")
testutil.Touch(t, tmpDir, "test3.py")
Expand All @@ -44,7 +45,7 @@ func TestToSlice(t *testing.T) {
require.NoError(t, err)
require.Len(t, f, 3)

s := f.ToSlice(tmpDir)
s := f.ToSlice(root)
require.Len(t, s, 3)

for _, file := range s {
Expand Down
4 changes: 3 additions & 1 deletion cmd/sync/sync_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,16 @@ import (
"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/cmd/root"
"github.com/databricks/cli/libs/vfs"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestSyncOptionsFromBundle(t *testing.T) {
tempDir := t.TempDir()
b := &bundle.Bundle{
RootPath: tempDir,
RootPath: tempDir,
BundleRoot: vfs.MustNew(tempDir),
Config: config.Root{
Bundle: config.Bundle{
Target: "default",
Expand Down
Loading