Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 1 addition & 3 deletions bundle/deploy/lock/acquire.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/permissions"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/filer"
"github.com/databricks/cli/libs/locker"
"github.com/databricks/cli/libs/log"
)
Expand Down Expand Up @@ -57,8 +56,7 @@ func (m *acquire) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics
return permissions.ReportPossiblePermissionDenied(ctx, b, b.Config.Workspace.StatePath)
}

notExistsError := filer.NoSuchDirectoryError{}
if errors.As(err, &notExistsError) {
if errors.Is(err, fs.ErrNotExist) {
// If we get a "doesn't exist" error from the API this indicates
// we either don't have permissions or the path is invalid.
return permissions.ReportPossiblePermissionDenied(ctx, b, b.Config.Workspace.StatePath)
Expand Down
3 changes: 2 additions & 1 deletion experimental/ssh/internal/client/releases.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"io"
"io/fs"
"net/http"
"os"
"path/filepath"
Expand Down Expand Up @@ -49,7 +50,7 @@ func uploadReleases(ctx context.Context, workspaceFiler filer.Filer, getRelease
if err == nil {
cmdio.LogString(ctx, fmt.Sprintf("File %s already exists in the workspace, skipping upload", remoteBinaryPath))
continue
} else if !errors.As(err, &filer.FileDoesNotExistError{}) {
} else if !errors.Is(err, fs.ErrNotExist) {
return fmt.Errorf("failed to check if file %s exists in workspace: %w", remoteBinaryPath, err)
}

Expand Down
2 changes: 1 addition & 1 deletion integration/cmd/fs/cat_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func TestFsCatOnADir(t *testing.T) {
require.NoError(t, err)

_, _, err = testcli.RequireErrorRun(t, ctx, "fs", "cat", path.Join(tmpDir, "dir1"))
assert.ErrorAs(t, err, &filer.NotAFile{})
assert.ErrorIs(t, err, fs.ErrInvalid)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated to this PR, but is fs.ErrInvalid a correct error to use? The docs say "invalid argument", implies bad arguments passed to the function, not an issue with remote object being a wrong type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. The fs package doesn't have a better alternative. But if we need one, we could add a few constants to libs/filer for more specific checks.

})
}
}
Expand Down
2 changes: 1 addition & 1 deletion integration/cmd/fs/ls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func TestFsLsOnFile(t *testing.T) {

_, _, err := testcli.RequireErrorRun(t, ctx, "fs", "ls", path.Join(tmpDir, "a", "hello.txt"), "--output=json")
assert.Regexp(t, "not a directory: .*/a/hello.txt", err.Error())
assert.ErrorAs(t, err, &filer.NotADirectory{})
assert.ErrorIs(t, err, fs.ErrInvalid)
})
}
}
Expand Down
4 changes: 2 additions & 2 deletions integration/cmd/fs/mkdir_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ package fs_test

import (
"context"
"io/fs"
"path"
"regexp"
"strings"
"testing"

"github.com/databricks/cli/internal/testcli"
"github.com/databricks/cli/libs/filer"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -129,6 +129,6 @@ func TestFsMkdirWhenFileExistsAtPath(t *testing.T) {
// assert mkdir fails
_, _, err = testcli.RequireErrorRun(t, ctx, "fs", "mkdir", path.Join(tmpDir, "hello"))

assert.ErrorAs(t, err, &filer.FileAlreadyExistsError{})
assert.ErrorIs(t, err, fs.ErrExist)
})
}
1 change: 0 additions & 1 deletion integration/cmd/fs/rm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ func TestFsRmNonEmptyDirectory(t *testing.T) {
// Run rm command
_, _, err = testcli.RequireErrorRun(t, ctx, "fs", "rm", path.Join(tmpDir, "a"))
assert.ErrorIs(t, err, fs.ErrInvalid)
assert.ErrorAs(t, err, &filer.DirectoryNotEmptyError{})
})
}
}
Expand Down
12 changes: 2 additions & 10 deletions integration/libs/filer/filer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,12 @@ func commonFilerRecursiveDeleteTest(t *testing.T, ctx context.Context, f filer.F
assert.Equal(t, []string{"file1", "file2", "subdir1", "subdir2"}, names)

err = f.Delete(ctx, "dir")
assert.ErrorAs(t, err, &filer.DirectoryNotEmptyError{})
assert.ErrorIs(t, err, fs.ErrInvalid)

err = f.Delete(ctx, "dir", filer.DeleteRecursively)
assert.NoError(t, err)
_, err = f.ReadDir(ctx, "dir")
assert.ErrorAs(t, err, &filer.NoSuchDirectoryError{})
assert.ErrorIs(t, err, fs.ErrNotExist)
}

