From b5f5f1103f7d4dac8af33b09ddb8e318830bc846 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Fri, 2 Jun 2023 17:24:04 +0200 Subject: [PATCH 01/18] Add fs ls command for dbfs --- cmd/fs/fs.go | 1 - cmd/fs/ls.go | 64 ++++++++++++++++++++++++++++++++++++--- cmd/fs/ls_output.go | 27 +++++++++++++++++ cmd/root/io.go | 12 +++++--- internal/ls_test.go | 2 ++ libs/cmdio/io.go | 19 +++++++----- libs/cmdio/render.go | 4 +++ libs/filer/dbfs_client.go | 2 ++ 8 files changed, 113 insertions(+), 18 deletions(-) create mode 100644 cmd/fs/ls_output.go create mode 100644 internal/ls_test.go diff --git a/cmd/fs/fs.go b/cmd/fs/fs.go index 74d725d4e8..8c6d034103 100644 --- a/cmd/fs/fs.go +++ b/cmd/fs/fs.go @@ -10,7 +10,6 @@ var fsCmd = &cobra.Command{ Use: "fs", Short: "Filesystem related commands", Long: `Commands to do DBFS operations.`, - Hidden: true, } func init() { diff --git a/cmd/fs/ls.go b/cmd/fs/ls.go index ac19238573..d8626f2993 100644 --- a/cmd/fs/ls.go +++ b/cmd/fs/ls.go @@ -2,22 +2,76 @@ package fs import ( "fmt" + "net/url" + "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/filer" "github.com/spf13/cobra" ) // lsCmd represents the ls command var lsCmd = &cobra.Command{ - Use: "ls ", - Short: "Lists files", - Long: `Lists files`, - Hidden: true, + Use: "ls ", + Short: "Lists files", + Long: `Lists files`, + Args: cobra.ExactArgs(1), + PreRunE: root.MustWorkspaceClient, + Annotations: map[string]string{ + "template_long": cmdio.Heredoc(` + {{range .}}{{if .IsDir}}DIRECTORY {{else}}FILE {{end}}{{.Size}} {{.ModTime|pretty_date}} {{.Name}} + {{end}} + `), + "template": cmdio.Heredoc(` + {{range .}}{{.Name}} + {{end}} + `), + }, RunE: func(cmd *cobra.Command, args []string) error { - return fmt.Errorf("TODO") + ctx := cmd.Context() + w := root.WorkspaceClient(ctx) + + fileUri, err := url.Parse(args[0]) + if err != nil { + return err + } + + // Only dbfs file scheme is supported + if fileUri.Scheme != filer.DbfsScheme { + return fmt.Errorf("expected dbfs path (with the dbfs:/ prefix): %s", args[0]) + } + + f, err := filer.NewDbfsClient(w, fileUri.Path) + if err != nil { + return err + } + + entries, err := f.ReadDir(ctx, "") + if err != nil { + return err + } + + lsOutputs := make([]lsOutput, 0) + for _, entry := range entries { + parsedEntry, err := toLsOutput(entry) + if err != nil { + return err + } + lsOutputs = append(lsOutputs, *parsedEntry) + } + + // Use template for long mode if the flag is set + if longMode { + return cmdio.RenderWithTemplate(ctx, lsOutputs, "template_long") + } + return cmdio.Render(ctx, lsOutputs) }, } +var longMode bool + func init() { + lsCmd.Flags().BoolVarP(&longMode, "long", "l", false, "Displays full information including size, file type and modification time since Epoch in milliseconds.") fsCmd.AddCommand(lsCmd) } diff --git a/cmd/fs/ls_output.go b/cmd/fs/ls_output.go new file mode 100644 index 0000000000..e8728b0465 --- /dev/null +++ b/cmd/fs/ls_output.go @@ -0,0 +1,27 @@ +package fs + +import ( + "io/fs" + "time" +) + +type lsOutput struct { + Name string `json:"name"` + IsDir bool `json:"is_directory"` + Size int64 `json:"size"` + ModTime time.Time `json:"last_modified"` +} + +func toLsOutput(f fs.DirEntry) (*lsOutput, error) { + info, err := f.Info() + if err != nil { + return nil, err + } + + return &lsOutput{ + Name: f.Name(), + IsDir: f.IsDir(), + Size: info.Size(), + ModTime: info.ModTime(), + }, nil +} diff --git a/cmd/root/io.go b/cmd/root/io.go index 93830c804b..07db9e0a0a 100644 --- a/cmd/root/io.go +++ b/cmd/root/io.go @@ -2,6 +2,7 @@ package root import ( "os" + "strings" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/flags" @@ -27,13 +28,14 @@ func OutputType() flags.Output { } func initializeIO(cmd *cobra.Command) error { - var template string - if cmd.Annotations != nil { - // rely on zeroval being an empty string - template = cmd.Annotations["template"] + templates := make(map[string]string, 0) + for k, v := range cmd.Annotations { + if strings.Contains(k, "template") { + templates[k] = v + } } - cmdIO := cmdio.NewIO(outputType, cmd.InOrStdin(), cmd.OutOrStdout(), cmd.ErrOrStderr(), template) + cmdIO := cmdio.NewIO(outputType, cmd.InOrStdin(), cmd.OutOrStdout(), cmd.ErrOrStderr(), templates) ctx := cmdio.InContext(cmd.Context(), cmdIO) cmd.SetContext(ctx) diff --git a/internal/ls_test.go b/internal/ls_test.go new file mode 100644 index 0000000000..8807a5d622 --- /dev/null +++ b/internal/ls_test.go @@ -0,0 +1,2 @@ +package internal + diff --git a/libs/cmdio/io.go b/libs/cmdio/io.go index beaa85717f..4f134c5100 100644 --- a/libs/cmdio/io.go +++ b/libs/cmdio/io.go @@ -24,17 +24,17 @@ type cmdIO struct { // e.g. if stdout is a terminal interactive bool outputFormat flags.Output - template string + templates map[string]string in io.Reader out io.Writer err io.Writer } -func NewIO(outputFormat flags.Output, in io.Reader, out io.Writer, err io.Writer, template string) *cmdIO { +func NewIO(outputFormat flags.Output, in io.Reader, out io.Writer, err io.Writer, templates map[string]string) *cmdIO { return &cmdIO{ interactive: !color.NoColor, outputFormat: outputFormat, - template: template, + templates: templates, in: in, out: out, err: err, @@ -66,14 +66,14 @@ func (c *cmdIO) IsTTY() bool { return isatty.IsTerminal(fd) || isatty.IsCygwinTerminal(fd) } -func (c *cmdIO) Render(v any) error { +func (c *cmdIO) Render(v any, templateName string) error { // TODO: add terminal width & white/dark theme detection switch c.outputFormat { case flags.OutputJSON: return renderJson(c.out, v) case flags.OutputText: - if c.template != "" { - return renderTemplate(c.out, c.template, v) + if c.templates[templateName] != "" { + return renderTemplate(c.out, c.templates[templateName], v) } return renderJson(c.out, v) default: @@ -83,7 +83,12 @@ func (c *cmdIO) Render(v any) error { func Render(ctx context.Context, v any) error { c := fromContext(ctx) - return c.Render(v) + return c.Render(v, "template") +} + +func RenderWithTemplate(ctx context.Context, v any, templateName string) error { + c := fromContext(ctx) + return c.Render(v, templateName) } type tuple struct{ Name, Id string } diff --git a/libs/cmdio/render.go b/libs/cmdio/render.go index 8aff2b8d2c..063d7cbcb4 100644 --- a/libs/cmdio/render.go +++ b/libs/cmdio/render.go @@ -6,6 +6,7 @@ import ( "strings" "text/tabwriter" "text/template" + "time" "github.com/fatih/color" "github.com/nwidger/jsoncolor" @@ -86,6 +87,9 @@ func renderTemplate(w io.Writer, tmpl string, v any) error { } return string(b), nil }, + "pretty_date": func(t time.Time) string { + return t.UTC().Format("2006-01-02T15:04:05Z") + }, }).Parse(tmpl) if err != nil { return err diff --git a/libs/filer/dbfs_client.go b/libs/filer/dbfs_client.go index 67878136b7..293ea39963 100644 --- a/libs/filer/dbfs_client.go +++ b/libs/filer/dbfs_client.go @@ -16,6 +16,8 @@ import ( "golang.org/x/exp/slices" ) +const DbfsScheme = "dbfs" + // Type that implements fs.DirEntry for DBFS. type dbfsDirEntry struct { dbfsFileInfo From a4879c0d91921554c482a23360aa20c31dafdeac Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 01:06:42 +0200 Subject: [PATCH 02/18] added integration tests --- cmd/fs/ls.go | 4 +++ internal/filer_test.go | 2 +- internal/ls_test.go | 79 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 1 deletion(-) diff --git a/cmd/fs/ls.go b/cmd/fs/ls.go index d8626f2993..828610d991 100644 --- a/cmd/fs/ls.go +++ b/cmd/fs/ls.go @@ -3,6 +3,7 @@ package fs import ( "fmt" "net/url" + "sort" "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/libs/cmdio" @@ -59,6 +60,9 @@ var lsCmd = &cobra.Command{ return err } lsOutputs = append(lsOutputs, *parsedEntry) + sort.Slice(lsOutputs, func(i, j int) bool { + return lsOutputs[i].Name < lsOutputs[j].Name + }) } // Use template for long mode if the flag is set diff --git a/internal/filer_test.go b/internal/filer_test.go index 5037f78403..81c3e4aea8 100644 --- a/internal/filer_test.go +++ b/internal/filer_test.go @@ -241,7 +241,7 @@ func TestAccFilerWorkspaceFilesReadDir(t *testing.T) { func temporaryDbfsDir(t *testing.T, w *databricks.WorkspaceClient) string { ctx := context.Background() - path := fmt.Sprintf("/tmp/%s", RandomName("integration-test-filer-dbfs-")) + path := fmt.Sprintf("/tmp/%s", RandomName("integration-test-dbfs-")) // This call fails if the path already exists. t.Logf("mkdir dbfs:%s", path) diff --git a/internal/ls_test.go b/internal/ls_test.go index 8807a5d622..1c716bf016 100644 --- a/internal/ls_test.go +++ b/internal/ls_test.go @@ -1,2 +1,81 @@ package internal +import ( + "context" + "encoding/json" + "io/fs" + "strings" + "testing" + + _ "github.com/databricks/cli/cmd/fs" + "github.com/databricks/cli/libs/filer" + "github.com/databricks/databricks-sdk-go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFsLsForDbfs(t *testing.T) { + t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) + + ctx := context.Background() + w, err := databricks.NewWorkspaceClient() + require.NoError(t, err) + + tmpDir := temporaryDbfsDir(t, w) + + f, err := filer.NewDbfsClient(w, tmpDir) + require.NoError(t, err) + + err = f.Mkdir(ctx, "a") + require.NoError(t, err) + err = f.Write(ctx, "a/hello.txt", strings.NewReader("abc"), filer.CreateParentDirectories) + require.NoError(t, err) + err = f.Write(ctx, "bye.txt", strings.NewReader("def")) + require.NoError(t, err) + + stdout, stderr := RequireSuccessfulRun(t, "fs", "ls", "dbfs:"+tmpDir, "--output=json") + assert.Equal(t, "", stderr.String()) + var parsedStdout []map[string]any + err = json.Unmarshal(stdout.Bytes(), &parsedStdout) + require.NoError(t, err) + + // assert on ls output + assert.Equal(t, "a", parsedStdout[0]["name"]) + assert.Equal(t, true, parsedStdout[0]["is_directory"]) + assert.Equal(t, float64(0), parsedStdout[0]["size"]) + assert.Equal(t, "bye.txt", parsedStdout[1]["name"]) + assert.Equal(t, false, parsedStdout[1]["is_directory"]) + assert.Equal(t, float64(3), parsedStdout[1]["size"]) +} + +func TestFsLsForDbfsOnEmptyDir(t *testing.T) { + t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) + + w, err := databricks.NewWorkspaceClient() + require.NoError(t, err) + + tmpDir := temporaryDbfsDir(t, w) + + stdout, stderr := RequireSuccessfulRun(t, "fs", "ls", "dbfs:"+tmpDir, "--output=json") + assert.Equal(t, "", stderr.String()) + var parsedStdout []map[string]any + err = json.Unmarshal(stdout.Bytes(), &parsedStdout) + require.NoError(t, err) + + // assert on ls output + assert.Equal(t, 0, len(parsedStdout)) +} + +func TestFsLsForDbfsForNonexistingDir(t *testing.T) { + t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) + + _, _, err := RequireErrorRun(t, "fs", "ls", "dbfs:/john-cena", "--output=json") + assert.ErrorIs(t, err, fs.ErrNotExist) +} + +func TestFsLsWithoutScheme(t *testing.T) { + t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) + + _, _, err := RequireErrorRun(t, "fs", "ls", "/ray-mysterio", "--output=json") + assert.ErrorContains(t, err, "expected dbfs path (with the dbfs:/ prefix): /ray-mysterio") +} From 2706bebb3841f57a284626c5aba8b7568afc22aa Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 01:10:15 +0200 Subject: [PATCH 03/18] lint --- cmd/fs/fs.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/fs/fs.go b/cmd/fs/fs.go index 8c6d034103..a69c4b62da 100644 --- a/cmd/fs/fs.go +++ b/cmd/fs/fs.go @@ -7,9 +7,9 @@ import ( // fsCmd represents the fs command var fsCmd = &cobra.Command{ - Use: "fs", - Short: "Filesystem related commands", - Long: `Commands to do DBFS operations.`, + Use: "fs", + Short: "Filesystem related commands", + Long: `Commands to do DBFS operations.`, } func init() { From 6b667b5e0106da0472129c2bc2b908f6b3cdaeb8 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 01:20:54 +0200 Subject: [PATCH 04/18] added test for ls on flie --- internal/ls_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/internal/ls_test.go b/internal/ls_test.go index 1c716bf016..060c706df6 100644 --- a/internal/ls_test.go +++ b/internal/ls_test.go @@ -4,6 +4,8 @@ import ( "context" "encoding/json" "io/fs" + "path" + "regexp" "strings" "testing" @@ -48,6 +50,27 @@ func TestFsLsForDbfs(t *testing.T) { assert.Equal(t, float64(3), parsedStdout[1]["size"]) } +func TestFsLsForDbfsOnFile(t *testing.T) { + t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) + + ctx := context.Background() + w, err := databricks.NewWorkspaceClient() + require.NoError(t, err) + + tmpDir := temporaryDbfsDir(t, w) + + f, err := filer.NewDbfsClient(w, tmpDir) + require.NoError(t, err) + + err = f.Mkdir(ctx, "a") + require.NoError(t, err) + err = f.Write(ctx, "a/hello.txt", strings.NewReader("abc"), filer.CreateParentDirectories) + require.NoError(t, err) + + _, _, err = RequireErrorRun(t, "fs", "ls", "dbfs:"+path.Join(tmpDir, "a", "hello.txt"), "--output=json") + assert.Regexp(t, regexp.MustCompile("not a directory: .*/a/hello.txt"), err.Error()) +} + func TestFsLsForDbfsOnEmptyDir(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) From 492382caa1eacdd6dcad2c12e09ff1501e46c3a9 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 01:34:28 +0200 Subject: [PATCH 05/18] added resolvedbfs path func --- cmd/fs/ls.go | 11 ++--------- libs/filer/dbfs_client.go | 16 ++++++++++++++++ libs/filer/dbfs_client_test.go | 35 ++++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 9 deletions(-) create mode 100644 libs/filer/dbfs_client_test.go diff --git a/cmd/fs/ls.go b/cmd/fs/ls.go index 828610d991..76b51ce7ea 100644 --- a/cmd/fs/ls.go +++ b/cmd/fs/ls.go @@ -1,8 +1,6 @@ package fs import ( - "fmt" - "net/url" "sort" "github.com/databricks/cli/cmd/root" @@ -33,17 +31,12 @@ var lsCmd = &cobra.Command{ ctx := cmd.Context() w := root.WorkspaceClient(ctx) - fileUri, err := url.Parse(args[0]) + path, err := filer.ResolveDbfsPath(args[0]) if err != nil { return err } - // Only dbfs file scheme is supported - if fileUri.Scheme != filer.DbfsScheme { - return fmt.Errorf("expected dbfs path (with the dbfs:/ prefix): %s", args[0]) - } - - f, err := filer.NewDbfsClient(w, fileUri.Path) + f, err := filer.NewDbfsClient(w, path) if err != nil { return err } diff --git a/libs/filer/dbfs_client.go b/libs/filer/dbfs_client.go index 293ea39963..efdcdf31c0 100644 --- a/libs/filer/dbfs_client.go +++ b/libs/filer/dbfs_client.go @@ -3,9 +3,11 @@ package filer import ( "context" "errors" + "fmt" "io" "io/fs" "net/http" + "net/url" "path" "sort" "time" @@ -272,3 +274,17 @@ func (w *DbfsClient) Stat(ctx context.Context, name string) (fs.FileInfo, error) return dbfsFileInfo{*info}, nil } + +func ResolveDbfsPath(path string) (string, error) { + fileUri, err := url.Parse(path) + if err != nil { + return "", err + } + + // Only dbfs file scheme is supported + if fileUri.Scheme != DbfsScheme { + return "", fmt.Errorf("expected dbfs path (with the dbfs:/ prefix): %s", path) + } + + return fileUri.Path, nil +} diff --git a/libs/filer/dbfs_client_test.go b/libs/filer/dbfs_client_test.go new file mode 100644 index 0000000000..7aca0383df --- /dev/null +++ b/libs/filer/dbfs_client_test.go @@ -0,0 +1,35 @@ +package filer + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestResolveDbfsPath(t *testing.T) { + path, err := ResolveDbfsPath("dbfs:/") + assert.NoError(t, err) + assert.Equal(t, "/", path) + + path, err = ResolveDbfsPath("dbfs:/abc") + assert.NoError(t, err) + assert.Equal(t, "/abc", path) + + path, err = ResolveDbfsPath("dbfs:/a/b/c") + assert.NoError(t, err) + assert.Equal(t, "/a/b/c", path) + + path, err = ResolveDbfsPath("dbfs:/a/b/.") + assert.NoError(t, err) + assert.Equal(t, "/a/b/.", path) + + path, err = ResolveDbfsPath("dbfs:/a/../c") + assert.NoError(t, err) + assert.Equal(t, "/a/../c", path) + + _, err = ResolveDbfsPath("dbf:/a/b/c") + assert.ErrorContains(t, err, "expected dbfs path (with the dbfs:/ prefix): dbf:/a/b/c") + + _, err = ResolveDbfsPath("/a/b/c") + assert.ErrorContains(t, err, "expected dbfs path (with the dbfs:/ prefix): /a/b/c") +} From 3dc47709d099784c0bd481aeb0e930e143143264 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 02:13:28 +0200 Subject: [PATCH 06/18] manually check for dbfs prefix --- libs/filer/dbfs_client.go | 14 +++----------- libs/filer/dbfs_client_test.go | 3 +++ 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/libs/filer/dbfs_client.go b/libs/filer/dbfs_client.go index efdcdf31c0..d1cf9270c7 100644 --- a/libs/filer/dbfs_client.go +++ b/libs/filer/dbfs_client.go @@ -7,9 +7,9 @@ import ( "io" "io/fs" "net/http" - "net/url" "path" "sort" + "strings" "time" "github.com/databricks/databricks-sdk-go" @@ -18,8 +18,6 @@ import ( "golang.org/x/exp/slices" ) -const DbfsScheme = "dbfs" - // Type that implements fs.DirEntry for DBFS. type dbfsDirEntry struct { dbfsFileInfo @@ -276,15 +274,9 @@ func (w *DbfsClient) Stat(ctx context.Context, name string) (fs.FileInfo, error) } func ResolveDbfsPath(path string) (string, error) { - fileUri, err := url.Parse(path) - if err != nil { - return "", err - } - - // Only dbfs file scheme is supported - if fileUri.Scheme != DbfsScheme { + if !strings.HasPrefix(path, "dbfs:/") { return "", fmt.Errorf("expected dbfs path (with the dbfs:/ prefix): %s", path) } - return fileUri.Path, nil + return strings.TrimPrefix(path, "dbfs:"), nil } diff --git a/libs/filer/dbfs_client_test.go b/libs/filer/dbfs_client_test.go index 7aca0383df..e9d1f136b2 100644 --- a/libs/filer/dbfs_client_test.go +++ b/libs/filer/dbfs_client_test.go @@ -32,4 +32,7 @@ func TestResolveDbfsPath(t *testing.T) { _, err = ResolveDbfsPath("/a/b/c") assert.ErrorContains(t, err, "expected dbfs path (with the dbfs:/ prefix): /a/b/c") + + _, err = ResolveDbfsPath("dbfs:a/b/c") + assert.ErrorContains(t, err, "expected dbfs path (with the dbfs:/ prefix): dbfs:a/b/c") } From 54a6602aaea3b944ab36af2da0dbc3a588fa46a8 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 03:07:19 +0200 Subject: [PATCH 07/18] Add fs rm command for dbfs --- cmd/fs/rm.go | 38 ++++++++++++ internal/rm_test.go | 146 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 184 insertions(+) create mode 100644 cmd/fs/rm.go create mode 100644 internal/rm_test.go diff --git a/cmd/fs/rm.go b/cmd/fs/rm.go new file mode 100644 index 0000000000..9e4bb68172 --- /dev/null +++ b/cmd/fs/rm.go @@ -0,0 +1,38 @@ +package fs + +import ( + "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/filer" + "github.com/databricks/databricks-sdk-go/service/files" + "github.com/spf13/cobra" +) + +var rmCmd = &cobra.Command{ + Use: "rm ", + Short: "Remove files and directories from dbfs.", + Long: `Remove files and directories from dbfs.`, + Args: cobra.ExactArgs(1), + PreRunE: root.MustWorkspaceClient, + + RunE: func(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + w := root.WorkspaceClient(ctx) + + path, err := filer.ResolveDbfsPath(args[0]) + if err != nil { + return err + } + + return w.Dbfs.Delete(ctx, files.Delete{ + Path: path, + Recursive: recursive, + }) + }, +} + +var recursive bool + +func init() { + rmCmd.Flags().BoolVarP(&recursive, "recursive", "r", false, "Allow deletion of non-empty directories.") + fsCmd.AddCommand(rmCmd) +} diff --git a/internal/rm_test.go b/internal/rm_test.go new file mode 100644 index 0000000000..5b11c29639 --- /dev/null +++ b/internal/rm_test.go @@ -0,0 +1,146 @@ +package internal + +import ( + "context" + "io/fs" + "path" + "strings" + "testing" + + "github.com/databricks/cli/libs/filer" + "github.com/databricks/databricks-sdk-go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFsRmForFile(t *testing.T) { + t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) + + ctx := context.Background() + w, err := databricks.NewWorkspaceClient() + require.NoError(t, err) + + tmpDir := temporaryDbfsDir(t, w) + + f, err := filer.NewDbfsClient(w, tmpDir) + require.NoError(t, err) + + // create file to delete + err = f.Write(ctx, "hello.txt", strings.NewReader("abc")) + require.NoError(t, err) + + // check file was created + info, err := f.Stat(ctx, "hello.txt") + require.NoError(t, err) + require.Equal(t, "hello.txt", info.Name()) + require.Equal(t, info.IsDir(), false) + + // Run rm command + stdout, stderr := RequireSuccessfulRun(t, "fs", "rm", "dbfs:"+path.Join(tmpDir, "hello.txt")) + assert.Equal(t, "", stderr.String()) + assert.Equal(t, "", stdout.String()) + + // assert file was deleted + _, err = f.Stat(ctx, "hello.txt") + assert.ErrorIs(t, err, fs.ErrNotExist) +} + +func TestFsRmForEmptyDirectory(t *testing.T) { + t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) + + ctx := context.Background() + w, err := databricks.NewWorkspaceClient() + require.NoError(t, err) + + tmpDir := temporaryDbfsDir(t, w) + + f, err := filer.NewDbfsClient(w, tmpDir) + require.NoError(t, err) + + // create directory to delete + err = f.Mkdir(ctx, "avacado") + require.NoError(t, err) + + // check directory was created + info, err := f.Stat(ctx, "avacado") + require.NoError(t, err) + require.Equal(t, "avacado", info.Name()) + require.Equal(t, info.IsDir(), true) + + // Run rm command + stdout, stderr := RequireSuccessfulRun(t, "fs", "rm", "dbfs:"+path.Join(tmpDir, "avacado")) + assert.Equal(t, "", stderr.String()) + assert.Equal(t, "", stdout.String()) + + // assert directory was deleted + _, err = f.Stat(ctx, "avacado") + assert.ErrorIs(t, err, fs.ErrNotExist) +} + +func TestFsRmForNonEmptyDirectory(t *testing.T) { + t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) + + ctx := context.Background() + w, err := databricks.NewWorkspaceClient() + require.NoError(t, err) + + tmpDir := temporaryDbfsDir(t, w) + + f, err := filer.NewDbfsClient(w, tmpDir) + require.NoError(t, err) + + // create file in dir + err = f.Write(ctx, "avacado/guacamole", strings.NewReader("abc"), filer.CreateParentDirectories) + require.NoError(t, err) + + // check file was created + info, err := f.Stat(ctx, "avacado/guacamole") + require.NoError(t, err) + require.Equal(t, "guacamole", info.Name()) + require.Equal(t, info.IsDir(), false) + + // Run rm command + _, _, err = RequireErrorRun(t, "fs", "rm", "dbfs:"+path.Join(tmpDir, "avacado")) + assert.ErrorContains(t, err, "Non-recursive delete of non-empty directory") +} + +func TestFsRmForNonExistantFile(t *testing.T) { + t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) + + // No error is returned on command run + stdout, stderr := RequireSuccessfulRun(t, "fs", "rm", "dbfs:/does-not-exist") + assert.Equal(t, "", stderr.String()) + assert.Equal(t, "", stdout.String()) +} + +func TestFsRmForNonEmptyDirectoryWithRecursiveFlag(t *testing.T) { + t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) + + ctx := context.Background() + w, err := databricks.NewWorkspaceClient() + require.NoError(t, err) + + tmpDir := temporaryDbfsDir(t, w) + + f, err := filer.NewDbfsClient(w, tmpDir) + require.NoError(t, err) + + // create file in dir + err = f.Write(ctx, "avacado/guacamole", strings.NewReader("abc"), filer.CreateParentDirectories) + require.NoError(t, err) + + // check file was created + info, err := f.Stat(ctx, "avacado/guacamole") + require.NoError(t, err) + require.Equal(t, "guacamole", info.Name()) + require.Equal(t, info.IsDir(), false) + + // Run rm command + stdout, stderr := RequireSuccessfulRun(t, "fs", "rm", "dbfs:"+path.Join(tmpDir, "avacado"), "--recursive") + assert.Equal(t, "", stderr.String()) + assert.Equal(t, "", stdout.String()) + + // assert directory was deleted + _, err = f.Stat(ctx, "avacado") + assert.ErrorIs(t, err, fs.ErrNotExist) +} From 19a20512e9870859113f1c28755d39c7695e7fd5 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 13:49:33 +0200 Subject: [PATCH 08/18] comments --- cmd/fs/ls.go | 36 +++++++++++++++++++++++++++++------- cmd/fs/ls_output.go | 27 --------------------------- cmd/root/io.go | 12 +++++------- libs/cmdio/io.go | 28 ++++++++++++---------------- 4 files changed, 46 insertions(+), 57 deletions(-) delete mode 100644 cmd/fs/ls_output.go diff --git a/cmd/fs/ls.go b/cmd/fs/ls.go index 76b51ce7ea..1df8dfcf48 100644 --- a/cmd/fs/ls.go +++ b/cmd/fs/ls.go @@ -1,7 +1,9 @@ package fs import ( + "io/fs" "sort" + "time" "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/libs/cmdio" @@ -9,6 +11,27 @@ import ( "github.com/spf13/cobra" ) +type jsonDirEntry struct { + Name string `json:"name"` + IsDir bool `json:"is_directory"` + Size int64 `json:"size"` + ModTime time.Time `json:"last_modified"` +} + +func toJsonDirEntry(f fs.DirEntry) (*jsonDirEntry, error) { + info, err := f.Info() + if err != nil { + return nil, err + } + + return &jsonDirEntry{ + Name: f.Name(), + IsDir: f.IsDir(), + Size: info.Size(), + ModTime: info.ModTime(), + }, nil +} + // lsCmd represents the ls command var lsCmd = &cobra.Command{ Use: "ls ", @@ -17,10 +40,6 @@ var lsCmd = &cobra.Command{ Args: cobra.ExactArgs(1), PreRunE: root.MustWorkspaceClient, Annotations: map[string]string{ - "template_long": cmdio.Heredoc(` - {{range .}}{{if .IsDir}}DIRECTORY {{else}}FILE {{end}}{{.Size}} {{.ModTime|pretty_date}} {{.Name}} - {{end}} - `), "template": cmdio.Heredoc(` {{range .}}{{.Name}} {{end}} @@ -46,9 +65,9 @@ var lsCmd = &cobra.Command{ return err } - lsOutputs := make([]lsOutput, 0) + lsOutputs := make([]jsonDirEntry, 0) for _, entry := range entries { - parsedEntry, err := toLsOutput(entry) + parsedEntry, err := toJsonDirEntry(entry) if err != nil { return err } @@ -60,7 +79,10 @@ var lsCmd = &cobra.Command{ // Use template for long mode if the flag is set if longMode { - return cmdio.RenderWithTemplate(ctx, lsOutputs, "template_long") + return cmdio.RenderWithTemplate(ctx, lsOutputs, cmdio.Heredoc(` + {{range .}}{{if .IsDir}}DIRECTORY {{else}}FILE {{end}}{{.Size}} {{.ModTime|pretty_date}} {{.Name}} + {{end}} + `)) } return cmdio.Render(ctx, lsOutputs) }, diff --git a/cmd/fs/ls_output.go b/cmd/fs/ls_output.go deleted file mode 100644 index e8728b0465..0000000000 --- a/cmd/fs/ls_output.go +++ /dev/null @@ -1,27 +0,0 @@ -package fs - -import ( - "io/fs" - "time" -) - -type lsOutput struct { - Name string `json:"name"` - IsDir bool `json:"is_directory"` - Size int64 `json:"size"` - ModTime time.Time `json:"last_modified"` -} - -func toLsOutput(f fs.DirEntry) (*lsOutput, error) { - info, err := f.Info() - if err != nil { - return nil, err - } - - return &lsOutput{ - Name: f.Name(), - IsDir: f.IsDir(), - Size: info.Size(), - ModTime: info.ModTime(), - }, nil -} diff --git a/cmd/root/io.go b/cmd/root/io.go index 07db9e0a0a..93830c804b 100644 --- a/cmd/root/io.go +++ b/cmd/root/io.go @@ -2,7 +2,6 @@ package root import ( "os" - "strings" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/flags" @@ -28,14 +27,13 @@ func OutputType() flags.Output { } func initializeIO(cmd *cobra.Command) error { - templates := make(map[string]string, 0) - for k, v := range cmd.Annotations { - if strings.Contains(k, "template") { - templates[k] = v - } + var template string + if cmd.Annotations != nil { + // rely on zeroval being an empty string + template = cmd.Annotations["template"] } - cmdIO := cmdio.NewIO(outputType, cmd.InOrStdin(), cmd.OutOrStdout(), cmd.ErrOrStderr(), templates) + cmdIO := cmdio.NewIO(outputType, cmd.InOrStdin(), cmd.OutOrStdout(), cmd.ErrOrStderr(), template) ctx := cmdio.InContext(cmd.Context(), cmdIO) cmd.SetContext(ctx) diff --git a/libs/cmdio/io.go b/libs/cmdio/io.go index 4f134c5100..1df6f5c188 100644 --- a/libs/cmdio/io.go +++ b/libs/cmdio/io.go @@ -24,17 +24,17 @@ type cmdIO struct { // e.g. if stdout is a terminal interactive bool outputFormat flags.Output - templates map[string]string + template string in io.Reader out io.Writer err io.Writer } -func NewIO(outputFormat flags.Output, in io.Reader, out io.Writer, err io.Writer, templates map[string]string) *cmdIO { +func NewIO(outputFormat flags.Output, in io.Reader, out io.Writer, err io.Writer, template string) *cmdIO { return &cmdIO{ interactive: !color.NoColor, outputFormat: outputFormat, - templates: templates, + template: template, in: in, out: out, err: err, @@ -66,14 +66,20 @@ func (c *cmdIO) IsTTY() bool { return isatty.IsTerminal(fd) || isatty.IsCygwinTerminal(fd) } -func (c *cmdIO) Render(v any, templateName string) error { +func Render(ctx context.Context, v any) error { + c := fromContext(ctx) + return RenderWithTemplate(ctx, v, c.template) +} + +func RenderWithTemplate(ctx context.Context, v any, template string) error { // TODO: add terminal width & white/dark theme detection + c := fromContext(ctx) switch c.outputFormat { case flags.OutputJSON: return renderJson(c.out, v) case flags.OutputText: - if c.templates[templateName] != "" { - return renderTemplate(c.out, c.templates[templateName], v) + if template != "" { + return renderTemplate(c.out, template, v) } return renderJson(c.out, v) default: @@ -81,16 +87,6 @@ func (c *cmdIO) Render(v any, templateName string) error { } } -func Render(ctx context.Context, v any) error { - c := fromContext(ctx) - return c.Render(v, "template") -} - -func RenderWithTemplate(ctx context.Context, v any, templateName string) error { - c := fromContext(ctx) - return c.Render(v, templateName) -} - type tuple struct{ Name, Id string } func (c *cmdIO) Select(names map[string]string, label string) (id string, err error) { From 8050dc524f0433b1eb9cf30854b318097a67f954 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 13:53:38 +0200 Subject: [PATCH 09/18] move sort outside --- cmd/fs/ls.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cmd/fs/ls.go b/cmd/fs/ls.go index 1df8dfcf48..43f0a4993d 100644 --- a/cmd/fs/ls.go +++ b/cmd/fs/ls.go @@ -65,26 +65,26 @@ var lsCmd = &cobra.Command{ return err } - lsOutputs := make([]jsonDirEntry, 0) + jsonDirEntries := make([]jsonDirEntry, 0) for _, entry := range entries { - parsedEntry, err := toJsonDirEntry(entry) + jsonDirEntry, err := toJsonDirEntry(entry) if err != nil { return err } - lsOutputs = append(lsOutputs, *parsedEntry) - sort.Slice(lsOutputs, func(i, j int) bool { - return lsOutputs[i].Name < lsOutputs[j].Name - }) + jsonDirEntries = append(jsonDirEntries, *jsonDirEntry) } + sort.Slice(jsonDirEntries, func(i, j int) bool { + return jsonDirEntries[i].Name < jsonDirEntries[j].Name + }) // Use template for long mode if the flag is set if longMode { - return cmdio.RenderWithTemplate(ctx, lsOutputs, cmdio.Heredoc(` + return cmdio.RenderWithTemplate(ctx, jsonDirEntries, cmdio.Heredoc(` {{range .}}{{if .IsDir}}DIRECTORY {{else}}FILE {{end}}{{.Size}} {{.ModTime|pretty_date}} {{.Name}} {{end}} `)) } - return cmdio.Render(ctx, lsOutputs) + return cmdio.Render(ctx, jsonDirEntries) }, } From fc7fbbec6f86d80d8d655e224607fc55bab31a49 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 13:55:29 +0200 Subject: [PATCH 10/18] - --- cmd/fs/ls.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/fs/ls.go b/cmd/fs/ls.go index 43f0a4993d..c92393c08d 100644 --- a/cmd/fs/ls.go +++ b/cmd/fs/ls.go @@ -91,6 +91,6 @@ var lsCmd = &cobra.Command{ var longMode bool func init() { - lsCmd.Flags().BoolVarP(&longMode, "long", "l", false, "Displays full information including size, file type and modification time since Epoch in milliseconds.") + lsCmd.Flags().BoolVarP(&longMode, "long", "l", false, "Displays full information including size, file type and modification time since Epoch in milliseconds.") fsCmd.AddCommand(lsCmd) } From b048c8675d9136fc70e3d2ceade892533ab2e67d Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 14:01:06 +0200 Subject: [PATCH 11/18] address comments --- cmd/fs/helpers.go | 14 ++++++++++++++ .../fs/helpers_test.go | 18 +++++++++--------- cmd/fs/ls.go | 4 ++-- libs/filer/dbfs_client.go | 10 ---------- 4 files changed, 25 insertions(+), 21 deletions(-) create mode 100644 cmd/fs/helpers.go rename libs/filer/dbfs_client_test.go => cmd/fs/helpers_test.go (65%) diff --git a/cmd/fs/helpers.go b/cmd/fs/helpers.go new file mode 100644 index 0000000000..e456bff985 --- /dev/null +++ b/cmd/fs/helpers.go @@ -0,0 +1,14 @@ +package fs + +import ( + "fmt" + "strings" +) + +func resolveDbfsPath(path string) (string, error) { + if !strings.HasPrefix(path, "dbfs:/") { + return "", fmt.Errorf("expected dbfs path (with the dbfs:/ prefix): %s", path) + } + + return strings.TrimPrefix(path, "dbfs:"), nil +} diff --git a/libs/filer/dbfs_client_test.go b/cmd/fs/helpers_test.go similarity index 65% rename from libs/filer/dbfs_client_test.go rename to cmd/fs/helpers_test.go index e9d1f136b2..1d174ef951 100644 --- a/libs/filer/dbfs_client_test.go +++ b/cmd/fs/helpers_test.go @@ -1,4 +1,4 @@ -package filer +package fs import ( "testing" @@ -7,32 +7,32 @@ import ( ) func TestResolveDbfsPath(t *testing.T) { - path, err := ResolveDbfsPath("dbfs:/") + path, err := resolveDbfsPath("dbfs:/") assert.NoError(t, err) assert.Equal(t, "/", path) - path, err = ResolveDbfsPath("dbfs:/abc") + path, err = resolveDbfsPath("dbfs:/abc") assert.NoError(t, err) assert.Equal(t, "/abc", path) - path, err = ResolveDbfsPath("dbfs:/a/b/c") + path, err = resolveDbfsPath("dbfs:/a/b/c") assert.NoError(t, err) assert.Equal(t, "/a/b/c", path) - path, err = ResolveDbfsPath("dbfs:/a/b/.") + path, err = resolveDbfsPath("dbfs:/a/b/.") assert.NoError(t, err) assert.Equal(t, "/a/b/.", path) - path, err = ResolveDbfsPath("dbfs:/a/../c") + path, err = resolveDbfsPath("dbfs:/a/../c") assert.NoError(t, err) assert.Equal(t, "/a/../c", path) - _, err = ResolveDbfsPath("dbf:/a/b/c") + _, err = resolveDbfsPath("dbf:/a/b/c") assert.ErrorContains(t, err, "expected dbfs path (with the dbfs:/ prefix): dbf:/a/b/c") - _, err = ResolveDbfsPath("/a/b/c") + _, err = resolveDbfsPath("/a/b/c") assert.ErrorContains(t, err, "expected dbfs path (with the dbfs:/ prefix): /a/b/c") - _, err = ResolveDbfsPath("dbfs:a/b/c") + _, err = resolveDbfsPath("dbfs:a/b/c") assert.ErrorContains(t, err, "expected dbfs path (with the dbfs:/ prefix): dbfs:a/b/c") } diff --git a/cmd/fs/ls.go b/cmd/fs/ls.go index c92393c08d..7049db049f 100644 --- a/cmd/fs/ls.go +++ b/cmd/fs/ls.go @@ -34,7 +34,7 @@ func toJsonDirEntry(f fs.DirEntry) (*jsonDirEntry, error) { // lsCmd represents the ls command var lsCmd = &cobra.Command{ - Use: "ls ", + Use: "ls DIR_PATH", Short: "Lists files", Long: `Lists files`, Args: cobra.ExactArgs(1), @@ -50,7 +50,7 @@ var lsCmd = &cobra.Command{ ctx := cmd.Context() w := root.WorkspaceClient(ctx) - path, err := filer.ResolveDbfsPath(args[0]) + path, err := resolveDbfsPath(args[0]) if err != nil { return err } diff --git a/libs/filer/dbfs_client.go b/libs/filer/dbfs_client.go index d1cf9270c7..67878136b7 100644 --- a/libs/filer/dbfs_client.go +++ b/libs/filer/dbfs_client.go @@ -3,13 +3,11 @@ package filer import ( "context" "errors" - "fmt" "io" "io/fs" "net/http" "path" "sort" - "strings" "time" "github.com/databricks/databricks-sdk-go" @@ -272,11 +270,3 @@ func (w *DbfsClient) Stat(ctx context.Context, name string) (fs.FileInfo, error) return dbfsFileInfo{*info}, nil } - -func ResolveDbfsPath(path string) (string, error) { - if !strings.HasPrefix(path, "dbfs:/") { - return "", fmt.Errorf("expected dbfs path (with the dbfs:/ prefix): %s", path) - } - - return strings.TrimPrefix(path, "dbfs:"), nil -} From 265e517126d08eea462bce16b032b839ca7d3847 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 15:07:08 +0200 Subject: [PATCH 12/18] address comments 2 --- cmd/fs/ls.go | 11 ++++------- libs/cmdio/render.go | 2 +- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/cmd/fs/ls.go b/cmd/fs/ls.go index 7049db049f..2e1e4f61f3 100644 --- a/cmd/fs/ls.go +++ b/cmd/fs/ls.go @@ -39,12 +39,6 @@ var lsCmd = &cobra.Command{ Long: `Lists files`, Args: cobra.ExactArgs(1), PreRunE: root.MustWorkspaceClient, - Annotations: map[string]string{ - "template": cmdio.Heredoc(` - {{range .}}{{.Name}} - {{end}} - `), - }, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() @@ -84,7 +78,10 @@ var lsCmd = &cobra.Command{ {{end}} `)) } - return cmdio.Render(ctx, jsonDirEntries) + return cmdio.RenderWithTemplate(ctx, jsonDirEntries, cmdio.Heredoc(` + {{range .}}{{.Name}} + {{end}} + `)) }, } diff --git a/libs/cmdio/render.go b/libs/cmdio/render.go index 063d7cbcb4..2e42c32c28 100644 --- a/libs/cmdio/render.go +++ b/libs/cmdio/render.go @@ -88,7 +88,7 @@ func renderTemplate(w io.Writer, tmpl string, v any) error { return string(b), nil }, "pretty_date": func(t time.Time) string { - return t.UTC().Format("2006-01-02T15:04:05Z") + return t.Format("2006-01-02T15:04:05Z") }, }).Parse(tmpl) if err != nil { From 08826094ce10142cd0524bfd0eed47612aa6c842 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 15:09:09 +0200 Subject: [PATCH 13/18] added preallocation of size --- cmd/fs/ls.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/fs/ls.go b/cmd/fs/ls.go index 2e1e4f61f3..85e3ffdfab 100644 --- a/cmd/fs/ls.go +++ b/cmd/fs/ls.go @@ -59,13 +59,13 @@ var lsCmd = &cobra.Command{ return err } - jsonDirEntries := make([]jsonDirEntry, 0) - for _, entry := range entries { + jsonDirEntries := make([]jsonDirEntry, len(entries)) + for i, entry := range entries { jsonDirEntry, err := toJsonDirEntry(entry) if err != nil { return err } - jsonDirEntries = append(jsonDirEntries, *jsonDirEntry) + jsonDirEntries[i] = *jsonDirEntry } sort.Slice(jsonDirEntries, func(i, j int) bool { return jsonDirEntries[i].Name < jsonDirEntries[j].Name From 9160a6e9ed2566b0b4d36a5388f0f70563509173 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 15:15:21 +0200 Subject: [PATCH 14/18] initialize dbfs client at root --- cmd/fs/ls.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/fs/ls.go b/cmd/fs/ls.go index 85e3ffdfab..200cbed526 100644 --- a/cmd/fs/ls.go +++ b/cmd/fs/ls.go @@ -49,12 +49,12 @@ var lsCmd = &cobra.Command{ return err } - f, err := filer.NewDbfsClient(w, path) + f, err := filer.NewDbfsClient(w, "/") if err != nil { return err } - entries, err := f.ReadDir(ctx, "") + entries, err := f.ReadDir(ctx, path) if err != nil { return err } From 25aa5b667cb93309d6cc0b4716ae6e49b037e1c9 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 15:42:56 +0200 Subject: [PATCH 15/18] - --- cmd/fs/rm.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cmd/fs/rm.go b/cmd/fs/rm.go index 9e4bb68172..aa45767eb9 100644 --- a/cmd/fs/rm.go +++ b/cmd/fs/rm.go @@ -2,13 +2,12 @@ package fs import ( "github.com/databricks/cli/cmd/root" - "github.com/databricks/cli/libs/filer" "github.com/databricks/databricks-sdk-go/service/files" "github.com/spf13/cobra" ) var rmCmd = &cobra.Command{ - Use: "rm ", + Use: "rm DIR_PATH", Short: "Remove files and directories from dbfs.", Long: `Remove files and directories from dbfs.`, Args: cobra.ExactArgs(1), @@ -18,7 +17,7 @@ var rmCmd = &cobra.Command{ ctx := cmd.Context() w := root.WorkspaceClient(ctx) - path, err := filer.ResolveDbfsPath(args[0]) + path, err := resolveDbfsPath(args[0]) if err != nil { return err } From b64b7ab1a7113f4c633b9b2bb648b9ba3911fa4c Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 17:48:21 +0200 Subject: [PATCH 16/18] - --- cmd/fs/rm.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/fs/rm.go b/cmd/fs/rm.go index aa45767eb9..ea35cf7273 100644 --- a/cmd/fs/rm.go +++ b/cmd/fs/rm.go @@ -7,7 +7,7 @@ import ( ) var rmCmd = &cobra.Command{ - Use: "rm DIR_PATH", + Use: "rm PATH", Short: "Remove files and directories from dbfs.", Long: `Remove files and directories from dbfs.`, Args: cobra.ExactArgs(1), @@ -32,6 +32,6 @@ var rmCmd = &cobra.Command{ var recursive bool func init() { - rmCmd.Flags().BoolVarP(&recursive, "recursive", "r", false, "Allow deletion of non-empty directories.") + rmCmd.Flags().BoolVarP(&recursive, "recursive", "r", false, "Recursively delete a non-empty directory.") fsCmd.AddCommand(rmCmd) } From 5619d0fed7ef2063fc3d711b641783a9d2346358 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 17:48:55 +0200 Subject: [PATCH 17/18] - --- internal/ls_test.go | 104 -------------------------------------------- 1 file changed, 104 deletions(-) delete mode 100644 internal/ls_test.go diff --git a/internal/ls_test.go b/internal/ls_test.go deleted file mode 100644 index 060c706df6..0000000000 --- a/internal/ls_test.go +++ /dev/null @@ -1,104 +0,0 @@ -package internal - -import ( - "context" - "encoding/json" - "io/fs" - "path" - "regexp" - "strings" - "testing" - - _ "github.com/databricks/cli/cmd/fs" - "github.com/databricks/cli/libs/filer" - "github.com/databricks/databricks-sdk-go" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestFsLsForDbfs(t *testing.T) { - t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) - - ctx := context.Background() - w, err := databricks.NewWorkspaceClient() - require.NoError(t, err) - - tmpDir := temporaryDbfsDir(t, w) - - f, err := filer.NewDbfsClient(w, tmpDir) - require.NoError(t, err) - - err = f.Mkdir(ctx, "a") - require.NoError(t, err) - err = f.Write(ctx, "a/hello.txt", strings.NewReader("abc"), filer.CreateParentDirectories) - require.NoError(t, err) - err = f.Write(ctx, "bye.txt", strings.NewReader("def")) - require.NoError(t, err) - - stdout, stderr := RequireSuccessfulRun(t, "fs", "ls", "dbfs:"+tmpDir, "--output=json") - assert.Equal(t, "", stderr.String()) - var parsedStdout []map[string]any - err = json.Unmarshal(stdout.Bytes(), &parsedStdout) - require.NoError(t, err) - - // assert on ls output - assert.Equal(t, "a", parsedStdout[0]["name"]) - assert.Equal(t, true, parsedStdout[0]["is_directory"]) - assert.Equal(t, float64(0), parsedStdout[0]["size"]) - assert.Equal(t, "bye.txt", parsedStdout[1]["name"]) - assert.Equal(t, false, parsedStdout[1]["is_directory"]) - assert.Equal(t, float64(3), parsedStdout[1]["size"]) -} - -func TestFsLsForDbfsOnFile(t *testing.T) { - t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) - - ctx := context.Background() - w, err := databricks.NewWorkspaceClient() - require.NoError(t, err) - - tmpDir := temporaryDbfsDir(t, w) - - f, err := filer.NewDbfsClient(w, tmpDir) - require.NoError(t, err) - - err = f.Mkdir(ctx, "a") - require.NoError(t, err) - err = f.Write(ctx, "a/hello.txt", strings.NewReader("abc"), filer.CreateParentDirectories) - require.NoError(t, err) - - _, _, err = RequireErrorRun(t, "fs", "ls", "dbfs:"+path.Join(tmpDir, "a", "hello.txt"), "--output=json") - assert.Regexp(t, regexp.MustCompile("not a directory: .*/a/hello.txt"), err.Error()) -} - -func TestFsLsForDbfsOnEmptyDir(t *testing.T) { - t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) - - w, err := databricks.NewWorkspaceClient() - require.NoError(t, err) - - tmpDir := temporaryDbfsDir(t, w) - - stdout, stderr := RequireSuccessfulRun(t, "fs", "ls", "dbfs:"+tmpDir, "--output=json") - assert.Equal(t, "", stderr.String()) - var parsedStdout []map[string]any - err = json.Unmarshal(stdout.Bytes(), &parsedStdout) - require.NoError(t, err) - - // assert on ls output - assert.Equal(t, 0, len(parsedStdout)) -} - -func TestFsLsForDbfsForNonexistingDir(t *testing.T) { - t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) - - _, _, err := RequireErrorRun(t, "fs", "ls", "dbfs:/john-cena", "--output=json") - assert.ErrorIs(t, err, fs.ErrNotExist) -} - -func TestFsLsWithoutScheme(t *testing.T) { - t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) - - _, _, err := RequireErrorRun(t, "fs", "ls", "/ray-mysterio", "--output=json") - assert.ErrorContains(t, err, "expected dbfs path (with the dbfs:/ prefix): /ray-mysterio") -} From da652c9957b790972649cd932243c6ed22b24db7 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Tue, 6 Jun 2023 01:03:20 +0200 Subject: [PATCH 18/18] - --- internal/rm_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/rm_test.go b/internal/rm_test.go index 5b11c29639..dd6a285934 100644 --- a/internal/rm_test.go +++ b/internal/rm_test.go @@ -104,7 +104,7 @@ func TestFsRmForNonEmptyDirectory(t *testing.T) { assert.ErrorContains(t, err, "Non-recursive delete of non-empty directory") } -func TestFsRmForNonExistantFile(t *testing.T) { +func TestFsRmForNonExistentFile(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) // No error is returned on command run