Skip to content

Commit

Permalink
Use forward-slash separated form for local relative paths
Browse files Browse the repository at this point in the history
  • Loading branch information
@pietern
pietern committed Jan 14, 2025
1 parent e683c1b commit 01b581c
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 33 deletions.
48 changes: 29 additions & 19 deletions bundle/config/mutator/translate_paths.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,15 @@ func (t *translateContext) rewritePath(
}

// Local path is relative to the directory the resource was defined in.
localPath := filepath.Join(dir, filepath.FromSlash(input))
// Note: the initialized variable is a platform-native path.
localPath := filepath.Join(dir, input)
if interp, ok := t.seen[localPath]; ok {
return interp, nil
}

// Local path must be contained in the sync root.
// If it isn't, it won't be synchronized into the workspace.
// Note: the initialized variable is a platform-native path.
localRelPath, err := filepath.Rel(t.b.SyncRootPath, localPath)
if err != nil {
return "", err
Expand All @@ -151,6 +153,10 @@ func (t *translateContext) rewritePath(
return "", fmt.Errorf("path %s is not contained in sync root path", localPath)
}

// Normalize paths we pass to the translation functions to use forward slashes.
localPath = filepath.ToSlash(localPath)
localRelPath = filepath.ToSlash(localRelPath)

// Convert local path into workspace path via specified function.
var interp string
switch opts.Mode {
Expand Down Expand Up @@ -180,9 +186,9 @@ func (t *translateContext) rewritePath(
}

func (t *translateContext) translateNotebookPath(ctx context.Context, literal, localFullPath, localRelPath string) (string, error) {
nb, _, err := notebook.DetectWithFS(t.b.SyncRoot, filepath.ToSlash(localRelPath))
nb, _, err := notebook.DetectWithFS(t.b.SyncRoot, localRelPath)
if errors.Is(err, fs.ErrNotExist) {
if filepath.Ext(localFullPath) != notebook.ExtensionNone {
if path.Ext(localFullPath) != notebook.ExtensionNone {
return "", fmt.Errorf("notebook %s not found", literal)
}

Expand All @@ -198,7 +204,7 @@ func (t *translateContext) translateNotebookPath(ctx context.Context, literal, l
// way we can provide a more targeted error message.
for _, ext := range extensions {
literalWithExt := literal + ext
localRelPathWithExt := filepath.ToSlash(localRelPath + ext)
localRelPathWithExt := localRelPath + ext
if _, err := fs.Stat(t.b.SyncRoot, localRelPathWithExt); err == nil {
return "", fmt.Errorf(`notebook %s not found. Did you mean %s?
Local notebook references are expected to contain one of the following
Expand All @@ -218,61 +224,65 @@ to contain one of the following file extensions: [%s]`, literal, strings.Join(ex
}

// Upon import, notebooks are stripped of their extension.
localRelPathNoExt := strings.TrimSuffix(localRelPath, filepath.Ext(localRelPath))
return path.Join(t.remoteRoot, filepath.ToSlash(localRelPathNoExt)), nil
localRelPathNoExt := strings.TrimSuffix(localRelPath, path.Ext(localRelPath))
return path.Join(t.remoteRoot, localRelPathNoExt), nil
}

func (t *translateContext) translateFilePath(ctx context.Context, literal, localFullPath, localRelPath string) (string, error) {
nb, _, err := notebook.DetectWithFS(t.b.SyncRoot, filepath.ToSlash(localRelPath))
nb, _, err := notebook.DetectWithFS(t.b.SyncRoot, localRelPath)
if errors.Is(err, fs.ErrNotExist) {
return "", fmt.Errorf("file %s not found", literal)
}
if err != nil {
return "", fmt.Errorf("unable to determine if %s is not a notebook: %w", localFullPath, err)
return "", fmt.Errorf("unable to determine if %s is not a notebook: %w", filepath.FromSlash(localFullPath), err)
}
if nb {
return "", ErrIsNotebook{localFullPath}
}
return path.Join(t.remoteRoot, filepath.ToSlash(localRelPath)), nil
return path.Join(t.remoteRoot, localRelPath), nil
}

func (t *translateContext) translateDirectoryPath(ctx context.Context, literal, localFullPath, localRelPath string) (string, error) {
info, err := t.b.SyncRoot.Stat(filepath.ToSlash(localRelPath))
info, err := t.b.SyncRoot.Stat(localRelPath)
if err != nil {
return "", err
}
if !info.IsDir() {
return "", fmt.Errorf("%s is not a directory", localFullPath)
return "", fmt.Errorf("%s is not a directory", filepath.FromSlash(localFullPath))
}
return path.Join(t.remoteRoot, filepath.ToSlash(localRelPath)), nil
return path.Join(t.remoteRoot, localRelPath), nil
}

func (t *translateContext) translateLocalAbsoluteFilePath(ctx context.Context, literal, localFullPath, localRelPath string) (string, error) {
info, err := t.b.SyncRoot.Stat(filepath.ToSlash(localRelPath))
info, err := t.b.SyncRoot.Stat(localRelPath)
if errors.Is(err, fs.ErrNotExist) {
return "", fmt.Errorf("file %s not found", literal)
}
if err != nil {
return "", fmt.Errorf("unable to determine if %s is a file: %w", localFullPath, err)
return "", fmt.Errorf("unable to determine if %s is a file: %w", filepath.FromSlash(localFullPath), err)
}
if info.IsDir() {
return "", fmt.Errorf("expected %s to be a file but found a directory", literal)
}
return localFullPath, nil
// Absolute paths are never externalized and always intended to be used locally.
// Return a platform-native path.
return filepath.FromSlash(localFullPath), nil
}

func (t *translateContext) translateLocalAbsoluteDirectoryPath(ctx context.Context, literal, localFullPath, _ string) (string, error) {
info, err := os.Stat(localFullPath)
info, err := os.Stat(filepath.FromSlash(localFullPath))
if errors.Is(err, fs.ErrNotExist) {
return "", fmt.Errorf("directory %s not found", literal)
}
if err != nil {
return "", fmt.Errorf("unable to determine if %s is a directory: %w", localFullPath, err)
return "", fmt.Errorf("unable to determine if %s is a directory: %w", filepath.FromSlash(localFullPath), err)
}
if !info.IsDir() {
return "", fmt.Errorf("expected %s to be a directory but found a file", literal)
}
return localFullPath, nil
// Absolute paths are never externalized and always intended to be used locally.
// Return a platform-native path.
return filepath.FromSlash(localFullPath), nil
}

func (t *translateContext) translateLocalRelativePath(ctx context.Context, literal, localFullPath, localRelPath string) (string, error) {
Expand All @@ -281,7 +291,7 @@ func (t *translateContext) translateLocalRelativePath(ctx context.Context, liter

func (t *translateContext) translateLocalRelativeWithPrefixPath(ctx context.Context, literal, localFullPath, localRelPath string) (string, error) {
if !strings.HasPrefix(localRelPath, ".") {
localRelPath = "." + string(filepath.Separator) + localRelPath
localRelPath = "./" + localRelPath
}
return localRelPath, nil
}
Expand Down
27 changes: 13 additions & 14 deletions bundle/config/mutator/translate_paths_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"os"
"path/filepath"
"runtime"
"strings"
"testing"

"github.com/databricks/cli/bundle"
Expand Down Expand Up @@ -226,7 +225,7 @@ func TestTranslatePaths(t *testing.T) {
)
assert.Equal(
t,
filepath.Join("dist", "task.whl"),
"dist/task.whl",
b.Config.Resources.Jobs["job"].Tasks[0].Libraries[0].Whl,
)
assert.Equal(
Expand All @@ -251,7 +250,7 @@ func TestTranslatePaths(t *testing.T) {
)
assert.Equal(
t,
filepath.Join("dist", "task.jar"),
"dist/task.jar",
b.Config.Resources.Jobs["job"].Tasks[5].Libraries[0].Jar,
)
assert.Equal(
Expand Down Expand Up @@ -362,7 +361,7 @@ func TestTranslatePathsInSubdirectories(t *testing.T) {
)
assert.Equal(
t,
filepath.Join("job", "dist", "task.jar"),
"job/dist/task.jar",
b.Config.Resources.Jobs["job"].Tasks[1].Libraries[0].Jar,
)
assert.Equal(
Expand Down Expand Up @@ -774,8 +773,8 @@ func TestTranslatePathJobEnvironments(t *testing.T) {
diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths())
require.NoError(t, diags.Error())

assert.Equal(t, strings.Join([]string{".", "job", "dist", "env1.whl"}, string(os.PathSeparator)), b.Config.Resources.Jobs["job"].JobSettings.Environments[0].Spec.Dependencies[0])
assert.Equal(t, strings.Join([]string{".", "dist", "env2.whl"}, string(os.PathSeparator)), b.Config.Resources.Jobs["job"].JobSettings.Environments[0].Spec.Dependencies[1])
assert.Equal(t, "./job/dist/env1.whl", b.Config.Resources.Jobs["job"].JobSettings.Environments[0].Spec.Dependencies[0])
assert.Equal(t, "./dist/env2.whl", b.Config.Resources.Jobs["job"].JobSettings.Environments[0].Spec.Dependencies[1])
assert.Equal(t, "simplejson", b.Config.Resources.Jobs["job"].JobSettings.Environments[0].Spec.Dependencies[2])
assert.Equal(t, "/Workspace/Users/foo@bar.com/test.whl", b.Config.Resources.Jobs["job"].JobSettings.Environments[0].Spec.Dependencies[3])
assert.Equal(t, "--extra-index-url https://name:token@gitlab.com/api/v4/projects/9876/packages/pypi/simple foobar", b.Config.Resources.Jobs["job"].JobSettings.Environments[0].Spec.Dependencies[4])
Expand Down Expand Up @@ -839,7 +838,7 @@ func TestTranslatePathWithComplexVariables(t *testing.T) {

assert.Equal(
t,
filepath.Join("variables", "local", "whl.whl"),
"variables/local/whl.whl",
b.Config.Resources.Jobs["job"].Tasks[0].Libraries[0].Whl,
)
}
Expand Down Expand Up @@ -952,34 +951,34 @@ func TestTranslatePathsWithSourceLinkedDeployment(t *testing.T) {
// updated to source path
assert.Equal(
t,
filepath.Join(dir, "my_job_notebook"),
dir+"/my_job_notebook",
b.Config.Resources.Jobs["job"].Tasks[0].NotebookTask.NotebookPath,
)
assert.Equal(
t,
filepath.Join(dir, "requirements.txt"),
dir+"/requirements.txt",
b.Config.Resources.Jobs["job"].Tasks[2].Libraries[0].Requirements,
)
assert.Equal(
t,
filepath.Join(dir, "my_python_file.py"),
dir+"/my_python_file.py",
b.Config.Resources.Jobs["job"].Tasks[3].SparkPythonTask.PythonFile,
)
assert.Equal(
t,
filepath.Join(dir, "my_pipeline_notebook"),
dir+"/my_pipeline_notebook",
b.Config.Resources.Pipelines["pipeline"].Libraries[0].Notebook.Path,
)
assert.Equal(
t,
filepath.Join(dir, "my_python_file.py"),
dir+"/my_python_file.py",
b.Config.Resources.Pipelines["pipeline"].Libraries[2].File.Path,
)

// left as is
assert.Equal(
t,
filepath.Join("dist", "task.whl"),
"dist/task.whl",
b.Config.Resources.Jobs["job"].Tasks[0].Libraries[0].Whl,
)
assert.Equal(
Expand All @@ -989,7 +988,7 @@ func TestTranslatePathsWithSourceLinkedDeployment(t *testing.T) {
)
assert.Equal(
t,
filepath.Join("dist", "task.jar"),
"dist/task.jar",
b.Config.Resources.Jobs["job"].Tasks[4].Libraries[0].Jar,
)
assert.Equal(
Expand Down

0 comments on commit 01b581c

Please sign in to comment.