From e89ffe9c43d7ffc0e090c5ad45e62fc2b0a78615 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 15 Aug 2024 11:38:44 +0200 Subject: [PATCH 1/6] Print non-json logs in `import-dir` and `export-dir` commands --- cmd/workspace/workspace/export_dir.go | 5 ++--- cmd/workspace/workspace/import_dir.go | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/cmd/workspace/workspace/export_dir.go b/cmd/workspace/workspace/export_dir.go index 0b53666f9b..7a3ec9a700 100644 --- a/cmd/workspace/workspace/export_dir.go +++ b/cmd/workspace/workspace/export_dir.go @@ -110,8 +110,7 @@ func newExportDir() *cobra.Command { } workspaceFS := filer.NewFS(ctx, workspaceFiler) - // TODO: print progress events on stderr instead: https://github.com/databricks/cli/issues/448 - err = cmdio.RenderJson(ctx, newExportStartedEvent(opts.sourceDir)) + err = cmdio.RenderWithTemplate(ctx, newExportStartedEvent(opts.sourceDir), "", "Exporting files from {{.SourcePath}}") if err != nil { return err } @@ -120,7 +119,7 @@ func newExportDir() *cobra.Command { if err != nil { return err } - return cmdio.RenderJson(ctx, newExportCompletedEvent(opts.targetDir)) + return cmdio.RenderWithTemplate(ctx, newImportCompletedEvent(opts.targetDir), "", "Exported complete. The files are available at {{.TargetPath}}\n") } return cmd diff --git a/cmd/workspace/workspace/import_dir.go b/cmd/workspace/workspace/import_dir.go index 19d9a0a172..c68c4d6128 100644 --- a/cmd/workspace/workspace/import_dir.go +++ b/cmd/workspace/workspace/import_dir.go @@ -134,8 +134,7 @@ Notebooks will have their extensions (one of .scala, .py, .sql, .ipynb, .r) stri return err } - // TODO: print progress events on stderr instead: https://github.com/databricks/cli/issues/448 - err = cmdio.RenderJson(ctx, newImportStartedEvent(opts.sourceDir)) + err = cmdio.RenderWithTemplate(ctx, newImportStartedEvent(opts.sourceDir), "", "Importing files from {{.SourcePath}}") if err != nil { return err } @@ -145,7 +144,7 @@ Notebooks will have their extensions (one of .scala, .py, .sql, .ipynb, .r) stri if err != nil { return err } - return cmdio.RenderJson(ctx, newImportCompletedEvent(opts.targetDir)) + return cmdio.RenderWithTemplate(ctx, newImportCompletedEvent(opts.targetDir), "", "Import complete. The files are available at {{.TargetPath}}\n") } return cmd From ae312d604bd7c62315a41bea11d69a7b032ba5b6 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 15 Aug 2024 11:49:18 +0200 Subject: [PATCH 2/6] new lines --- cmd/workspace/workspace/export_dir.go | 2 +- cmd/workspace/workspace/import_dir.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/workspace/workspace/export_dir.go b/cmd/workspace/workspace/export_dir.go index 7a3ec9a700..566c5dafaa 100644 --- a/cmd/workspace/workspace/export_dir.go +++ b/cmd/workspace/workspace/export_dir.go @@ -110,7 +110,7 @@ func newExportDir() *cobra.Command { } workspaceFS := filer.NewFS(ctx, workspaceFiler) - err = cmdio.RenderWithTemplate(ctx, newExportStartedEvent(opts.sourceDir), "", "Exporting files from {{.SourcePath}}") + err = cmdio.RenderWithTemplate(ctx, newExportStartedEvent(opts.sourceDir), "", "Exporting files from {{.SourcePath}}\n") if err != nil { return err } diff --git a/cmd/workspace/workspace/import_dir.go b/cmd/workspace/workspace/import_dir.go index c68c4d6128..f93cf0b239 100644 --- a/cmd/workspace/workspace/import_dir.go +++ b/cmd/workspace/workspace/import_dir.go @@ -134,7 +134,7 @@ Notebooks will have their extensions (one of .scala, .py, .sql, .ipynb, .r) stri return err } - err = cmdio.RenderWithTemplate(ctx, newImportStartedEvent(opts.sourceDir), "", "Importing files from {{.SourcePath}}") + err = cmdio.RenderWithTemplate(ctx, newImportStartedEvent(opts.sourceDir), "", "Importing files from {{.SourcePath}}\n") if err != nil { return err } From 8adb2dce2772264f106bdd3bfe8bd1a033928ff6 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 15 Aug 2024 11:56:11 +0200 Subject: [PATCH 3/6] remove unused method --- libs/cmdio/render.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/libs/cmdio/render.go b/libs/cmdio/render.go index ec851b8fff..4114db5caf 100644 --- a/libs/cmdio/render.go +++ b/libs/cmdio/render.go @@ -280,14 +280,6 @@ func RenderIteratorWithTemplate[T any](ctx context.Context, i listing.Iterator[T return renderWithTemplate(newIteratorRenderer(i), ctx, c.outputFormat, c.out, headerTemplate, template) } -func RenderJson(ctx context.Context, v any) error { - c := fromContext(ctx) - if _, ok := v.(listingInterface); ok { - panic("use RenderIteratorJson instead") - } - return renderWithTemplate(newRenderer(v), ctx, flags.OutputJSON, c.out, c.headerTemplate, c.template) -} - func RenderIteratorJson[T any](ctx context.Context, i listing.Iterator[T]) error { c := fromContext(ctx) return renderWithTemplate(newIteratorRenderer(i), ctx, c.outputFormat, c.out, c.headerTemplate, c.template) From 280573bccb8589d8f1b028404372ef5bdd8de296 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 15 Aug 2024 11:58:36 +0200 Subject: [PATCH 4/6] fix --- cmd/workspace/workspace/export_dir.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/workspace/workspace/export_dir.go b/cmd/workspace/workspace/export_dir.go index 566c5dafaa..9a7ac6e908 100644 --- a/cmd/workspace/workspace/export_dir.go +++ b/cmd/workspace/workspace/export_dir.go @@ -119,7 +119,7 @@ func newExportDir() *cobra.Command { if err != nil { return err } - return cmdio.RenderWithTemplate(ctx, newImportCompletedEvent(opts.targetDir), "", "Exported complete. The files are available at {{.TargetPath}}\n") + return cmdio.RenderWithTemplate(ctx, newExportCompletedEvent(opts.targetDir), "", "Exported complete. The files are available at {{.TargetPath}}\n") } return cmd From 3e98c2bdc0ab187be87c856cdbcc8fa84f526c49 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 15 Aug 2024 13:13:00 +0200 Subject: [PATCH 5/6] address comment --- cmd/workspace/workspace/export_dir.go | 2 +- cmd/workspace/workspace/import_dir.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/workspace/workspace/export_dir.go b/cmd/workspace/workspace/export_dir.go index 9a7ac6e908..ee06bfaf8f 100644 --- a/cmd/workspace/workspace/export_dir.go +++ b/cmd/workspace/workspace/export_dir.go @@ -119,7 +119,7 @@ func newExportDir() *cobra.Command { if err != nil { return err } - return cmdio.RenderWithTemplate(ctx, newExportCompletedEvent(opts.targetDir), "", "Exported complete. The files are available at {{.TargetPath}}\n") + return cmdio.RenderWithTemplate(ctx, newExportCompletedEvent(opts.targetDir), "", "Exported complete\n") } return cmd diff --git a/cmd/workspace/workspace/import_dir.go b/cmd/workspace/workspace/import_dir.go index f93cf0b239..a197d7dd9f 100644 --- a/cmd/workspace/workspace/import_dir.go +++ b/cmd/workspace/workspace/import_dir.go @@ -144,7 +144,7 @@ Notebooks will have their extensions (one of .scala, .py, .sql, .ipynb, .r) stri if err != nil { return err } - return cmdio.RenderWithTemplate(ctx, newImportCompletedEvent(opts.targetDir), "", "Import complete. The files are available at {{.TargetPath}}\n") + return cmdio.RenderWithTemplate(ctx, newImportCompletedEvent(opts.targetDir), "", "Import complete\n") } return cmd From dd4cb013cfb478d0fcff4b9f53c1c21c49813fa8 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 15 Aug 2024 13:40:40 +0200 Subject: [PATCH 6/6] add regression test --- cmd/workspace/workspace/export_dir.go | 2 +- internal/workspace_test.go | 53 ++++++++++++++++++--------- 2 files changed, 36 insertions(+), 19 deletions(-) diff --git a/cmd/workspace/workspace/export_dir.go b/cmd/workspace/workspace/export_dir.go index ee06bfaf8f..0046f46ef9 100644 --- a/cmd/workspace/workspace/export_dir.go +++ b/cmd/workspace/workspace/export_dir.go @@ -119,7 +119,7 @@ func newExportDir() *cobra.Command { if err != nil { return err } - return cmdio.RenderWithTemplate(ctx, newExportCompletedEvent(opts.targetDir), "", "Exported complete\n") + return cmdio.RenderWithTemplate(ctx, newExportCompletedEvent(opts.targetDir), "", "Export complete\n") } return cmd diff --git a/internal/workspace_test.go b/internal/workspace_test.go index bc354914fe..4453616545 100644 --- a/internal/workspace_test.go +++ b/internal/workspace_test.go @@ -3,18 +3,17 @@ package internal import ( "context" "encoding/base64" - "errors" + "fmt" "io" - "net/http" "os" "path" "path/filepath" "strings" "testing" + "github.com/databricks/cli/internal/acc" "github.com/databricks/cli/libs/filer" "github.com/databricks/databricks-sdk-go" - "github.com/databricks/databricks-sdk-go/apierr" "github.com/databricks/databricks-sdk-go/service/workspace" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -63,21 +62,12 @@ func TestAccWorkpaceExportPrintsContents(t *testing.T) { } func setupWorkspaceImportExportTest(t *testing.T) (context.Context, filer.Filer, string) { - t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) + ctx, wt := acc.WorkspaceTest(t) - ctx := context.Background() - w := databricks.Must(databricks.NewWorkspaceClient()) - tmpdir := TemporaryWorkspaceDir(t, w) - f, err := filer.NewWorkspaceFilesClient(w, tmpdir) + tmpdir := TemporaryWorkspaceDir(t, wt.W) + f, err := filer.NewWorkspaceFilesClient(wt.W, tmpdir) require.NoError(t, err) - // Check if we can use this API here, skip test if we cannot. - _, err = f.Read(ctx, "we_use_this_call_to_test_if_this_api_is_enabled") - var aerr *apierr.APIError - if errors.As(err, &aerr) && aerr.StatusCode == http.StatusBadRequest { - t.Skip(aerr.Message) - } - return ctx, f, tmpdir } @@ -122,8 +112,21 @@ func TestAccExportDir(t *testing.T) { err = f.Write(ctx, "a/b/c/file-b", strings.NewReader("def"), filer.CreateParentDirectories) require.NoError(t, err) + expectedLogs := strings.Join([]string{ + fmt.Sprintf("Exporting files from %s", sourceDir), + fmt.Sprintf("%s -> %s", path.Join(sourceDir, "a/b/c/file-b"), filepath.Join(targetDir, "a/b/c/file-b")), + fmt.Sprintf("%s -> %s", path.Join(sourceDir, "file-a"), filepath.Join(targetDir, "file-a")), + fmt.Sprintf("%s -> %s", path.Join(sourceDir, "pyNotebook"), filepath.Join(targetDir, "pyNotebook.py")), + fmt.Sprintf("%s -> %s", path.Join(sourceDir, "rNotebook"), filepath.Join(targetDir, "rNotebook.r")), + fmt.Sprintf("%s -> %s", path.Join(sourceDir, "scalaNotebook"), filepath.Join(targetDir, "scalaNotebook.scala")), + fmt.Sprintf("%s -> %s", path.Join(sourceDir, "sqlNotebook"), filepath.Join(targetDir, "sqlNotebook.sql")), + "Export complete\n", + }, "\n") + // Run Export - RequireSuccessfulRun(t, "workspace", "export-dir", sourceDir, targetDir) + stdout, stderr := RequireSuccessfulRun(t, "workspace", "export-dir", sourceDir, targetDir) + assert.Equal(t, expectedLogs, stdout.String()) + assert.Equal(t, "", stderr.String()) // Assert files were exported assertLocalFileContents(t, filepath.Join(targetDir, "file-a"), "abc") @@ -176,10 +179,24 @@ func TestAccExportDirWithOverwriteFlag(t *testing.T) { assertLocalFileContents(t, filepath.Join(targetDir, "file-a"), "content from workspace") } -// TODO: Add assertions on progress logs for workspace import-dir command. https://github.com/databricks/cli/issues/455 func TestAccImportDir(t *testing.T) { ctx, workspaceFiler, targetDir := setupWorkspaceImportExportTest(t) - RequireSuccessfulRun(t, "workspace", "import-dir", "./testdata/import_dir", targetDir, "--log-level=debug") + stdout, stderr := RequireSuccessfulRun(t, "workspace", "import-dir", "./testdata/import_dir", targetDir, "--log-level=debug") + + expectedLogs := strings.Join([]string{ + fmt.Sprintf("Importing files from %s", "./testdata/import_dir"), + fmt.Sprintf("%s -> %s", filepath.FromSlash("a/b/c/file-b"), path.Join(targetDir, "a/b/c/file-b")), + fmt.Sprintf("%s -> %s", filepath.FromSlash("file-a"), path.Join(targetDir, "file-a")), + fmt.Sprintf("%s -> %s", filepath.FromSlash("jupyterNotebook.ipynb"), path.Join(targetDir, "jupyterNotebook")), + fmt.Sprintf("%s -> %s", filepath.FromSlash("pyNotebook.py"), path.Join(targetDir, "pyNotebook")), + fmt.Sprintf("%s -> %s", filepath.FromSlash("rNotebook.r"), path.Join(targetDir, "rNotebook")), + fmt.Sprintf("%s -> %s", filepath.FromSlash("scalaNotebook.scala"), path.Join(targetDir, "scalaNotebook")), + fmt.Sprintf("%s -> %s", filepath.FromSlash("sqlNotebook.sql"), path.Join(targetDir, "sqlNotebook")), + "Import complete\n", + }, "\n") + + assert.Equal(t, expectedLogs, stdout.String()) + assert.Equal(t, "", stderr.String()) // Assert files are imported assertFilerFileContents(t, ctx, workspaceFiler, "file-a", "hello, world")