Skip to content

fix: allow file:/// paths to reference assets in container images #3884

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.

Sign up for GitHub

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

Closed
sshpuntoff wants to merge 1 commit into from
Closed

fix: allow file:/// paths to reference assets in container images #3884

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions NEXT_CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@

### Bundles
* Add validation that served_models and served_entities are not used at the same time. Add client side translation logic. ([#3880](https://github.com/databricks/cli/pull/3880))
* Fix handling of `file://` URIs in job libraries: `file://relative/path` uploads local files, while `file:///absolute/path` references runtime container assets. ([#3884](https://github.com/databricks/cli/pull/3884))

### API Changes
9 changes: 7 additions & 2 deletions bundle/libraries/local_path.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,14 @@ import (
// - s3:/mybucket/myfile.txt
// - /Users/jane@doe.com/myfile.txt
func IsLocalPath(p string) bool {
// If the path has the explicit file scheme, it's a local path.
// If the path has the explicit file scheme, check if it's an absolute path.
// file:///absolute/path is a runtime container path (remote).
// file://relative/path is a local path to be uploaded.
if strings.HasPrefix(p, "file://") {
return true
// Extract the path after file://
pathAfterScheme := strings.TrimPrefix(p, "file://")
// If it's an absolute path (starts with /), it's a runtime path, not local
return !path.IsAbs(pathAfterScheme)
}

// If the path has another scheme, it's a remote path.
Expand Down
19 changes: 14 additions & 5 deletions bundle/libraries/local_path_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,31 @@ import (
)

func TestIsLocalPath(t *testing.T) {
// Relative paths, paths with the file scheme, and Windows paths.
// Relative paths and Windows paths.
assert.True(t, IsLocalPath("some/local/path"))
assert.True(t, IsLocalPath("./some/local/path"))
assert.True(t, IsLocalPath("file://path/to/package"))
assert.True(t, IsLocalPath("C:\\path\\to\\package"))
assert.True(t, IsLocalPath("myfile.txt"))
assert.True(t, IsLocalPath("./myfile.txt"))
assert.True(t, IsLocalPath("../myfile.txt"))
assert.True(t, IsLocalPath("file:///foo/bar/myfile.txt"))

// Absolute paths.
// file:// with relative paths (local files to upload).
assert.True(t, IsLocalPath("file://path/to/package"))
assert.True(t, IsLocalPath("file://foo/bar/myfile.txt"))
assert.True(t, IsLocalPath("file://./relative.jar"))
assert.True(t, IsLocalPath("file://../lib/package.whl"))

// Absolute paths without scheme (remote).
assert.False(t, IsLocalPath("/some/full/path"))
assert.False(t, IsLocalPath("/Workspace/path/to/package"))
assert.False(t, IsLocalPath("/Users/path/to/package"))

// Paths with schemes.
// file:/// with absolute paths (runtime container paths - remote).
assert.False(t, IsLocalPath("file:///foo/bar/myfile.txt"))
assert.False(t, IsLocalPath("file:///opt/spark/jars/driver.jar"))
assert.False(t, IsLocalPath("file:///"))
assert.False(t, IsLocalPath("file:///absolute/path"))
// Paths with other schemes (remote).
assert.False(t, IsLocalPath("dbfs://path/to/package"))
assert.False(t, IsLocalPath("dbfs:/path/to/package"))
assert.False(t, IsLocalPath("s3://path/to/package"))
Expand Down