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.

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

Abstract over filesystem interaction with libs/vfs #1452

Merged
merged 23 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/databricks/cli/libs/log"
"github.com/databricks/cli/libs/tags"
"github.com/databricks/cli/libs/terraform"
"github.com/databricks/cli/libs/vfs"
"github.com/databricks/databricks-sdk-go"
sdkconfig "github.com/databricks/databricks-sdk-go/config"
"github.com/hashicorp/terraform-exec/tfexec"
Expand Down Expand Up @@ -208,7 +209,7 @@ func (b *Bundle) GitRepository() (*git.Repository, error) {
return nil, fmt.Errorf("unable to locate repository root: %w", err)
}

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

// AuthEnv returns a map with environment variables and their values
Expand Down
3 changes: 2 additions & 1 deletion bundle/config/mutator/load_git_details.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ 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 @@ -22,7 +23,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.RootPath)
repo, err := git.NewRepository(vfs.MustNew(b.RootPath))
if err != nil {
return diag.FromErr(err)
}
Expand Down
3 changes: 2 additions & 1 deletion bundle/config/validate/validate_sync_patterns.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ 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 @@ -50,7 +51,7 @@ func checkPatterns(patterns []string, path string, rb bundle.ReadOnlyBundle) (di
index := i
p := pattern
errs.Go(func() error {
fs, err := fileset.NewGlobSet(rb.RootPath(), []string{p})
fs, err := fileset.NewGlobSet(vfs.MustNew(rb.RootPath()), []string{p})
if err != nil {
return err
}
Expand Down
3 changes: 2 additions & 1 deletion bundle/deploy/files/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ 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 @@ -28,7 +29,7 @@ func GetSyncOptions(ctx context.Context, rb bundle.ReadOnlyBundle) (*sync.SyncOp
}

opts := &sync.SyncOptions{
LocalPath: rb.RootPath(),
LocalPath: vfs.MustNew(rb.RootPath()),
RemotePath: rb.Config().Workspace.FilePath,
Include: includes,
Exclude: rb.Config().Sync.Exclude,
Expand Down
13 changes: 10 additions & 3 deletions bundle/deploy/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

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

const DeploymentStateFileName = "deployment.json"
Expand Down Expand Up @@ -112,12 +113,18 @@ func FromSlice(files []fileset.File) (Filelist, error) {

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

// Snapshots created with versions <= v0.220.0 use platform-specific
// paths (i.e. with backslashes). Files returned by [libs/fileset] always
// contain forward slashes after this version. Normalize before using.
relative := filepath.ToSlash(file.LocalPath)
if file.IsNotebook {
files = append(files, fileset.NewNotebookFile(newEntry(absPath), absPath, file.LocalPath))
files = append(files, fileset.NewNotebookFile(root, entry, relative))
} else {
files = append(files, fileset.NewSourceFile(newEntry(absPath), absPath, file.LocalPath))
files = append(files, fileset.NewSourceFile(root, entry, relative))
}
}
return files
Expand Down
23 changes: 8 additions & 15 deletions bundle/deploy/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,17 @@ package deploy
import (
"bytes"
"encoding/json"
"path/filepath"
"testing"

"github.com/databricks/cli/internal/testutil"
"github.com/databricks/cli/libs/fileset"
"github.com/databricks/cli/libs/vfs"
"github.com/stretchr/testify/require"
)

func TestFromSlice(t *testing.T) {
tmpDir := t.TempDir()
fileset := fileset.New(tmpDir)
fileset := fileset.New(vfs.MustNew(tmpDir))
testutil.Touch(t, tmpDir, "test1.py")
testutil.Touch(t, tmpDir, "test2.py")
testutil.Touch(t, tmpDir, "test3.py")
Expand All @@ -32,7 +32,7 @@ func TestFromSlice(t *testing.T) {

func TestToSlice(t *testing.T) {
tmpDir := t.TempDir()
fileset := fileset.New(tmpDir)
fileset := fileset.New(vfs.MustNew(tmpDir))
testutil.Touch(t, tmpDir, "test1.py")
testutil.Touch(t, tmpDir, "test2.py")
testutil.Touch(t, tmpDir, "test3.py")
Expand All @@ -48,18 +48,11 @@ func TestToSlice(t *testing.T) {
require.Len(t, s, 3)

for _, file := range s {
require.Contains(t, []string{"test1.py", "test2.py", "test3.py"}, file.Name())
require.Contains(t, []string{
filepath.Join(tmpDir, "test1.py"),
filepath.Join(tmpDir, "test2.py"),
filepath.Join(tmpDir, "test3.py"),
}, file.Absolute)
require.False(t, file.IsDir())
require.NotZero(t, file.Type())
info, err := file.Info()
require.NoError(t, err)
require.NotNil(t, info)
require.Equal(t, file.Name(), info.Name())
require.Contains(t, []string{"test1.py", "test2.py", "test3.py"}, file.Relative)

// If the mtime is not zero we know we produced a valid fs.DirEntry.
ts := file.Modified()
require.NotZero(t, ts)
}
}

Expand Down
3 changes: 2 additions & 1 deletion cmd/sync/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/databricks/cli/cmd/root"
"github.com/databricks/cli/libs/flags"
"github.com/databricks/cli/libs/sync"
"github.com/databricks/cli/libs/vfs"
"github.com/spf13/cobra"
)

Expand Down Expand Up @@ -46,7 +47,7 @@ func (f *syncFlags) syncOptionsFromArgs(cmd *cobra.Command, args []string) (*syn
}

opts := sync.SyncOptions{
LocalPath: args[0],
LocalPath: vfs.MustNew(args[0]),
pietern marked this conversation as resolved.
Show resolved Hide resolved
RemotePath: args[1],
Full: f.full,
PollInterval: f.interval,
Expand Down
11 changes: 7 additions & 4 deletions cmd/sync/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func TestSyncOptionsFromBundle(t *testing.T) {
f := syncFlags{}
opts, err := f.syncOptionsFromBundle(New(), []string{}, b)
require.NoError(t, err)
assert.Equal(t, tempDir, opts.LocalPath)
assert.Equal(t, tempDir, opts.LocalPath.Native())
assert.Equal(t, "/Users/jane@doe.com/path", opts.RemotePath)
assert.Equal(t, filepath.Join(tempDir, ".databricks", "bundle", "default"), opts.SnapshotBasePath)
assert.NotNil(t, opts.WorkspaceClient)
Expand All @@ -49,11 +49,14 @@ func TestSyncOptionsFromArgsRequiredTwoArgs(t *testing.T) {
}

func TestSyncOptionsFromArgs(t *testing.T) {
local := t.TempDir()
remote := "/remote"

f := syncFlags{}
cmd := New()
cmd.SetContext(root.SetWorkspaceClient(context.Background(), nil))
opts, err := f.syncOptionsFromArgs(cmd, []string{"/local", "/remote"})
opts, err := f.syncOptionsFromArgs(cmd, []string{local, remote})
require.NoError(t, err)
assert.Equal(t, "/local", opts.LocalPath)
assert.Equal(t, "/remote", opts.RemotePath)
assert.Equal(t, local, opts.LocalPath.Native())
assert.Equal(t, remote, opts.RemotePath)
}
6 changes: 3 additions & 3 deletions internal/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ func TestAccSyncNestedFolderSync(t *testing.T) {
assertSync.remoteDirContent(ctx, "dir1", []string{"dir2"})
assertSync.remoteDirContent(ctx, "dir1/dir2", []string{"dir3"})
assertSync.remoteDirContent(ctx, "dir1/dir2/dir3", []string{"foo.txt"})
assertSync.snapshotContains(append(repoFiles, ".gitignore", filepath.FromSlash("dir1/dir2/dir3/foo.txt")))
assertSync.snapshotContains(append(repoFiles, ".gitignore", "dir1/dir2/dir3/foo.txt"))

// delete
f.Remove(t)
Expand Down Expand Up @@ -374,7 +374,7 @@ func TestAccSyncNestedSpacePlusAndHashAreEscapedSync(t *testing.T) {
assertSync.remoteDirContent(ctx, "dir1", []string{"a b+c"})
assertSync.remoteDirContent(ctx, "dir1/a b+c", []string{"c+d e"})
assertSync.remoteDirContent(ctx, "dir1/a b+c/c+d e", []string{"e+f g#i.txt"})
assertSync.snapshotContains(append(repoFiles, ".gitignore", filepath.FromSlash("dir1/a b+c/c+d e/e+f g#i.txt")))
assertSync.snapshotContains(append(repoFiles, ".gitignore", "dir1/a b+c/c+d e/e+f g#i.txt"))

// delete
f.Remove(t)
Expand Down Expand Up @@ -404,7 +404,7 @@ func TestAccSyncIncrementalFileOverwritesFolder(t *testing.T) {
assertSync.waitForCompletionMarker()
assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore", "foo"))
assertSync.remoteDirContent(ctx, "foo", []string{"bar.txt"})
assertSync.snapshotContains(append(repoFiles, ".gitignore", filepath.FromSlash("foo/bar.txt")))
assertSync.snapshotContains(append(repoFiles, ".gitignore", "foo/bar.txt"))

// delete foo/bar.txt
f.Remove(t)
Expand Down
44 changes: 27 additions & 17 deletions libs/fileset/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"time"

"github.com/databricks/cli/libs/notebook"
"github.com/databricks/cli/libs/vfs"
)

type fileType int
Expand All @@ -16,40 +17,49 @@ const (
)

type File struct {
fs.DirEntry
Absolute, Relative string
fileType fileType
// Root path of the fileset.
root vfs.Path

// File entry as returned by the [fs.WalkDir] function.
entry fs.DirEntry

// Type of the file.
fileType fileType

// Relative path within the fileset.
// Combine with the [vfs.Path] to interact with the underlying file.
Relative string
}

func NewNotebookFile(entry fs.DirEntry, absolute string, relative string) File {
func NewNotebookFile(root vfs.Path, entry fs.DirEntry, relative string) File {
return File{
DirEntry: entry,
Absolute: absolute,
Relative: relative,
root: root,
entry: entry,
fileType: Notebook,
Relative: relative,
}
}

func NewFile(entry fs.DirEntry, absolute string, relative string) File {
func NewFile(root vfs.Path, entry fs.DirEntry, relative string) File {
return File{
DirEntry: entry,
Absolute: absolute,
Relative: relative,
root: root,
entry: entry,
fileType: Unknown,
Relative: relative,
}
}

func NewSourceFile(entry fs.DirEntry, absolute string, relative string) File {
func NewSourceFile(root vfs.Path, entry fs.DirEntry, relative string) File {
return File{
DirEntry: entry,
Absolute: absolute,
Relative: relative,
root: root,
entry: entry,
fileType: Source,
Relative: relative,
}
}

func (f File) Modified() (ts time.Time) {
info, err := f.Info()
info, err := f.entry.Info()
if err != nil {
// return default time, beginning of epoch
return ts
Expand All @@ -63,7 +73,7 @@ func (f *File) IsNotebook() (bool, error) {
}

// Otherwise, detect the notebook type.
isNotebook, _, err := notebook.Detect(f.Absolute)
isNotebook, _, err := notebook.DetectWithFS(f.root, f.Relative)
if err != nil {
return false, err
}
Expand Down
15 changes: 8 additions & 7 deletions libs/fileset/file_test.go
Original file line number Diff line number Diff line change
@@ -1,41 +1,42 @@
package fileset

import (
"path/filepath"
"testing"

"github.com/databricks/cli/internal/testutil"
"github.com/databricks/cli/libs/vfs"
"github.com/stretchr/testify/require"
)

func TestNotebookFileIsNotebook(t *testing.T) {
f := NewNotebookFile(nil, "", "")
f := NewNotebookFile(nil, nil, "")
isNotebook, err := f.IsNotebook()
require.NoError(t, err)
require.True(t, isNotebook)
}

func TestSourceFileIsNotNotebook(t *testing.T) {
f := NewSourceFile(nil, "", "")
f := NewSourceFile(nil, nil, "")
isNotebook, err := f.IsNotebook()
require.NoError(t, err)
require.False(t, isNotebook)
}

func TestUnknownFileDetectsNotebook(t *testing.T) {
tmpDir := t.TempDir()
root := vfs.MustNew(tmpDir)

t.Run("file", func(t *testing.T) {
path := testutil.Touch(t, tmpDir, "test.py")
f := NewFile(nil, path, filepath.Base(path))
testutil.Touch(t, tmpDir, "test.py")
f := NewFile(root, nil, "test.py")
isNotebook, err := f.IsNotebook()
require.NoError(t, err)
require.False(t, isNotebook)
})

t.Run("notebook", func(t *testing.T) {
path := testutil.TouchNotebook(t, tmpDir, "notebook.py")
f := NewFile(nil, path, filepath.Base(path))
testutil.TouchNotebook(t, tmpDir, "notebook.py")
f := NewFile(root, nil, "notebook.py")
isNotebook, err := f.IsNotebook()
require.NoError(t, err)
require.True(t, isNotebook)
Expand Down
Loading
Loading