Use `vfs.Path` for filesystem interaction (#1554)

## Changes

Note: this doesn't cover _all_ filesystem interaction.

To intercept calls where read or stat files to determine their type, we
need a layer between our code and the `os` package calls that interact
with the local file system. Interception is necessary to accommodate
differences between a regular local file system and the FUSE-mounted
Workspace File System when running the CLI on DBR.

This change makes use of #1452 in the bundle struct.

It uses #1525 to access the bundle variable in path rewriting.

## Tests

* Unit tests pass.
* Integration tests pass.
This commit is contained in:
Pieter Noordhuis 2024-07-03 12:13:22 +02:00 committed by GitHub
parent 4787edba36
commit b3c044c461
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 61 additions and 38 deletions

View File

@ -17,7 +17,6 @@ import (
"github.com/databricks/cli/bundle/env" "github.com/databricks/cli/bundle/env"
"github.com/databricks/cli/bundle/metadata" "github.com/databricks/cli/bundle/metadata"
"github.com/databricks/cli/libs/fileset" "github.com/databricks/cli/libs/fileset"
"github.com/databricks/cli/libs/folders"
"github.com/databricks/cli/libs/git" "github.com/databricks/cli/libs/git"
"github.com/databricks/cli/libs/locker" "github.com/databricks/cli/libs/locker"
"github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/log"
@ -36,6 +35,10 @@ type Bundle struct {
// It is set when we instantiate a new bundle instance. // It is set when we instantiate a new bundle instance.
RootPath string 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 Config config.Root
// Metadata about the bundle deployment. This is the interface Databricks services // Metadata about the bundle deployment. This is the interface Databricks services
@ -73,7 +76,8 @@ type Bundle struct {
func Load(ctx context.Context, path string) (*Bundle, error) { func Load(ctx context.Context, path string) (*Bundle, error) {
b := &Bundle{ b := &Bundle{
RootPath: filepath.Clean(path), RootPath: filepath.Clean(path),
BundleRoot: vfs.MustNew(path),
} }
configFile, err := config.FileNames.FindInPath(path) configFile, err := config.FileNames.FindInPath(path)
if err != nil { if err != nil {
@ -208,12 +212,12 @@ func (b *Bundle) GetSyncIncludePatterns(ctx context.Context) ([]string, error) {
} }
func (b *Bundle) GitRepository() (*git.Repository, error) { func (b *Bundle) GitRepository() (*git.Repository, error) {
rootPath, err := folders.FindDirWithLeaf(b.RootPath, ".git") _, err := vfs.FindLeafInTree(b.BundleRoot, ".git")
if err != nil { if err != nil {
return nil, fmt.Errorf("unable to locate repository root: %w", err) 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 // AuthEnv returns a map with environment variables and their values

View File

@ -4,6 +4,7 @@ import (
"context" "context"
"github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/libs/vfs"
"github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go"
) )
@ -23,6 +24,10 @@ func (r ReadOnlyBundle) RootPath() string {
return r.b.RootPath return r.b.RootPath
} }
func (r ReadOnlyBundle) BundleRoot() vfs.Path {
return r.b.BundleRoot
}
func (r ReadOnlyBundle) WorkspaceClient() *databricks.WorkspaceClient { func (r ReadOnlyBundle) WorkspaceClient() *databricks.WorkspaceClient {
return r.b.WorkspaceClient() return r.b.WorkspaceClient()
} }

View File

@ -8,7 +8,6 @@ import (
"github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/git" "github.com/databricks/cli/libs/git"
"github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/log"
"github.com/databricks/cli/libs/vfs"
) )
type loadGitDetails struct{} type loadGitDetails struct{}
@ -23,7 +22,7 @@ func (m *loadGitDetails) Name() string {
func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
// Load relevant git repository // Load relevant git repository
repo, err := git.NewRepository(vfs.MustNew(b.RootPath)) repo, err := git.NewRepository(b.BundleRoot)
if err != nil { if err != nil {
return diag.FromErr(err) return diag.FromErr(err)
} }

View File

@ -6,7 +6,6 @@ import (
"fmt" "fmt"
"io/fs" "io/fs"
"net/url" "net/url"
"os"
"path" "path"
"path/filepath" "path/filepath"
"strings" "strings"
@ -119,7 +118,7 @@ func (t *translateContext) rewritePath(
} }
func (t *translateContext) translateNotebookPath(literal, localFullPath, localRelPath, remotePath string) (string, error) { 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) { if errors.Is(err, fs.ErrNotExist) {
return "", fmt.Errorf("notebook %s not found", literal) return "", fmt.Errorf("notebook %s not found", literal)
} }
@ -135,7 +134,7 @@ func (t *translateContext) translateNotebookPath(literal, localFullPath, localRe
} }
func (t *translateContext) translateFilePath(literal, localFullPath, localRelPath, remotePath string) (string, error) { 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) { if errors.Is(err, fs.ErrNotExist) {
return "", fmt.Errorf("file %s not found", literal) return "", fmt.Errorf("file %s not found", literal)
} }
@ -149,7 +148,7 @@ func (t *translateContext) translateFilePath(literal, localFullPath, localRelPat
} }
func (t *translateContext) translateDirectoryPath(literal, localFullPath, localRelPath, remotePath string) (string, error) { 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 { if err != nil {
return "", err return "", err
} }

View File

@ -12,6 +12,7 @@ import (
"github.com/databricks/cli/bundle/config/mutator" "github.com/databricks/cli/bundle/config/mutator"
"github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/bundle/config/resources"
"github.com/databricks/cli/bundle/internal/bundletest" "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/compute"
"github.com/databricks/databricks-sdk-go/service/jobs" "github.com/databricks/databricks-sdk-go/service/jobs"
"github.com/databricks/databricks-sdk-go/service/pipelines" "github.com/databricks/databricks-sdk-go/service/pipelines"
@ -37,7 +38,8 @@ func touchEmptyFile(t *testing.T, path string) {
func TestTranslatePathsSkippedWithGitSource(t *testing.T) { func TestTranslatePathsSkippedWithGitSource(t *testing.T) {
dir := t.TempDir() dir := t.TempDir()
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: dir, RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{ Config: config.Root{
Workspace: config.Workspace{ Workspace: config.Workspace{
FilePath: "/bundle", FilePath: "/bundle",
@ -107,7 +109,8 @@ func TestTranslatePaths(t *testing.T) {
touchEmptyFile(t, filepath.Join(dir, "dist", "task.jar")) touchEmptyFile(t, filepath.Join(dir, "dist", "task.jar"))
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: dir, RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{ Config: config.Root{
Workspace: config.Workspace{ Workspace: config.Workspace{
FilePath: "/bundle", FilePath: "/bundle",
@ -274,7 +277,8 @@ func TestTranslatePathsInSubdirectories(t *testing.T) {
touchEmptyFile(t, filepath.Join(dir, "job", "my_dbt_project", "dbt_project.yml")) touchEmptyFile(t, filepath.Join(dir, "job", "my_dbt_project", "dbt_project.yml"))
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: dir, RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{ Config: config.Root{
Workspace: config.Workspace{ Workspace: config.Workspace{
FilePath: "/bundle", FilePath: "/bundle",
@ -368,7 +372,8 @@ func TestTranslatePathsOutsideBundleRoot(t *testing.T) {
dir := t.TempDir() dir := t.TempDir()
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: dir, RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{ Config: config.Root{
Workspace: config.Workspace{ Workspace: config.Workspace{
FilePath: "/bundle", FilePath: "/bundle",
@ -401,7 +406,8 @@ func TestJobNotebookDoesNotExistError(t *testing.T) {
dir := t.TempDir() dir := t.TempDir()
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: dir, RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{ Config: config.Root{
Resources: config.Resources{ Resources: config.Resources{
Jobs: map[string]*resources.Job{ Jobs: map[string]*resources.Job{
@ -431,7 +437,8 @@ func TestJobFileDoesNotExistError(t *testing.T) {
dir := t.TempDir() dir := t.TempDir()
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: dir, RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{ Config: config.Root{
Resources: config.Resources{ Resources: config.Resources{
Jobs: map[string]*resources.Job{ Jobs: map[string]*resources.Job{
@ -461,7 +468,8 @@ func TestPipelineNotebookDoesNotExistError(t *testing.T) {
dir := t.TempDir() dir := t.TempDir()
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: dir, RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{ Config: config.Root{
Resources: config.Resources{ Resources: config.Resources{
Pipelines: map[string]*resources.Pipeline{ Pipelines: map[string]*resources.Pipeline{
@ -491,7 +499,8 @@ func TestPipelineFileDoesNotExistError(t *testing.T) {
dir := t.TempDir() dir := t.TempDir()
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: dir, RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{ Config: config.Root{
Resources: config.Resources{ Resources: config.Resources{
Pipelines: map[string]*resources.Pipeline{ Pipelines: map[string]*resources.Pipeline{
@ -522,7 +531,8 @@ func TestJobSparkPythonTaskWithNotebookSourceError(t *testing.T) {
touchNotebookFile(t, filepath.Join(dir, "my_notebook.py")) touchNotebookFile(t, filepath.Join(dir, "my_notebook.py"))
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: dir, RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{ Config: config.Root{
Workspace: config.Workspace{ Workspace: config.Workspace{
FilePath: "/bundle", FilePath: "/bundle",
@ -556,7 +566,8 @@ func TestJobNotebookTaskWithFileSourceError(t *testing.T) {
touchEmptyFile(t, filepath.Join(dir, "my_file.py")) touchEmptyFile(t, filepath.Join(dir, "my_file.py"))
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: dir, RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{ Config: config.Root{
Workspace: config.Workspace{ Workspace: config.Workspace{
FilePath: "/bundle", FilePath: "/bundle",
@ -590,7 +601,8 @@ func TestPipelineNotebookLibraryWithFileSourceError(t *testing.T) {
touchEmptyFile(t, filepath.Join(dir, "my_file.py")) touchEmptyFile(t, filepath.Join(dir, "my_file.py"))
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: dir, RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{ Config: config.Root{
Workspace: config.Workspace{ Workspace: config.Workspace{
FilePath: "/bundle", FilePath: "/bundle",
@ -624,7 +636,8 @@ func TestPipelineFileLibraryWithNotebookSourceError(t *testing.T) {
touchNotebookFile(t, filepath.Join(dir, "my_notebook.py")) touchNotebookFile(t, filepath.Join(dir, "my_notebook.py"))
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: dir, RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{ Config: config.Root{
Workspace: config.Workspace{ Workspace: config.Workspace{
FilePath: "/bundle", FilePath: "/bundle",
@ -659,7 +672,8 @@ func TestTranslatePathJobEnvironments(t *testing.T) {
touchEmptyFile(t, filepath.Join(dir, "env2.py")) touchEmptyFile(t, filepath.Join(dir, "env2.py"))
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: dir, RootPath: dir,
BundleRoot: vfs.MustNew(dir),
Config: config.Root{ Config: config.Root{
Resources: config.Resources{ Resources: config.Resources{
Jobs: map[string]*resources.Job{ Jobs: map[string]*resources.Job{

View File

@ -8,7 +8,6 @@ import (
"github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle"
"github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/fileset" "github.com/databricks/cli/libs/fileset"
"github.com/databricks/cli/libs/vfs"
"golang.org/x/sync/errgroup" "golang.org/x/sync/errgroup"
) )
@ -51,7 +50,7 @@ func checkPatterns(patterns []string, path string, rb bundle.ReadOnlyBundle) (di
index := i index := i
p := pattern p := pattern
errs.Go(func() error { 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 { if err != nil {
return err return err
} }

View File

@ -6,7 +6,6 @@ import (
"github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle"
"github.com/databricks/cli/libs/sync" "github.com/databricks/cli/libs/sync"
"github.com/databricks/cli/libs/vfs"
) )
func GetSync(ctx context.Context, rb bundle.ReadOnlyBundle) (*sync.Sync, error) { func GetSync(ctx context.Context, rb bundle.ReadOnlyBundle) (*sync.Sync, error) {
@ -29,7 +28,7 @@ func GetSyncOptions(ctx context.Context, rb bundle.ReadOnlyBundle) (*sync.SyncOp
} }
opts := &sync.SyncOptions{ opts := &sync.SyncOptions{
LocalPath: vfs.MustNew(rb.RootPath()), LocalPath: rb.BundleRoot(),
RemotePath: rb.Config().Workspace.FilePath, RemotePath: rb.Config().Workspace.FilePath,
Include: includes, Include: includes,
Exclude: rb.Config().Sync.Exclude, Exclude: rb.Config().Sync.Exclude,

View File

@ -6,7 +6,6 @@ import (
"fmt" "fmt"
"io" "io"
"io/fs" "io/fs"
"os"
"path/filepath" "path/filepath"
"time" "time"
@ -59,8 +58,8 @@ type entry struct {
info fs.FileInfo info fs.FileInfo
} }
func newEntry(path string) *entry { func newEntry(root vfs.Path, path string) *entry {
info, err := os.Stat(path) info, err := root.Stat(path)
if err != nil { if err != nil {
return &entry{path, nil} return &entry{path, nil}
} }
@ -111,11 +110,10 @@ func FromSlice(files []fileset.File) (Filelist, error) {
return f, nil return f, nil
} }
func (f Filelist) ToSlice(basePath string) []fileset.File { func (f Filelist) ToSlice(root vfs.Path) []fileset.File {
var files []fileset.File var files []fileset.File
root := vfs.MustNew(basePath)
for _, file := range f { 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 // Snapshots created with versions <= v0.220.0 use platform-specific
// paths (i.e. with backslashes). Files returned by [libs/fileset] always // paths (i.e. with backslashes). Files returned by [libs/fileset] always

View File

@ -85,7 +85,7 @@ func (s *statePull) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostic
} }
log.Infof(ctx, "Creating new snapshot") 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 { if err != nil {
return diag.FromErr(err) return diag.FromErr(err)
} }

View File

@ -17,6 +17,7 @@ import (
"github.com/databricks/cli/internal/testutil" "github.com/databricks/cli/internal/testutil"
"github.com/databricks/cli/libs/filer" "github.com/databricks/cli/libs/filer"
"github.com/databricks/cli/libs/sync" "github.com/databricks/cli/libs/sync"
"github.com/databricks/cli/libs/vfs"
"github.com/databricks/databricks-sdk-go/service/iam" "github.com/databricks/databricks-sdk-go/service/iam"
"github.com/stretchr/testify/mock" "github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
@ -59,8 +60,10 @@ func testStatePull(t *testing.T, opts statePullOpts) {
return f, nil return f, nil
}} }}
tmpDir := t.TempDir()
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: t.TempDir(), RootPath: tmpDir,
BundleRoot: vfs.MustNew(tmpDir),
Config: config.Root{ Config: config.Root{
Bundle: config.Bundle{ Bundle: config.Bundle{
Target: "default", Target: "default",

View File

@ -32,7 +32,8 @@ func TestFromSlice(t *testing.T) {
func TestToSlice(t *testing.T) { func TestToSlice(t *testing.T) {
tmpDir := t.TempDir() 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, "test1.py")
testutil.Touch(t, tmpDir, "test2.py") testutil.Touch(t, tmpDir, "test2.py")
testutil.Touch(t, tmpDir, "test3.py") testutil.Touch(t, tmpDir, "test3.py")
@ -44,7 +45,7 @@ func TestToSlice(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
require.Len(t, f, 3) require.Len(t, f, 3)
s := f.ToSlice(tmpDir) s := f.ToSlice(root)
require.Len(t, s, 3) require.Len(t, s, 3)
for _, file := range s { for _, file := range s {

View File

@ -9,6 +9,7 @@ import (
"github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/cmd/root" "github.com/databricks/cli/cmd/root"
"github.com/databricks/cli/libs/vfs"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
@ -16,7 +17,8 @@ import (
func TestSyncOptionsFromBundle(t *testing.T) { func TestSyncOptionsFromBundle(t *testing.T) {
tempDir := t.TempDir() tempDir := t.TempDir()
b := &bundle.Bundle{ b := &bundle.Bundle{
RootPath: tempDir, RootPath: tempDir,
BundleRoot: vfs.MustNew(tempDir),
Config: config.Root{ Config: config.Root{
Bundle: config.Bundle{ Bundle: config.Bundle{
Target: "default", Target: "default",