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

Use vfs.Path for bundle file I/O #1489

Closed
wants to merge 13 commits into from
8 changes: 7 additions & 1 deletion bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ type Bundle struct {
// It is set when we instantiate a new bundle instance.
RootPath string

//
BundleRoot vfs.Path
BundleRootRelative string

Config config.Root

// Metadata about the bundle deployment. This is the interface Databricks services
Expand Down Expand Up @@ -69,7 +73,9 @@ 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),
BundleRootRelative: ".",
}
configFile, err := config.FileNames.FindInPath(path)
if err != nil {
Expand Down
5 changes: 5 additions & 0 deletions bundle/bundle_read_only.go
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
48 changes: 48 additions & 0 deletions bundle/config/mutator/configure_wsfs.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package mutator

import (
"context"
"strings"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/env"
"github.com/databricks/cli/libs/filer"
"github.com/databricks/cli/libs/vfs"
)

type configureWsfs struct{}

func ConfigureWsfs() bundle.Mutator {
return &configureWsfs{}
}

func (m *configureWsfs) Name() string {
return "ConfigureWsfs"
}

func (m *configureWsfs) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
root := b.BundleRoot.Native()

// The bundle root must be is located in /Workspace/
if !strings.HasPrefix(root, "/Workspace/") {
return nil
}

// The executable must be running on DBR.
if _, ok := env.Lookup(ctx, "DATABRICKS_RUNTIME_VERSION"); !ok {
return nil
}

// If so, swap out vfs.Path instance of the sync root with one that
// makes all Workspace File System interactions extension aware.
p, err := vfs.NewFilerPath(ctx, root, func(path string) (filer.Filer, error) {
return filer.NewWorkspaceFilesExtensionsClient(b.WorkspaceClient(), path)
})
if err != nil {
return diag.FromErr(err)
}

b.BundleRoot = p
return nil
}
76 changes: 44 additions & 32 deletions bundle/config/mutator/translate_paths.go
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 All @@ -33,9 +32,7 @@ func (err ErrIsNotNotebook) Error() string {
return fmt.Sprintf("file at %s is not a notebook", err.path)
}

type translatePaths struct {
seen map[string]string
}
type translatePaths struct{}