func TestFilerRecursiveDelete(t *testing.T) {
Expand Down Expand Up @@ -145,12 +145,10 @@ func commonFilerReadWriteTests(t *testing.T, ctx context.Context, f filer.Filer)

// Write should fail because the intermediate directory doesn't exist.
err = f.Write(ctx, "/foo/bar", strings.NewReader(`hello world`))
assert.ErrorAs(t, err, &filer.NoSuchDirectoryError{})
assert.ErrorIs(t, err, fs.ErrNotExist)

// Read should fail because the intermediate directory doesn't yet exist.
_, err = f.Read(ctx, "/foo/bar")
assert.ErrorAs(t, err, &filer.FileDoesNotExistError{})
assert.ErrorIs(t, err, fs.ErrNotExist)

// Read should fail because the path points to a directory
Expand All @@ -166,7 +164,6 @@ func commonFilerReadWriteTests(t *testing.T, ctx context.Context, f filer.Filer)

// Write should fail because there is an existing file at the specified path.
err = f.Write(ctx, "/foo/bar", strings.NewReader(`hello universe`))
assert.ErrorAs(t, err, &filer.FileAlreadyExistsError{})
assert.ErrorIs(t, err, fs.ErrExist)

// Write with OverwriteIfExists should succeed.
Expand Down Expand Up @@ -196,12 +193,10 @@ func commonFilerReadWriteTests(t *testing.T, ctx context.Context, f filer.Filer)

// Delete should fail if the file doesn't exist.
err = f.Delete(ctx, "/doesnt_exist")
assert.ErrorAs(t, err, &filer.FileDoesNotExistError{})
assert.ErrorIs(t, err, fs.ErrNotExist)

// Stat should fail if the file doesn't exist.
_, err = f.Stat(ctx, "/doesnt_exist")
assert.ErrorAs(t, err, &filer.FileDoesNotExistError{})
assert.ErrorIs(t, err, fs.ErrNotExist)

// Delete should succeed for file that does exist.
Expand All @@ -210,7 +205,6 @@ func commonFilerReadWriteTests(t *testing.T, ctx context.Context, f filer.Filer)

// Delete should fail for a non-empty directory.
err = f.Delete(ctx, "/foo")
assert.ErrorAs(t, err, &filer.DirectoryNotEmptyError{})
assert.ErrorIs(t, err, fs.ErrInvalid)

// Delete should succeed for a non-empty directory if the DeleteRecursively flag is set.
Expand All @@ -220,7 +214,6 @@ func commonFilerReadWriteTests(t *testing.T, ctx context.Context, f filer.Filer)
// Delete of the filer root should ALWAYS fail, otherwise subsequent writes would fail.
// It is not in the filer's purview to delete its root directory.
err = f.Delete(ctx, "/")
assert.ErrorAs(t, err, &filer.CannotDeleteRootError{})
assert.ErrorIs(t, err, fs.ErrInvalid)
}

