From 6a2c4119ab58cd8dcda4f42a0534e6147c6781cc Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Fri, 2 Jun 2023 00:14:11 +0200 Subject: [PATCH 1/8] Add check for path is a directory in filer.ReadDir --- libs/filer/dbfs_client.go | 42 +++++++--------------------- libs/filer/filer.go | 18 ++++++++++-- libs/filer/workspace_files_client.go | 29 ++++--------------- 3 files changed, 31 insertions(+), 58 deletions(-) diff --git a/libs/filer/dbfs_client.go b/libs/filer/dbfs_client.go index c86a80b1e1..0a9d5b086a 100644 --- a/libs/filer/dbfs_client.go +++ b/libs/filer/dbfs_client.go @@ -22,7 +22,11 @@ type dbfsDirEntry struct { } func (entry dbfsDirEntry) Type() fs.FileMode { - return entry.Mode() + typ := fs.ModePerm + if entry.fi.IsDir { + typ |= fs.ModeDir + } + return typ } func (entry dbfsDirEntry) Info() (fs.FileInfo, error) { @@ -43,11 +47,7 @@ func (info dbfsFileInfo) Size() int64 { } func (info dbfsFileInfo) Mode() fs.FileMode { - mode := fs.ModePerm - if info.fi.IsDir { - mode |= fs.ModeDir - } - return mode + return fs.ModePerm } func (info dbfsFileInfo) ModTime() time.Time { @@ -222,6 +222,10 @@ func (w *DbfsClient) ReadDir(ctx context.Context, name string) ([]fs.DirEntry, e return nil, err } + if len(res.Files) == 1 && !res.Files[0].IsDir && res.Files[0].Path == absPath { + return nil, NotADirectory{absPath} + } + info := make([]fs.DirEntry, len(res.Files)) for i, v := range res.Files { info[i] = dbfsDirEntry{dbfsFileInfo: dbfsFileInfo{fi: v}} @@ -240,29 +244,3 @@ func (w *DbfsClient) Mkdir(ctx context.Context, name string) error { return w.workspaceClient.Dbfs.MkdirsByPath(ctx, dirPath) } - -func (w *DbfsClient) Stat(ctx context.Context, name string) (fs.FileInfo, error) { - absPath, err := w.root.Join(name) - if err != nil { - return nil, err - } - - info, err := w.workspaceClient.Dbfs.GetStatusByPath(ctx, absPath) - if err != nil { - var aerr *apierr.APIError - if !errors.As(err, &aerr) { - return nil, err - } - - // 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, err - } - - return dbfsFileInfo{*info}, nil -} diff --git a/libs/filer/filer.go b/libs/filer/filer.go index ff01ea7981..9d50539951 100644 --- a/libs/filer/filer.go +++ b/libs/filer/filer.go @@ -2,6 +2,7 @@ package filer import ( "context" + "errors" "fmt" "io" "io/fs" @@ -50,6 +51,20 @@ func (err NoSuchDirectoryError) Is(other error) bool { return other == fs.ErrNotExist } +var ErrNotADirectory = errors.New("not a directory") + +type NotADirectory struct { + path string +} + +func (err NotADirectory) Error() string { + return fmt.Sprintf("%s is not a directory", err.path) +} + +func (err NotADirectory) Is(other error) bool { + return other == ErrNotADirectory +} + // Filer is used to access files in a workspace. // It has implementations for accessing files in WSFS and in DBFS. type Filer interface { @@ -68,7 +83,4 @@ type Filer interface { // Creates directory at `path`, creating any intermediate directories as required. Mkdir(ctx context.Context, path string) error - - // Stat returns information about the file at `path`. - Stat(ctx context.Context, name string) (fs.FileInfo, error) } diff --git a/libs/filer/workspace_files_client.go b/libs/filer/workspace_files_client.go index 967f9a1de5..594e1dbc80 100644 --- a/libs/filer/workspace_files_client.go +++ b/libs/filer/workspace_files_client.go @@ -222,6 +222,12 @@ func (w *WorkspaceFilesClient) ReadDir(ctx context.Context, name string) ([]fs.D objects, err := w.workspaceClient.Workspace.ListAll(ctx, workspace.ListWorkspaceRequest{ Path: absPath, }) + + // TODO: add integration test for this + if len(objects) == 1 && objects[0].Path == absPath { + return nil, NotADirectory{absPath} + } + if err != nil { // If we got an API error we deal with it below. var aerr *apierr.APIError @@ -256,26 +262,3 @@ func (w *WorkspaceFilesClient) Mkdir(ctx context.Context, name string) error { Path: dirPath, }) } - -func (w *WorkspaceFilesClient) Stat(ctx context.Context, name string) (fs.FileInfo, error) { - absPath, err := w.root.Join(name) - if err != nil { - return nil, err - } - - info, err := w.workspaceClient.Workspace.GetStatusByPath(ctx, absPath) - if err != nil { - // If we got an API error we deal with it below. - var aerr *apierr.APIError - if !errors.As(err, &aerr) { - return nil, err - } - - // This API returns a 404 if the specified path does not exist. - if aerr.StatusCode == http.StatusNotFound { - return nil, FileDoesNotExistError{absPath} - } - } - - return wsfsFileInfo{*info}, nil -} From 610bb3ccb9d0343351f61d37ca4347d1c8982f9b Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Fri, 2 Jun 2023 00:15:57 +0200 Subject: [PATCH 2/8] - --- internal/filer_test.go | 53 +++--------------------------------------- 1 file changed, 3 insertions(+), 50 deletions(-) diff --git a/internal/filer_test.go b/internal/filer_test.go index 0dda1d1bfe..223f5d847b 100644 --- a/internal/filer_test.go +++ b/internal/filer_test.go @@ -15,7 +15,6 @@ import ( "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/apierr" "github.com/databricks/databricks-sdk-go/service/files" - "github.com/databricks/databricks-sdk-go/service/workspace" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -68,32 +67,11 @@ func runFilerReadWriteTest(t *testing.T, ctx context.Context, f filer.Filer) { assert.NoError(t, err) filerTest{t, f}.assertContents(ctx, "/foo/bar", `hello universe`) - // Stat on a directory should succeed. - // Note: size and modification time behave differently between WSFS and DBFS. - info, err := f.Stat(ctx, "/foo") - require.NoError(t, err) - assert.Equal(t, "foo", info.Name()) - assert.True(t, info.Mode().IsDir()) - assert.Equal(t, true, info.IsDir()) - - // Stat on a file should succeed. - // Note: size and modification time behave differently between WSFS and DBFS. - info, err = f.Stat(ctx, "/foo/bar") - require.NoError(t, err) - assert.Equal(t, "bar", info.Name()) - assert.True(t, info.Mode().IsRegular()) - assert.Equal(t, false, info.IsDir()) - // Delete should fail if the file doesn't exist. err = f.Delete(ctx, "/doesnt_exist") assert.True(t, errors.As(err, &filer.FileDoesNotExistError{})) assert.True(t, errors.Is(err, fs.ErrNotExist)) - // Stat should fail if the file doesn't exist. - _, err = f.Stat(ctx, "/doesnt_exist") - assert.True(t, errors.As(err, &filer.FileDoesNotExistError{})) - assert.True(t, errors.Is(err, fs.ErrNotExist)) - // Delete should succeed for file that does exist. err = f.Delete(ctx, "/foo/bar") assert.NoError(t, err) @@ -159,35 +137,10 @@ func runFilerReadDirTest(t *testing.T, ctx context.Context, f filer.Filer) { assert.Len(t, entries, 1) assert.Equal(t, "c", entries[0].Name()) assert.True(t, entries[0].IsDir()) -} - -func temporaryWorkspaceDir(t *testing.T, w *databricks.WorkspaceClient) string { - ctx := context.Background() - me, err := w.CurrentUser.Me(ctx) - require.NoError(t, err) - - path := fmt.Sprintf("/Users/%s/%s", me.UserName, RandomName("integration-test-filer-wsfs-")) - // Ensure directory exists, but doesn't exist YET! - // Otherwise we could inadvertently remove a directory that already exists on cleanup. - t.Logf("mkdir %s", path) - err = w.Workspace.MkdirsByPath(ctx, path) - require.NoError(t, err) - - // Remove test directory on test completion. - t.Cleanup(func() { - t.Logf("rm -rf %s", path) - err := w.Workspace.Delete(ctx, workspace.Delete{ - Path: path, - Recursive: true, - }) - if err == nil || apierr.IsMissing(err) { - return - } - t.Logf("unable to remove temporary workspace directory %s: %#v", path, err) - }) - - return path + // TODO: split into a separate PR + _, err = f.ReadDir(ctx, "/hello.txt") + assert.ErrorIs(t, err, filer.ErrNotADirectory) } func setupWorkspaceFilesTest(t *testing.T) (context.Context, filer.Filer) { From 772b2eecbd1c97e30de175e1916232841b2eb2e7 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Fri, 2 Jun 2023 00:18:49 +0200 Subject: [PATCH 3/8] - --- internal/filer_test.go | 21 +++++++++++++++ libs/filer/dbfs_client.go | 38 +++++++++++++++++++++++----- libs/filer/filer.go | 3 +++ libs/filer/workspace_files_client.go | 23 +++++++++++++++++ 4 files changed, 79 insertions(+), 6 deletions(-) diff --git a/internal/filer_test.go b/internal/filer_test.go index 223f5d847b..02a8333985 100644 --- a/internal/filer_test.go +++ b/internal/filer_test.go @@ -67,11 +67,32 @@ func runFilerReadWriteTest(t *testing.T, ctx context.Context, f filer.Filer) { assert.NoError(t, err) filerTest{t, f}.assertContents(ctx, "/foo/bar", `hello universe`) + // Stat on a directory should succeed. + // Note: size and modification time behave differently between WSFS and DBFS. + info, err := f.Stat(ctx, "/foo") + require.NoError(t, err) + assert.Equal(t, "foo", info.Name()) + assert.True(t, info.Mode().IsDir()) + assert.Equal(t, true, info.IsDir()) + + // Stat on a file should succeed. + // Note: size and modification time behave differently between WSFS and DBFS. + info, err = f.Stat(ctx, "/foo/bar") + require.NoError(t, err) + assert.Equal(t, "bar", info.Name()) + assert.True(t, info.Mode().IsRegular()) + assert.Equal(t, false, info.IsDir()) + // Delete should fail if the file doesn't exist. err = f.Delete(ctx, "/doesnt_exist") assert.True(t, errors.As(err, &filer.FileDoesNotExistError{})) assert.True(t, errors.Is(err, fs.ErrNotExist)) + // Stat should fail if the file doesn't exist. + _, err = f.Stat(ctx, "/doesnt_exist") + assert.True(t, errors.As(err, &filer.FileDoesNotExistError{})) + assert.True(t, errors.Is(err, fs.ErrNotExist)) + // Delete should succeed for file that does exist. err = f.Delete(ctx, "/foo/bar") assert.NoError(t, err) diff --git a/libs/filer/dbfs_client.go b/libs/filer/dbfs_client.go index 0a9d5b086a..8229e97b11 100644 --- a/libs/filer/dbfs_client.go +++ b/libs/filer/dbfs_client.go @@ -22,11 +22,7 @@ type dbfsDirEntry struct { } func (entry dbfsDirEntry) Type() fs.FileMode { - typ := fs.ModePerm - if entry.fi.IsDir { - typ |= fs.ModeDir - } - return typ + return entry.Mode() } func (entry dbfsDirEntry) Info() (fs.FileInfo, error) { @@ -47,7 +43,11 @@ func (info dbfsFileInfo) Size() int64 { } func (info dbfsFileInfo) Mode() fs.FileMode { - return fs.ModePerm + mode := fs.ModePerm + if info.fi.IsDir { + mode |= fs.ModeDir + } + return mode } func (info dbfsFileInfo) ModTime() time.Time { @@ -244,3 +244,29 @@ func (w *DbfsClient) Mkdir(ctx context.Context, name string) error { return w.workspaceClient.Dbfs.MkdirsByPath(ctx, dirPath) } + +func (w *DbfsClient) Stat(ctx context.Context, name string) (fs.FileInfo, error) { + absPath, err := w.root.Join(name) + if err != nil { + return nil, err + } + + info, err := w.workspaceClient.Dbfs.GetStatusByPath(ctx, absPath) + if err != nil { + var aerr *apierr.APIError + if !errors.As(err, &aerr) { + return nil, err + } + + // 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, err + } + + return dbfsFileInfo{*info}, nil +} diff --git a/libs/filer/filer.go b/libs/filer/filer.go index 9d50539951..1525aba3a0 100644 --- a/libs/filer/filer.go +++ b/libs/filer/filer.go @@ -83,4 +83,7 @@ type Filer interface { // Creates directory at `path`, creating any intermediate directories as required. Mkdir(ctx context.Context, path string) error + + // Stat returns information about the file at `path`. + Stat(ctx context.Context, name string) (fs.FileInfo, error) } diff --git a/libs/filer/workspace_files_client.go b/libs/filer/workspace_files_client.go index 594e1dbc80..7111d2678c 100644 --- a/libs/filer/workspace_files_client.go +++ b/libs/filer/workspace_files_client.go @@ -262,3 +262,26 @@ func (w *WorkspaceFilesClient) Mkdir(ctx context.Context, name string) error { Path: dirPath, }) } + +func (w *WorkspaceFilesClient) Stat(ctx context.Context, name string) (fs.FileInfo, error) { + absPath, err := w.root.Join(name) + if err != nil { + return nil, err + } + + info, err := w.workspaceClient.Workspace.GetStatusByPath(ctx, absPath) + if err != nil { + // If we got an API error we deal with it below. + var aerr *apierr.APIError + if !errors.As(err, &aerr) { + return nil, err + } + + // This API returns a 404 if the specified path does not exist. + if aerr.StatusCode == http.StatusNotFound { + return nil, FileDoesNotExistError{absPath} + } + } + + return wsfsFileInfo{*info}, nil +} From bce7df8f19086c01b15708d93b2e5b7c673c7eb5 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Fri, 2 Jun 2023 00:19:15 +0200 Subject: [PATCH 4/8] - --- internal/helpers.go | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/internal/helpers.go b/internal/helpers.go index b51d005b27..f1901f14e1 100644 --- a/internal/helpers.go +++ b/internal/helpers.go @@ -14,6 +14,9 @@ import ( "github.com/databricks/cli/cmd/root" _ "github.com/databricks/cli/cmd/version" + "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/apierr" + "github.com/databricks/databricks-sdk-go/service/workspace" "github.com/stretchr/testify/require" _ "github.com/databricks/cli/cmd/workspace" @@ -176,3 +179,32 @@ func writeFile(t *testing.T, name string, body string) string { f.Close() return f.Name() } + +func temporaryWorkspaceDir(t *testing.T, w *databricks.WorkspaceClient) string { + ctx := context.Background() + me, err := w.CurrentUser.Me(ctx) + require.NoError(t, err) + + path := fmt.Sprintf("/Users/%s/%s", me.UserName, RandomName("integration-test-wsfs-")) + + // Ensure directory exists, but doesn't exist YET! + // Otherwise we could inadvertently remove a directory that already exists on cleanup. + t.Logf("mkdir %s", path) + err = w.Workspace.MkdirsByPath(ctx, path) + require.NoError(t, err) + + // Remove test directory on test completion. + t.Cleanup(func() { + t.Logf("rm -rf %s", path) + err := w.Workspace.Delete(ctx, workspace.Delete{ + Path: path, + Recursive: true, + }) + if err == nil || apierr.IsMissing(err) { + return + } + t.Logf("unable to remove temporary workspace directory %s: %#v", path, err) + }) + + return path +} From b402500535ce219af67f90bc85b9135cf32b4173 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Fri, 2 Jun 2023 00:20:23 +0200 Subject: [PATCH 5/8] cleanup todos --- internal/filer_test.go | 1 - libs/filer/workspace_files_client.go | 1 - 2 files changed, 2 deletions(-) diff --git a/internal/filer_test.go b/internal/filer_test.go index 02a8333985..1f4e8df968 100644 --- a/internal/filer_test.go +++ b/internal/filer_test.go @@ -159,7 +159,6 @@ func runFilerReadDirTest(t *testing.T, ctx context.Context, f filer.Filer) { assert.Equal(t, "c", entries[0].Name()) assert.True(t, entries[0].IsDir()) - // TODO: split into a separate PR _, err = f.ReadDir(ctx, "/hello.txt") assert.ErrorIs(t, err, filer.ErrNotADirectory) } diff --git a/libs/filer/workspace_files_client.go b/libs/filer/workspace_files_client.go index 7111d2678c..b06a25146b 100644 --- a/libs/filer/workspace_files_client.go +++ b/libs/filer/workspace_files_client.go @@ -223,7 +223,6 @@ func (w *WorkspaceFilesClient) ReadDir(ctx context.Context, name string) ([]fs.D Path: absPath, }) - // TODO: add integration test for this if len(objects) == 1 && objects[0].Path == absPath { return nil, NotADirectory{absPath} } From dd7e8947c058718a2632dea51e7ed51f55557cc7 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Fri, 2 Jun 2023 00:28:32 +0200 Subject: [PATCH 6/8] add test for empty dir case --- internal/filer_test.go | 8 ++++++++ libs/filer/dbfs_client.go | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/internal/filer_test.go b/internal/filer_test.go index 1f4e8df968..69598cb866 100644 --- a/internal/filer_test.go +++ b/internal/filer_test.go @@ -159,8 +159,16 @@ func runFilerReadDirTest(t *testing.T, ctx context.Context, f filer.Filer) { assert.Equal(t, "c", entries[0].Name()) assert.True(t, entries[0].IsDir()) + // Expect an error trying to call ReadDir on a file _, err = f.ReadDir(ctx, "/hello.txt") assert.ErrorIs(t, err, filer.ErrNotADirectory) + + // Expect 0 entries for an empty directory + err = f.Mkdir(ctx, "empty-dir") + require.NoError(t, err) + entries, err = f.ReadDir(ctx, "empty-dir") + assert.NoError(t, err) + assert.Len(t, entries, 0) } func setupWorkspaceFilesTest(t *testing.T) (context.Context, filer.Filer) { diff --git a/libs/filer/dbfs_client.go b/libs/filer/dbfs_client.go index 8229e97b11..67878136b7 100644 --- a/libs/filer/dbfs_client.go +++ b/libs/filer/dbfs_client.go @@ -222,7 +222,7 @@ func (w *DbfsClient) ReadDir(ctx context.Context, name string) ([]fs.DirEntry, e return nil, err } - if len(res.Files) == 1 && !res.Files[0].IsDir && res.Files[0].Path == absPath { + if len(res.Files) == 1 && res.Files[0].Path == absPath { return nil, NotADirectory{absPath} } From 4feabc5376f01f67c0319265deaa393c8c1a3ba6 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Fri, 2 Jun 2023 11:53:56 +0200 Subject: [PATCH 7/8] - --- internal/filer_test.go | 30 ++++++++++++++++++++++++++++++ internal/helpers.go | 32 -------------------------------- 2 files changed, 30 insertions(+), 32 deletions(-) diff --git a/internal/filer_test.go b/internal/filer_test.go index 69598cb866..3f5fd60f3f 100644 --- a/internal/filer_test.go +++ b/internal/filer_test.go @@ -15,6 +15,7 @@ import ( "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/apierr" "github.com/databricks/databricks-sdk-go/service/files" + "github.com/databricks/databricks-sdk-go/service/workspace" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -171,6 +172,35 @@ func runFilerReadDirTest(t *testing.T, ctx context.Context, f filer.Filer) { assert.Len(t, entries, 0) } +func temporaryWorkspaceDir(t *testing.T, w *databricks.WorkspaceClient) string { + ctx := context.Background() + me, err := w.CurrentUser.Me(ctx) + require.NoError(t, err) + + path := fmt.Sprintf("/Users/%s/%s", me.UserName, RandomName("integration-test-wsfs-")) + + // Ensure directory exists, but doesn't exist YET! + // Otherwise we could inadvertently remove a directory that already exists on cleanup. + t.Logf("mkdir %s", path) + err = w.Workspace.MkdirsByPath(ctx, path) + require.NoError(t, err) + + // Remove test directory on test completion. + t.Cleanup(func() { + t.Logf("rm -rf %s", path) + err := w.Workspace.Delete(ctx, workspace.Delete{ + Path: path, + Recursive: true, + }) + if err == nil || apierr.IsMissing(err) { + return + } + t.Logf("unable to remove temporary workspace directory %s: %#v", path, err) + }) + + return path +} + func setupWorkspaceFilesTest(t *testing.T) (context.Context, filer.Filer) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) diff --git a/internal/helpers.go b/internal/helpers.go index f1901f14e1..b51d005b27 100644 --- a/internal/helpers.go +++ b/internal/helpers.go @@ -14,9 +14,6 @@ import ( "github.com/databricks/cli/cmd/root" _ "github.com/databricks/cli/cmd/version" - "github.com/databricks/databricks-sdk-go" - "github.com/databricks/databricks-sdk-go/apierr" - "github.com/databricks/databricks-sdk-go/service/workspace" "github.com/stretchr/testify/require" _ "github.com/databricks/cli/cmd/workspace" @@ -179,32 +176,3 @@ func writeFile(t *testing.T, name string, body string) string { f.Close() return f.Name() } - -func temporaryWorkspaceDir(t *testing.T, w *databricks.WorkspaceClient) string { - ctx := context.Background() - me, err := w.CurrentUser.Me(ctx) - require.NoError(t, err) - - path := fmt.Sprintf("/Users/%s/%s", me.UserName, RandomName("integration-test-wsfs-")) - - // Ensure directory exists, but doesn't exist YET! - // Otherwise we could inadvertently remove a directory that already exists on cleanup. - t.Logf("mkdir %s", path) - err = w.Workspace.MkdirsByPath(ctx, path) - require.NoError(t, err) - - // Remove test directory on test completion. - t.Cleanup(func() { - t.Logf("rm -rf %s", path) - err := w.Workspace.Delete(ctx, workspace.Delete{ - Path: path, - Recursive: true, - }) - if err == nil || apierr.IsMissing(err) { - return - } - t.Logf("unable to remove temporary workspace directory %s: %#v", path, err) - }) - - return path -} From 54534966ac286b89c6c845f8a7f3caf070c9d2a9 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Fri, 2 Jun 2023 12:02:37 +0200 Subject: [PATCH 8/8] address comments --- internal/filer_test.go | 11 ++++++++++- libs/filer/filer.go | 7 ++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/internal/filer_test.go b/internal/filer_test.go index 3f5fd60f3f..5037f78403 100644 --- a/internal/filer_test.go +++ b/internal/filer_test.go @@ -162,7 +162,7 @@ func runFilerReadDirTest(t *testing.T, ctx context.Context, f filer.Filer) { // Expect an error trying to call ReadDir on a file _, err = f.ReadDir(ctx, "/hello.txt") - assert.ErrorIs(t, err, filer.ErrNotADirectory) + assert.ErrorIs(t, err, fs.ErrInvalid) // Expect 0 entries for an empty directory err = f.Mkdir(ctx, "empty-dir") @@ -170,6 +170,15 @@ func runFilerReadDirTest(t *testing.T, ctx context.Context, f filer.Filer) { entries, err = f.ReadDir(ctx, "empty-dir") assert.NoError(t, err) assert.Len(t, entries, 0) + + // Expect one entry for a directory with a file in it + err = f.Write(ctx, "dir-with-one-file/my-file.txt", strings.NewReader("abc"), filer.CreateParentDirectories) + require.NoError(t, err) + entries, err = f.ReadDir(ctx, "dir-with-one-file") + assert.NoError(t, err) + assert.Len(t, entries, 1) + assert.Equal(t, entries[0].Name(), "my-file.txt") + assert.False(t, entries[0].IsDir()) } func temporaryWorkspaceDir(t *testing.T, w *databricks.WorkspaceClient) string { diff --git a/libs/filer/filer.go b/libs/filer/filer.go index 1525aba3a0..58273fff36 100644 --- a/libs/filer/filer.go +++ b/libs/filer/filer.go @@ -2,7 +2,6 @@ package filer import ( "context" - "errors" "fmt" "io" "io/fs" @@ -51,18 +50,16 @@ func (err NoSuchDirectoryError) Is(other error) bool { return other == fs.ErrNotExist } -var ErrNotADirectory = errors.New("not a directory") - type NotADirectory struct { path string } func (err NotADirectory) Error() string { - return fmt.Sprintf("%s is not a directory", err.path) + return fmt.Sprintf("not a directory: %s", err.path) } func (err NotADirectory) Is(other error) bool { - return other == ErrNotADirectory + return other == fs.ErrInvalid } // Filer is used to access files in a workspace.