// TranslatePaths converts paths to local notebook files into paths in the workspace file system.
func TranslatePaths() bundle.Mutator {
Expand All @@ -48,6 +45,14 @@ func (m *translatePaths) Name() string {

type rewriteFunc func(literal, localFullPath, localRelPath, remotePath string) (string, error)

// rewriteContext is a context for rewriting paths in a config.
// It is freshly instantiated on every mutator apply call.
type rewriteContext struct {
b *bundle.Bundle

seen map[string]string
}

// rewritePath converts a given relative path from the loaded config to a new path based on the passed rewriting function
//
// It takes these arguments:
Expand All @@ -57,14 +62,13 @@ type rewriteFunc func(literal, localFullPath, localRelPath, remotePath string) (
// This logic is different between regular files or notebooks.
//
// The function returns an error if it is impossible to rewrite the given relative path.
func (m *translatePaths) rewritePath(
func (r *rewriteContext) rewritePath(
dir string,
b *bundle.Bundle,
p *string,
fn rewriteFunc,
) error {
// We assume absolute paths point to a location in the workspace
if path.IsAbs(filepath.ToSlash(*p)) {
if path.IsAbs(*p) {
return nil
}

Expand All @@ -80,36 +84,41 @@ func (m *translatePaths) rewritePath(

// Local path is relative to the directory the resource was defined in.
localPath := filepath.Join(dir, filepath.FromSlash(*p))
if interp, ok := m.seen[localPath]; ok {
if interp, ok := r.seen[localPath]; ok {
*p = interp
return nil
}

// Remote path must be relative to the bundle root.
localRelPath, err := filepath.Rel(b.RootPath, localPath)
// Local path must be contained in the sync root.
// If it isn't, it won't be synchronized into the workspace.
localRelPath, err := filepath.Rel(r.b.RootPath, localPath)
if err != nil {
return err
}
if strings.HasPrefix(localRelPath, "..") {
return fmt.Errorf("path %s is not contained in bundle root path", localPath)
}

// Convert platform-native paths back to slash-separated paths.
localPath = filepath.ToSlash(localPath)
localRelPath = filepath.ToSlash(localRelPath)

// Prefix remote path with its remote root path.
remotePath := path.Join(b.Config.Workspace.FilePath, filepath.ToSlash(localRelPath))
remotePath := path.Join(r.b.Config.Workspace.FilePath, localRelPath)

// Convert local path into workspace path via specified function.
interp, err := fn(*p, localPath, localRelPath, filepath.ToSlash(remotePath))
interp, err := fn(*p, localPath, localRelPath, remotePath)
if err != nil {
return err
}

*p = interp
m.seen[localPath] = interp
r.seen[localPath] = interp
return nil
}

func translateNotebookPath(literal, localFullPath, localRelPath, remotePath string) (string, error) {
nb, _, err := notebook.Detect(localFullPath)
func (r *rewriteContext) translateNotebookPath(literal, localFullPath, localRelPath, remotePath string) (string, error) {
nb, _, err := notebook.DetectWithFS(r.b.BundleRoot, localRelPath)
if errors.Is(err, fs.ErrNotExist) {
return "", fmt.Errorf("notebook %s not found", literal)
}
Expand All @@ -124,8 +133,8 @@ func translateNotebookPath(literal, localFullPath, localRelPath, remotePath stri
return strings.TrimSuffix(remotePath, filepath.Ext(localFullPath)), nil
}

func translateFilePath(literal, localFullPath, localRelPath, remotePath string) (string, error) {
nb, _, err := notebook.Detect(localFullPath)
func (r *rewriteContext) translateFilePath(literal, localFullPath, localRelPath, remotePath string) (string, error) {
nb, _, err := notebook.DetectWithFS(r.b.BundleRoot, localRelPath)
if errors.Is(err, fs.ErrNotExist) {
return "", fmt.Errorf("file %s not found", literal)
}
Expand All @@ -138,8 +147,8 @@ func translateFilePath(literal, localFullPath, localRelPath, remotePath string)
return remotePath, nil
}

func translateDirectoryPath(literal, localFullPath, localRelPath, remotePath string) (string, error) {
info, err := os.Stat(localFullPath)
func (r *rewriteContext) translateDirectoryPath(literal, localFullPath, localRelPath, remotePath string) (string, error) {
info, err := r.b.BundleRoot.Stat(localRelPath)
if err != nil {
return "", err
}
Expand All @@ -149,20 +158,20 @@ func translateDirectoryPath(literal, localFullPath, localRelPath, remotePath str
return remotePath, nil
}

func translateNoOp(literal, localFullPath, localRelPath, remotePath string) (string, error) {
func (r *rewriteContext) translateNoOp(literal, localFullPath, localRelPath, remotePath string) (string, error) {
return localRelPath, nil
}

func translateNoOpWithPrefix(literal, localFullPath, localRelPath, remotePath string) (string, error) {
func (r *rewriteContext) translateNoOpWithPrefix(literal, localFullPath, localRelPath, remotePath string) (string, error) {
if !strings.HasPrefix(localRelPath, ".") {
localRelPath = "." + string(filepath.Separator) + localRelPath
}
return localRelPath, nil
}

func (m *translatePaths) rewriteValue(b *bundle.Bundle, p dyn.Path, v dyn.Value, fn rewriteFunc, dir string) (dyn.Value, error) {
func (r *rewriteContext) rewriteValue(p dyn.Path, v dyn.Value, fn rewriteFunc, dir string) (dyn.Value, error) {
out := v.MustString()
err := m.rewritePath(dir, b, &out, fn)
err := r.rewritePath(dir, &out, fn)
if err != nil {
if target := (&ErrIsNotebook{}); errors.As(err, target) {
return dyn.InvalidValue, fmt.Errorf(`expected a file for "%s" but got a notebook: %w`, p, target)
Expand All @@ -176,15 +185,15 @@ func (m *translatePaths) rewriteValue(b *bundle.Bundle, p dyn.Path, v dyn.Value,
return dyn.NewValue(out, v.Location()), nil
}

func (m *translatePaths) rewriteRelativeTo(b *bundle.Bundle, p dyn.Path, v dyn.Value, fn rewriteFunc, dir, fallback string) (dyn.Value, error) {
nv, err := m.rewriteValue(b, p, v, fn, dir)
func (r *rewriteContext) rewriteRelativeTo(p dyn.Path, v dyn.Value, fn rewriteFunc, dir, fallback string) (dyn.Value, error) {
nv, err := r.rewriteValue(p, v, fn, dir)
if err == nil {
return nv, nil
}

// If we failed to rewrite the path, try to rewrite it relative to the fallback directory.
if fallback != "" {
nv, nerr := m.rewriteValue(b, p, v, fn, fallback)
nv, nerr := r.rewriteValue(p, v, fn, fallback)
if nerr == nil {
// TODO: Emit a warning that this path should be rewritten.
return nv, nil
Expand All @@ -195,16 +204,19 @@ func (m *translatePaths) rewriteRelativeTo(b *bundle.Bundle, p dyn.Path, v dyn.V
}

func (m *translatePaths) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics {
m.seen = make(map[string]string)
r := &rewriteContext{
b: b,
seen: make(map[string]string),
}

err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) {
var err error
for _, fn := range []func(*bundle.Bundle, dyn.Value) (dyn.Value, error){
m.applyJobTranslations,
m.applyPipelineTranslations,
m.applyArtifactTranslations,
for _, fn := range []func(dyn.Value) (dyn.Value, error){
r.applyJobTranslations,
r.applyPipelineTranslations,
r.applyArtifactTranslations,
} {
v, err = fn(b, v)
v, err = fn(v)
if err != nil {
return dyn.InvalidValue, err
}
Expand Down
7 changes: 3 additions & 4 deletions bundle/config/mutator/translate_paths_artifacts.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@ package mutator
import (
"fmt"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/libs/dyn"
)

func (m *translatePaths) applyArtifactTranslations(b *bundle.Bundle, v dyn.Value) (dyn.Value, error) {
func (r *rewriteContext) applyArtifactTranslations(v dyn.Value) (dyn.Value, error) {
var err error

// Base pattern to match all artifacts.
Expand All @@ -22,7 +21,7 @@ func (m *translatePaths) applyArtifactTranslations(b *bundle.Bundle, v dyn.Value
}{
{
base.Append(dyn.Key("path")),
translateNoOp,
r.translateNoOp,
},
} {
v, err = dyn.MapByPattern(v, t.pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) {
Expand All @@ -32,7 +31,7 @@ func (m *translatePaths) applyArtifactTranslations(b *bundle.Bundle, v dyn.Value
return dyn.InvalidValue, fmt.Errorf("unable to determine directory for artifact %s: %w", key, err)
}

return m.rewriteRelativeTo(b, p, v, t.fn, dir, "")
return r.rewriteRelativeTo(p, v, t.fn, dir, "")
})
if err != nil {
return dyn.InvalidValue, err
Expand Down
Loading
Loading