Expand Down Expand Up @@ -276,7 +269,6 @@ func commonFilerReadDirTest(t *testing.T, ctx context.Context, f filer.Filer) {

// Expect an error if the path doesn't exist.
_, err = f.ReadDir(ctx, "/dir/a/b/c/d/e")
assert.ErrorAs(t, err, &filer.NoSuchDirectoryError{}, err)
assert.ErrorIs(t, err, fs.ErrNotExist)

// Expect two entries in the root.
Expand Down
22 changes: 11 additions & 11 deletions libs/filer/dbfs_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func (w *DbfsClient) Write(ctx context.Context, name string, reader io.Reader, m
// This API returns a 404 if the file doesn't exist.
if aerr.StatusCode == http.StatusNotFound {
if aerr.ErrorCode == "RESOURCE_DOES_NOT_EXIST" {
return NoSuchDirectoryError{path.Dir(absPath)}
return noSuchDirectoryError{path.Dir(absPath)}
}
}

Expand All @@ -124,7 +124,7 @@ func (w *DbfsClient) Write(ctx context.Context, name string, reader io.Reader, m
// This API returns a 400 if the file already exists.
if aerr.StatusCode == http.StatusBadRequest {
if aerr.ErrorCode == "RESOURCE_ALREADY_EXISTS" {
return FileAlreadyExistsError{absPath}
return fileAlreadyExistsError{absPath}
}
}

Expand All @@ -150,7 +150,7 @@ func (w *DbfsClient) Read(ctx context.Context, name string) (io.ReadCloser, erro
if err != nil {
// Return error if file is a directory
if strings.Contains(err.Error(), "cannot open directory for reading") {
return nil, NotAFile{absPath}
return nil, notAFile{absPath}
}

var aerr *apierr.APIError
Expand All @@ -161,7 +161,7 @@ func (w *DbfsClient) Read(ctx context.Context, name string) (io.ReadCloser, erro
// This API returns a 404 if the file doesn't exist.
if aerr.StatusCode == http.StatusNotFound {
if aerr.ErrorCode == "RESOURCE_DOES_NOT_EXIST" {
return nil, FileDoesNotExistError{absPath}
return nil, fileDoesNotExistError{absPath}
}
}

Expand All @@ -180,7 +180,7 @@ func (w *DbfsClient) Delete(ctx context.Context, name string, mode ...DeleteMode

// Illegal to delete the root path.
if absPath == w.root.rootPath {
return CannotDeleteRootError{}
return cannotDeleteRootError{}
}

// Issue info call before delete because delete succeeds if the specified path doesn't exist.
Expand All @@ -198,7 +198,7 @@ func (w *DbfsClient) Delete(ctx context.Context, name string, mode ...DeleteMode
// This API returns a 404 if the file doesn't exist.
if aerr.StatusCode == http.StatusNotFound {
if aerr.ErrorCode == "RESOURCE_DOES_NOT_EXIST" {
return FileDoesNotExistError{absPath}
return fileDoesNotExistError{absPath}
}
}

Expand Down Expand Up @@ -227,13 +227,13 @@ func (w *DbfsClient) Delete(ctx context.Context, name string, mode ...DeleteMode
case http.StatusBadRequest:
// Anecdotally, this error is returned when attempting to delete a non-empty directory.
if aerr.ErrorCode == "IO_ERROR" {
return DirectoryNotEmptyError{absPath}
return directoryNotEmptyError{absPath}
}

// Since 17th december we are observing the backend return an error_code of BAD_REQUEST
// instead of IO_ERROR. Doing direct comparison on the error string instead.
if strings.Contains(aerr.Message, "Directory is not empty. Use recursive delete or remove contents first.") {
return DirectoryNotEmptyError{absPath}
return directoryNotEmptyError{absPath}
}
}

Expand All @@ -256,15 +256,15 @@ func (w *DbfsClient) ReadDir(ctx context.Context, name string) ([]fs.DirEntry, e
// This API returns a 404 if the file doesn't exist.
if aerr.StatusCode == http.StatusNotFound {
if aerr.ErrorCode == "RESOURCE_DOES_NOT_EXIST" {
return nil, NoSuchDirectoryError{absPath}
return nil, noSuchDirectoryError{absPath}
}
}

return nil, err
}

if len(res.Files) == 1 && res.Files[0].Path == absPath {
return nil, NotADirectory{absPath}
return nil, notADirectory{absPath}
}

info := make([]fs.DirEntry, len(res.Files))
Expand Down Expand Up @@ -302,7 +302,7 @@ func (w *DbfsClient) Stat(ctx context.Context, name string) (fs.FileInfo, error)
// This API returns a 404 if the file doesn't exist.
if aerr.StatusCode == http.StatusNotFound {
if aerr.ErrorCode == "RESOURCE_DOES_NOT_EXIST" {
return nil, FileDoesNotExistError{absPath}
return nil, fileDoesNotExistError{absPath}
}
}

Expand Down
117 changes: 117 additions & 0 deletions libs/filer/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
package filer

import "io/fs"

// fileAlreadyExistsError is returned when attempting to write a file at a path
// that already exists, without using the OverwriteIfExists WriteMode flag.
type fileAlreadyExistsError struct {
path string
}

func (err fileAlreadyExistsError) Error() string {
return "file already exists: " + err.path
}

func (err fileAlreadyExistsError) Is(other error) bool {
return other == fs.ErrExist
}

// fileDoesNotExistError is returned when attempting to read, delete, or stat
// a file that does not exist. It is also returned by the workspace files
// extensions client when a notebook exists at a path but a file was expected.
type fileDoesNotExistError struct {
path string
}

func (err fileDoesNotExistError) Is(other error) bool {
return other == fs.ErrNotExist
}

func (err fileDoesNotExistError) Error() string {
return "file does not exist: " + err.path
}

// noSuchDirectoryError is returned when attempting to write a file to a path
// whose parent directory does not exist (without CreateParentDirectories mode),
// or when attempting to read a directory that does not exist.
type noSuchDirectoryError struct {
path string
}

func (err noSuchDirectoryError) Error() string {
return "no such directory: " + err.path
}

func (err noSuchDirectoryError) Is(other error) bool {
return other == fs.ErrNotExist
}

// notADirectory is returned when attempting to read a directory (ReadDir)
// but the path points to a file instead of a directory.
type notADirectory struct {
path string
}

func (err notADirectory) Error() string {
return "not a directory: " + err.path
}

func (err notADirectory) Is(other error) bool {
return other == fs.ErrInvalid
}

// notAFile is returned when attempting to read a file but the path points
// to a directory instead of a file.
type notAFile struct {
path string
}

func (err notAFile) Error() string {
return "not a file: " + err.path
}

func (err notAFile) Is(other error) bool {
return other == fs.ErrInvalid
}

// directoryNotEmptyError is returned when attempting to delete a directory
// that contains files or subdirectories, without using the DeleteRecursively
// DeleteMode flag.
type directoryNotEmptyError struct {
path string
}

func (err directoryNotEmptyError) Error() string {
return "directory not empty: " + err.path
}

func (err directoryNotEmptyError) Is(other error) bool {
return other == fs.ErrInvalid
}

// cannotDeleteRootError is returned when attempting to delete the root path
// of the filer. Deleting the root is not allowed as it would break subsequent
// file operations.
type cannotDeleteRootError struct{}

func (err cannotDeleteRootError) Error() string {
return "unable to delete filer root"
}

func (err cannotDeleteRootError) Is(other error) bool {
return other == fs.ErrInvalid
}

// permissionError is returned when access is denied to a path, for example
// when attempting to create a directory but lacking write permissions.
type permissionError struct {
path string
}

func (err permissionError) Error() string {
return "access denied: " + err.path
}

func (err permissionError) Is(other error) bool {
return other == fs.ErrPermission
}
Loading