steve/databricks-cli
steve
/
databricks-cli
mirror of https://github.com/databricks/cli.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Default to forward slash-separated paths for path translation (#2145) 

## Changes

This came up in #2122 where relative library paths showed up with
backslashes on Windows. It's hard to run acceptance tests where paths
may be in either form. This change updates path translation logic to
always use forward slash-separated paths, including for absolute paths.

## Tests

* Unit tests pass.
* Confirmed that code where library paths are used uses the `filepath`
package for path manipulation. The functions in this package always
normalize their inputs to be platform-native paths.
* Confirmed that code that uses absolute paths works with forward
slash-separated paths on Windows.
This commit is contained in:
Pieter Noordhuis 2025-01-17 10:38:01 +01:00 committed by GitHub
parent 2cd0d88bdd
commit 9061635789
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 41 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
bundle/config/mutator/translate_paths.go
View File

@ -136,7 +136,7 @@ func (t *translateContext) rewritePath(
} }
// Local path is relative to the directory the resource was defined in. // Local path is relative to the directory the resource was defined in.
localPath := filepath.Join(dir, filepath.FromSlash(input)) localPath := filepath.Join(dir, input)
if interp, ok := t.seen[localPath]; ok { if interp, ok := t.seen[localPath]; ok {
return interp, nil return interp, nil
} }
@ -151,6 +151,10 @@ func (t *translateContext) rewritePath(
return "", fmt.Errorf("path %s is not contained in sync root path", localPath) return "", fmt.Errorf("path %s is not contained in sync root path", localPath)
} }
// Normalize paths to separated by forward slashes.
localPath = filepath.ToSlash(localPath)
localRelPath = filepath.ToSlash(localRelPath)
// Convert local path into workspace path via specified function. // Convert local path into workspace path via specified function.
var interp string var interp string
switch opts.Mode { switch opts.Mode {
@ -180,9 +184,9 @@ func (t *translateContext) rewritePath(
} }
func (t *translateContext) translateNotebookPath(ctx context.Context, literal, localFullPath, localRelPath string) (string, error) { 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 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) return "", fmt.Errorf("notebook %s not found", literal)
} }
@ -198,7 +202,7 @@ func (t *translateContext) translateNotebookPath(ctx context.Context, literal, l
// way we can provide a more targeted error message. // way we can provide a more targeted error message.
for _, ext := range extensions { for _, ext := range extensions {
literalWithExt := literal + ext literalWithExt := literal + ext
localRelPathWithExt := filepath.ToSlash(localRelPath + ext) localRelPathWithExt := localRelPath + ext
if _, err := fs.Stat(t.b.SyncRoot, localRelPathWithExt); err == nil { if _, err := fs.Stat(t.b.SyncRoot, localRelPathWithExt); err == nil {
return "", fmt.Errorf(`notebook %s not found. Did you mean %s? return "", fmt.Errorf(`notebook %s not found. Did you mean %s?
Local notebook references are expected to contain one of the following Local notebook references are expected to contain one of the following
@ -218,42 +222,42 @@ to contain one of the following file extensions: [%s]`, literal, strings.Join(ex
} }
// Upon import, notebooks are stripped of their extension. // Upon import, notebooks are stripped of their extension.
localRelPathNoExt := strings.TrimSuffix(localRelPath, filepath.Ext(localRelPath)) localRelPathNoExt := strings.TrimSuffix(localRelPath, path.Ext(localRelPath))
return path.Join(t.remoteRoot, filepath.ToSlash(localRelPathNoExt)), nil return path.Join(t.remoteRoot, localRelPathNoExt), nil
} }
func (t *translateContext) translateFilePath(ctx context.Context, literal, localFullPath, localRelPath string) (string, error) { 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) { if errors.Is(err, fs.ErrNotExist) {
return "", fmt.Errorf("file %s not found", literal) return "", fmt.Errorf("file %s not found", literal)
} }
if err != nil { 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 { if nb {
return "", ErrIsNotebook{localFullPath} 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) { 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 { if err != nil {
return "", err return "", err
} }
if !info.IsDir() { 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) { 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) { if errors.Is(err, fs.ErrNotExist) {
return "", fmt.Errorf("file %s not found", literal) return "", fmt.Errorf("file %s not found", literal)
} }
if err != nil { 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() { if info.IsDir() {
return "", fmt.Errorf("expected %s to be a file but found a directory", literal) return "", fmt.Errorf("expected %s to be a file but found a directory", literal)
@ -262,12 +266,12 @@ func (t *translateContext) translateLocalAbsoluteFilePath(ctx context.Context, l
} }
func (t *translateContext) translateLocalAbsoluteDirectoryPath(ctx context.Context, literal, localFullPath, _ string) (string, error) { 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) { if errors.Is(err, fs.ErrNotExist) {
return "", fmt.Errorf("directory %s not found", literal) return "", fmt.Errorf("directory %s not found", literal)
} }
if err != nil { 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() { if !info.IsDir() {
return "", fmt.Errorf("expected %s to be a directory but found a file", literal) return "", fmt.Errorf("expected %s to be a directory but found a file", literal)
@ -281,7 +285,7 @@ func (t *translateContext) translateLocalRelativePath(ctx context.Context, liter
func (t *translateContext) translateLocalRelativeWithPrefixPath(ctx context.Context, literal, localFullPath, localRelPath string) (string, error) { func (t *translateContext) translateLocalRelativeWithPrefixPath(ctx context.Context, literal, localFullPath, localRelPath string) (string, error) {
if !strings.HasPrefix(localRelPath, ".") { if !strings.HasPrefix(localRelPath, ".") {
localRelPath = "." + string(filepath.Separator) + localRelPath localRelPath = "./" + localRelPath
} }
return localRelPath, nil return localRelPath, nil
} }

4
bundle/config/mutator/translate_paths_artifacts_test.go
View File

@ -46,7 +46,7 @@ func TestTranslatePathsArtifacts_InsideSyncRoot(t *testing.T) {
require.NoError(t, diags.Error()) require.NoError(t, diags.Error())
// Assert that the artifact path has been converted to a local absolute path. // Assert that the artifact path has been converted to a local absolute path.
assert.Equal(t, lib, b.Config.Artifacts["my_artifact"].Path) assert.Equal(t, filepath.ToSlash(lib), b.Config.Artifacts["my_artifact"].Path)
} }
func TestTranslatePathsArtifacts_OutsideSyncRoot(t *testing.T) { func TestTranslatePathsArtifacts_OutsideSyncRoot(t *testing.T) {
@ -79,5 +79,5 @@ func TestTranslatePathsArtifacts_OutsideSyncRoot(t *testing.T) {
require.NoError(t, diags.Error()) require.NoError(t, diags.Error())
// Assert that the artifact path has been converted to a local absolute path. // Assert that the artifact path has been converted to a local absolute path.
assert.Equal(t, lib, b.Config.Artifacts["my_artifact"].Path) assert.Equal(t, filepath.ToSlash(lib), b.Config.Artifacts["my_artifact"].Path)
} }

2
bundle/config/mutator/translate_paths_dashboards_test.go
View File

@ -48,7 +48,7 @@ func TestTranslatePathsDashboards_FilePathRelativeSubDirectory(t *testing.T) {
// Assert that the file path for the dashboard has been converted to its local absolute path. // Assert that the file path for the dashboard has been converted to its local absolute path.
assert.Equal( assert.Equal(
t, t,
filepath.Join(dir, "src", "my_dashboard.lvdash.json"), filepath.ToSlash(filepath.Join(dir, "src", "my_dashboard.lvdash.json")),
b.Config.Resources.Dashboards["dashboard"].FilePath, b.Config.Resources.Dashboards["dashboard"].FilePath,
) )
} }

27
bundle/config/mutator/translate_paths_test.go
View File

@ -6,7 +6,6 @@ import (
"os" "os"
"path/filepath" "path/filepath"
"runtime" "runtime"
"strings"
"testing" "testing"
"github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle"
@ -226,7 +225,7 @@ func TestTranslatePaths(t *testing.T) {
) )
assert.Equal( assert.Equal(
t, t,
filepath.Join("dist", "task.whl"), "dist/task.whl",
b.Config.Resources.Jobs["job"].Tasks[0].Libraries[0].Whl, b.Config.Resources.Jobs["job"].Tasks[0].Libraries[0].Whl,
) )
assert.Equal( assert.Equal(
@ -251,7 +250,7 @@ func TestTranslatePaths(t *testing.T) {
) )
assert.Equal( assert.Equal(
t, t,
filepath.Join("dist", "task.jar"), "dist/task.jar",
b.Config.Resources.Jobs["job"].Tasks[5].Libraries[0].Jar, b.Config.Resources.Jobs["job"].Tasks[5].Libraries[0].Jar,
) )
assert.Equal( assert.Equal(
@ -362,7 +361,7 @@ func TestTranslatePathsInSubdirectories(t *testing.T) {
) )
assert.Equal( assert.Equal(
t, t,
filepath.Join("job", "dist", "task.jar"), "job/dist/task.jar",
b.Config.Resources.Jobs["job"].Tasks[1].Libraries[0].Jar, b.Config.Resources.Jobs["job"].Tasks[1].Libraries[0].Jar,
) )
assert.Equal( assert.Equal(
@ -774,8 +773,8 @@ func TestTranslatePathJobEnvironments(t *testing.T) {
diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths())
require.NoError(t, diags.Error()) 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, "./job/dist/env1.whl", 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, "./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, "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, "/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]) 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])
@ -839,7 +838,7 @@ func TestTranslatePathWithComplexVariables(t *testing.T) {
assert.Equal( assert.Equal(
t, t,
filepath.Join("variables", "local", "whl.whl"), "variables/local/whl.whl",
b.Config.Resources.Jobs["job"].Tasks[0].Libraries[0].Whl, b.Config.Resources.Jobs["job"].Tasks[0].Libraries[0].Whl,
) )
} }
@ -952,34 +951,34 @@ func TestTranslatePathsWithSourceLinkedDeployment(t *testing.T) {
// updated to source path // updated to source path
assert.Equal( assert.Equal(
t, t,
filepath.Join(dir, "my_job_notebook"), dir+"/my_job_notebook",
b.Config.Resources.Jobs["job"].Tasks[0].NotebookTask.NotebookPath, b.Config.Resources.Jobs["job"].Tasks[0].NotebookTask.NotebookPath,
) )
assert.Equal( assert.Equal(
t, t,
filepath.Join(dir, "requirements.txt"), dir+"/requirements.txt",
b.Config.Resources.Jobs["job"].Tasks[2].Libraries[0].Requirements, b.Config.Resources.Jobs["job"].Tasks[2].Libraries[0].Requirements,
) )
assert.Equal( assert.Equal(
t, t,
filepath.Join(dir, "my_python_file.py"), dir+"/my_python_file.py",
b.Config.Resources.Jobs["job"].Tasks[3].SparkPythonTask.PythonFile, b.Config.Resources.Jobs["job"].Tasks[3].SparkPythonTask.PythonFile,
) )
assert.Equal( assert.Equal(
t, t,
filepath.Join(dir, "my_pipeline_notebook"), dir+"/my_pipeline_notebook",
b.Config.Resources.Pipelines["pipeline"].Libraries[0].Notebook.Path, b.Config.Resources.Pipelines["pipeline"].Libraries[0].Notebook.Path,
) )
assert.Equal( assert.Equal(
t, t,
filepath.Join(dir, "my_python_file.py"), dir+"/my_python_file.py",
b.Config.Resources.Pipelines["pipeline"].Libraries[2].File.Path, b.Config.Resources.Pipelines["pipeline"].Libraries[2].File.Path,
) )
// left as is // left as is
assert.Equal( assert.Equal(
t, t,
filepath.Join("dist", "task.whl"), "dist/task.whl",
b.Config.Resources.Jobs["job"].Tasks[0].Libraries[0].Whl, b.Config.Resources.Jobs["job"].Tasks[0].Libraries[0].Whl,
) )
assert.Equal( assert.Equal(
@ -989,7 +988,7 @@ func TestTranslatePathsWithSourceLinkedDeployment(t *testing.T) {
) )
assert.Equal( assert.Equal(
t, t,
filepath.Join("dist", "task.jar"), "dist/task.jar",
b.Config.Resources.Jobs["job"].Tasks[4].Libraries[0].Jar, b.Config.Resources.Jobs["job"].Tasks[4].Libraries[0].Jar,
) )
assert.Equal( assert.Equal(

8
bundle/tests/relative_path_with_includes_test.go
View File

@ -17,8 +17,8 @@ func TestRelativePathsWithIncludes(t *testing.T) {
diags := bundle.Apply(context.Background(), b, m) diags := bundle.Apply(context.Background(), b, m)
assert.NoError(t, diags.Error()) assert.NoError(t, diags.Error())
assert.Equal(t, filepath.Join(b.SyncRootPath, "artifact_a"), b.Config.Artifacts["test_a"].Path) assert.Equal(t, b.SyncRootPath+"/artifact_a", b.Config.Artifacts["test_a"].Path)
assert.Equal(t, filepath.Join(b.SyncRootPath, "subfolder", "artifact_b"), b.Config.Artifacts["test_b"].Path) assert.Equal(t, b.SyncRootPath+"/subfolder/artifact_b", b.Config.Artifacts["test_b"].Path)
assert.ElementsMatch( assert.ElementsMatch(
t, t,
@ -37,6 +37,6 @@ func TestRelativePathsWithIncludes(t *testing.T) {
b.Config.Sync.Exclude, b.Config.Sync.Exclude,
) )
assert.Equal(t, filepath.Join("dist", "job_a.whl"), b.Config.Resources.Jobs["job_a"].Tasks[0].Libraries[0].Whl) assert.Equal(t, "dist/job_a.whl", b.Config.Resources.Jobs["job_a"].Tasks[0].Libraries[0].Whl)
assert.Equal(t, filepath.Join("subfolder", "dist", "job_b.whl"), b.Config.Resources.Jobs["job_b"].Tasks[0].Libraries[0].Whl) assert.Equal(t, "subfolder/dist/job_b.whl", b.Config.Resources.Jobs["job_b"].Tasks[0].Libraries[0].Whl)
} }