From b5f5f1103f7d4dac8af33b09ddb8e318830bc846 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Fri, 2 Jun 2023 17:24:04 +0200 Subject: [PATCH 01/21] 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/21] 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/21] 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/21] 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/21] 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 3e947bbeedeead1457705f03d73559cca0cb68c2 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 02:08:48 +0200 Subject: [PATCH 06/21] Add fs cat command for dbfs files --- cmd/fs/cat.go | 52 ++++++++++++++++++++++++++++++++++++++ cmd/fs/cat_output.go | 11 +++++++++ internal/cat_test.go | 59 ++++++++++++++++++++++++++++++++++++++++++++ libs/filer/filer.go | 12 +++++++++ 4 files changed, 134 insertions(+) create mode 100644 cmd/fs/cat.go create mode 100644 cmd/fs/cat_output.go create mode 100644 internal/cat_test.go diff --git a/cmd/fs/cat.go b/cmd/fs/cat.go new file mode 100644 index 0000000000..a0e8d2840a --- /dev/null +++ b/cmd/fs/cat.go @@ -0,0 +1,52 @@ +package fs + +import ( + "io/ioutil" + + "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/filer" + "github.com/spf13/cobra" +) + +var catCmd = &cobra.Command{ + Use: "cat ", + Short: "Show file content", + Long: `Show the contents of a file. Does not work for directories.`, + Args: cobra.ExactArgs(1), + PreRunE: root.MustWorkspaceClient, + Annotations: map[string]string{ + "template": cmdio.Heredoc(`{{.Content}}`), + }, + + 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 + } + + f, err := filer.NewDbfsClient(w, "/") + if err != nil { + return err + } + + r, err := f.Read(ctx, path) + if err != nil { + return err + } + + b, err := ioutil.ReadAll(r) + if err != nil { + return err + } + content := string(b) + return cmdio.Render(ctx, toCatOutput(content)) + }, +} + +func init() { + fsCmd.AddCommand(catCmd) +} diff --git a/cmd/fs/cat_output.go b/cmd/fs/cat_output.go new file mode 100644 index 0000000000..7519eb7cec --- /dev/null +++ b/cmd/fs/cat_output.go @@ -0,0 +1,11 @@ +package fs + +type catOutput struct { + Content string `json:"content"` +} + +func toCatOutput(content string) *catOutput { + return &catOutput{ + Content: content, + } +} diff --git a/internal/cat_test.go b/internal/cat_test.go new file mode 100644 index 0000000000..3fffbe4ff3 --- /dev/null +++ b/internal/cat_test.go @@ -0,0 +1,59 @@ +package internal + +import ( + "context" + "encoding/json" + "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 TestFsCatForDbfs(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.Write(ctx, "a/hello.txt", strings.NewReader("abc"), filer.CreateParentDirectories) + require.NoError(t, err) + + stdout, stderr := RequireSuccessfulRun(t, "fs", "cat", "dbfs:"+path.Join(tmpDir, "a", "hello.txt"), "--output=json") + assert.Equal(t, "", stderr.String()) + var parsedStdout map[string]any + err = json.Unmarshal(stdout.Bytes(), &parsedStdout) + require.NoError(t, err) + + // assert on cat output + assert.Equal(t, map[string]any{ + "content": "abc", + }, parsedStdout) +} + +func TestFsCatForDbfsOnNonExistantFile(t *testing.T) { + t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) + + _, _, err := RequireErrorRun(t, "fs", "cat", "dbfs:/non-existant-file", "--output=json") + assert.ErrorIs(t, err, fs.ErrNotExist) +} + +func TestFsCatForDbfsInvalidScheme(t *testing.T) { + // t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) + + _, _, err := RequireErrorRun(t, "fs", "cat", "dbfs:non-existant-file", "--output=json") + assert.ErrorIs(t, err, fs.ErrNotExist) +} + +// TODO: Add test asserting an error when cat is called on an directory. Need this to be +// fixed in the SDK first (https://github.com/databricks/databricks-sdk-go/issues/414) diff --git a/libs/filer/filer.go b/libs/filer/filer.go index 58273fff36..cad1788664 100644 --- a/libs/filer/filer.go +++ b/libs/filer/filer.go @@ -62,6 +62,18 @@ func (err NotADirectory) Is(other error) bool { return other == fs.ErrInvalid } +type NotAFile struct { + path string +} + +func (err NotAFile) Error() string { + return fmt.Sprintf("not a file: %s", err.path) +} + +func (err NotAFile) Is(other error) bool { + return other == fs.ErrInvalid +} + // Filer is used to access files in a workspace. // It has implementations for accessing files in WSFS and in DBFS. type Filer interface { From 3dc47709d099784c0bd481aeb0e930e143143264 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 02:13:28 +0200 Subject: [PATCH 07/21] 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 19d838761c323748103cc5c0f192bb8cfde5308b Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 02:15:16 +0200 Subject: [PATCH 08/21] fin --- internal/cat_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/cat_test.go b/internal/cat_test.go index 3fffbe4ff3..d738bba2c9 100644 --- a/internal/cat_test.go +++ b/internal/cat_test.go @@ -49,10 +49,10 @@ func TestFsCatForDbfsOnNonExistantFile(t *testing.T) { } func TestFsCatForDbfsInvalidScheme(t *testing.T) { - // t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) + t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) - _, _, err := RequireErrorRun(t, "fs", "cat", "dbfs:non-existant-file", "--output=json") - assert.ErrorIs(t, err, fs.ErrNotExist) + _, _, err := RequireErrorRun(t, "fs", "cat", "dab:/non-existant-file", "--output=json") + assert.ErrorContains(t, err, "expected dbfs path (with the dbfs:/ prefix): dab:/non-existant-file") } // TODO: Add test asserting an error when cat is called on an directory. Need this to be From 19a20512e9870859113f1c28755d39c7695e7fd5 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 13:49:33 +0200 Subject: [PATCH 09/21] 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 10/21] 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 11/21] - --- 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 12/21] 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 13/21] 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 14/21] 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 15/21] 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 bd9d9807a6fcc777e0b8602a6611b2b5272b6f10 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 15:31:34 +0200 Subject: [PATCH 16/21] address comments --- cmd/fs/cat.go | 19 ++++--------------- cmd/fs/cat_output.go | 11 ----------- internal/cat_test.go | 35 +++++++++++++++++++++++------------ libs/cmdio/io.go | 13 +++++++++++++ 4 files changed, 40 insertions(+), 38 deletions(-) delete mode 100644 cmd/fs/cat_output.go diff --git a/cmd/fs/cat.go b/cmd/fs/cat.go index a0e8d2840a..01f28d3806 100644 --- a/cmd/fs/cat.go +++ b/cmd/fs/cat.go @@ -1,8 +1,6 @@ package fs import ( - "io/ioutil" - "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/filer" @@ -10,20 +8,17 @@ import ( ) var catCmd = &cobra.Command{ - Use: "cat ", + Use: "cat FILE_PATH", Short: "Show file content", - Long: `Show the contents of a file. Does not work for directories.`, + Long: `Show the contents of a file.`, Args: cobra.ExactArgs(1), PreRunE: root.MustWorkspaceClient, - Annotations: map[string]string{ - "template": cmdio.Heredoc(`{{.Content}}`), - }, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() w := root.WorkspaceClient(ctx) - path, err := filer.ResolveDbfsPath(args[0]) + path, err := resolveDbfsPath(args[0]) if err != nil { return err } @@ -37,13 +32,7 @@ var catCmd = &cobra.Command{ if err != nil { return err } - - b, err := ioutil.ReadAll(r) - if err != nil { - return err - } - content := string(b) - return cmdio.Render(ctx, toCatOutput(content)) + return cmdio.RenderReader(ctx, r) }, } diff --git a/cmd/fs/cat_output.go b/cmd/fs/cat_output.go deleted file mode 100644 index 7519eb7cec..0000000000 --- a/cmd/fs/cat_output.go +++ /dev/null @@ -1,11 +0,0 @@ -package fs - -type catOutput struct { - Content string `json:"content"` -} - -func toCatOutput(content string) *catOutput { - return &catOutput{ - Content: content, - } -} diff --git a/internal/cat_test.go b/internal/cat_test.go index d738bba2c9..bf50446973 100644 --- a/internal/cat_test.go +++ b/internal/cat_test.go @@ -2,7 +2,6 @@ package internal import ( "context" - "encoding/json" "io/fs" "path" "strings" @@ -29,31 +28,43 @@ func TestFsCatForDbfs(t *testing.T) { err = f.Write(ctx, "a/hello.txt", strings.NewReader("abc"), filer.CreateParentDirectories) require.NoError(t, err) - stdout, stderr := RequireSuccessfulRun(t, "fs", "cat", "dbfs:"+path.Join(tmpDir, "a", "hello.txt"), "--output=json") + stdout, stderr := RequireSuccessfulRun(t, "fs", "cat", "dbfs:"+path.Join(tmpDir, "a", "hello.txt")) assert.Equal(t, "", stderr.String()) - var parsedStdout map[string]any - err = json.Unmarshal(stdout.Bytes(), &parsedStdout) - require.NoError(t, err) - - // assert on cat output - assert.Equal(t, map[string]any{ - "content": "abc", - }, parsedStdout) + assert.Equal(t, "abc", stdout.String()) } func TestFsCatForDbfsOnNonExistantFile(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) - _, _, err := RequireErrorRun(t, "fs", "cat", "dbfs:/non-existant-file", "--output=json") + _, _, err := RequireErrorRun(t, "fs", "cat", "dbfs:/non-existant-file") assert.ErrorIs(t, err, fs.ErrNotExist) } func TestFsCatForDbfsInvalidScheme(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) - _, _, err := RequireErrorRun(t, "fs", "cat", "dab:/non-existant-file", "--output=json") + _, _, err := RequireErrorRun(t, "fs", "cat", "dab:/non-existant-file") assert.ErrorContains(t, err, "expected dbfs path (with the dbfs:/ prefix): dab:/non-existant-file") } +func TestFsCatDoesNotSupportOutputModeJson(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.Write(ctx, "hello.txt", strings.NewReader("abc")) + require.NoError(t, err) + + _, _, err = RequireErrorRun(t, "fs", "cat", "dbfs:"+path.Join(tmpDir, "hello.txt"), "--output=json") + assert.ErrorContains(t, err, "json output not supported") +} + // TODO: Add test asserting an error when cat is called on an directory. Need this to be // fixed in the SDK first (https://github.com/databricks/databricks-sdk-go/issues/414) diff --git a/libs/cmdio/io.go b/libs/cmdio/io.go index 1df6f5c188..32637b1d4d 100644 --- a/libs/cmdio/io.go +++ b/libs/cmdio/io.go @@ -87,6 +87,19 @@ func RenderWithTemplate(ctx context.Context, v any, template string) error { } } +func RenderReader(ctx context.Context, r io.Reader) error { + c := fromContext(ctx) + switch c.outputFormat { + case flags.OutputJSON: + return fmt.Errorf("json output not supported") + case flags.OutputText: + _, err := io.Copy(c.out, r) + return err + default: + return fmt.Errorf("invalid output format: %s", c.outputFormat) + } +} + type tuple struct{ Name, Id string } func (c *cmdIO) Select(names map[string]string, label string) (id string, err error) { From 8c4c41ac8ae186be387098caff9166249e79a731 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 15:34:03 +0200 Subject: [PATCH 17/21] remove unused error --- libs/filer/filer.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/libs/filer/filer.go b/libs/filer/filer.go index cad1788664..58273fff36 100644 --- a/libs/filer/filer.go +++ b/libs/filer/filer.go @@ -62,18 +62,6 @@ func (err NotADirectory) Is(other error) bool { return other == fs.ErrInvalid } -type NotAFile struct { - path string -} - -func (err NotAFile) Error() string { - return fmt.Sprintf("not a file: %s", err.path) -} - -func (err NotAFile) Is(other error) bool { - return other == fs.ErrInvalid -} - // Filer is used to access files in a workspace. // It has implementations for accessing files in WSFS and in DBFS. type Filer interface { From bf37dc498373c828351f93bd9faec7302cb40773 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 15:39:37 +0200 Subject: [PATCH 18/21] - --- internal/cat_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/cat_test.go b/internal/cat_test.go index bf50446973..daf04e26c7 100644 --- a/internal/cat_test.go +++ b/internal/cat_test.go @@ -48,7 +48,7 @@ func TestFsCatForDbfsInvalidScheme(t *testing.T) { } func TestFsCatDoesNotSupportOutputModeJson(t *testing.T) { - // t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) + t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) ctx := context.Background() w, err := databricks.NewWorkspaceClient() From 0202679d28135e9d9b53b60eed3cbe1c2e7a23fc Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 5 Jun 2023 17:36:36 +0200 Subject: [PATCH 19/21] - --- internal/{cat_test.go => fs_cat_test.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename internal/{cat_test.go => fs_cat_test.go} (100%) diff --git a/internal/cat_test.go b/internal/fs_cat_test.go similarity index 100% rename from internal/cat_test.go rename to internal/fs_cat_test.go From 588b9532accc3cd5bbb2411a3a4ad280e04c4a99 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Tue, 6 Jun 2023 01:07:56 +0200 Subject: [PATCH 20/21] - --- 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 abfe279fb663ef0d9fda4dbee62a8875be80b889 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Tue, 6 Jun 2023 01:09:31 +0200 Subject: [PATCH 21/21] - --- internal/fs_cat_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/fs_cat_test.go b/internal/fs_cat_test.go index daf04e26c7..5d6952f4fe 100644 --- a/internal/fs_cat_test.go +++ b/internal/fs_cat_test.go @@ -33,18 +33,18 @@ func TestFsCatForDbfs(t *testing.T) { assert.Equal(t, "abc", stdout.String()) } -func TestFsCatForDbfsOnNonExistantFile(t *testing.T) { +func TestFsCatForDbfsOnNonExistentFile(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) - _, _, err := RequireErrorRun(t, "fs", "cat", "dbfs:/non-existant-file") + _, _, err := RequireErrorRun(t, "fs", "cat", "dbfs:/non-existent-file") assert.ErrorIs(t, err, fs.ErrNotExist) } func TestFsCatForDbfsInvalidScheme(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) - _, _, err := RequireErrorRun(t, "fs", "cat", "dab:/non-existant-file") - assert.ErrorContains(t, err, "expected dbfs path (with the dbfs:/ prefix): dab:/non-existant-file") + _, _, err := RequireErrorRun(t, "fs", "cat", "dab:/non-existent-file") + assert.ErrorContains(t, err, "expected dbfs path (with the dbfs:/ prefix): dab:/non-existent-file") } func TestFsCatDoesNotSupportOutputModeJson(t *testing.T) {