Skip to content

Commit

Permalink
Print diagnostics in 'bundle deploy' (#1579)
Browse files Browse the repository at this point in the history
## Changes
Print diagnostics in 'bundle deploy' similar to 'bundle validate'. This
way if a bundle has any errors or warnings, they are going to be easy to
notice.

NB: due to how we render errors, there is one extra trailing new line in
output, preserved in examples below

## Example: No errors or warnings

```
% databricks bundle deploy
Building default...
Deploying resources...
Updating deployment state...
Deployment complete!
```

## Example: Error on load

```
% databricks bundle deploy
Error: Databricks CLI version constraint not satisfied. Required: >= 1337.0.0, current: 0.0.0-dev

```

## Example: Warning on load

```
% databricks bundle deploy
Building default...
Deploying resources...
Updating deployment state...
Deployment complete!
Warning: unknown field: foo
  in databricks.yml:6:1

```

## Example: Error + warning on load

```
% databricks bundle deploy
Warning: unknown field: foo
  in databricks.yml:6:1

Error: something went wrong

```

## Example: Warning on load + error in init

```
% databricks bundle deploy
Warning: unknown field: foo
  in databricks.yml:6:1

Error: Failed to xxx
  in yyy.yml

Detailed explanation
in multiple lines

```

## Tests
Tested manually
  • Loading branch information
kanterov authored Jul 10, 2024
1 parent 1da04a4 commit af975ca
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 32 deletions.
19 changes: 14 additions & 5 deletions bundle/render/render_text_output.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func renderDiagnostics(out io.Writer, b *bundle.Bundle, diags diag.Diagnostics)
}

// Make file relative to bundle root
if d.Location.File != "" {
if d.Location.File != "" && b != nil {
out, err := filepath.Rel(b.RootPath, d.Location.File)
// if we can't relativize the path, just use path as-is
if err == nil {
Expand All @@ -160,16 +160,25 @@ func renderDiagnostics(out io.Writer, b *bundle.Bundle, diags diag.Diagnostics)
return nil
}

// RenderOptions contains options for rendering diagnostics.
type RenderOptions struct {
// variable to include leading new line

RenderSummaryTable bool
}

// RenderTextOutput renders the diagnostics in a human-readable format.
func RenderTextOutput(out io.Writer, b *bundle.Bundle, diags diag.Diagnostics) error {
func RenderTextOutput(out io.Writer, b *bundle.Bundle, diags diag.Diagnostics, opts RenderOptions) error {
err := renderDiagnostics(out, b, diags)
if err != nil {
return fmt.Errorf("failed to render diagnostics: %w", err)
}

err = renderSummaryTemplate(out, b, diags)
if err != nil {
return fmt.Errorf("failed to render summary: %w", err)
if opts.RenderSummaryTable {
err = renderSummaryTemplate(out, b, diags)
if err != nil {
return fmt.Errorf("failed to render summary: %w", err)
}
}

return nil
Expand Down
46 changes: 45 additions & 1 deletion bundle/render/render_text_output_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ type renderTestOutputTestCase struct {
name string
bundle *bundle.Bundle
diags diag.Diagnostics
opts RenderOptions
expected string
}

Expand All @@ -39,6 +40,7 @@ func TestRenderTextOutput(t *testing.T) {
Summary: "failed to load xxx",
},
},
opts: RenderOptions{RenderSummaryTable: true},
expected: "Error: failed to load xxx\n" +
"\n" +
"Found 1 error\n",
Expand All @@ -47,6 +49,7 @@ func TestRenderTextOutput(t *testing.T) {
name: "bundle during 'load' and 1 error",
bundle: loadingBundle,
diags: diag.Errorf("failed to load bundle"),
opts: RenderOptions{RenderSummaryTable: true},
expected: "Error: failed to load bundle\n" +
"\n" +
"Name: test-bundle\n" +
Expand All @@ -58,6 +61,7 @@ func TestRenderTextOutput(t *testing.T) {
name: "bundle during 'load' and 1 warning",
bundle: loadingBundle,
diags: diag.Warningf("failed to load bundle"),
opts: RenderOptions{RenderSummaryTable: true},
expected: "Warning: failed to load bundle\n" +
"\n" +
"Name: test-bundle\n" +
Expand All @@ -69,6 +73,7 @@ func TestRenderTextOutput(t *testing.T) {
name: "bundle during 'load' and 2 warnings",
bundle: loadingBundle,
diags: diag.Warningf("warning (1)").Extend(diag.Warningf("warning (2)")),
opts: RenderOptions{RenderSummaryTable: true},
expected: "Warning: warning (1)\n" +
"\n" +
"Warning: warning (2)\n" +
Expand Down Expand Up @@ -113,6 +118,7 @@ func TestRenderTextOutput(t *testing.T) {
},
},
},
opts: RenderOptions{RenderSummaryTable: true},
expected: "Error: error (1)\n" +
" in foo.py:1:1\n" +
"\n" +
Expand Down Expand Up @@ -153,6 +159,7 @@ func TestRenderTextOutput(t *testing.T) {
},
},
diags: nil,
opts: RenderOptions{RenderSummaryTable: true},
expected: "Name: test-bundle\n" +
"Target: test-target\n" +
"Workspace:\n" +
Expand All @@ -162,13 +169,50 @@ func TestRenderTextOutput(t *testing.T) {
"\n" +
"Validation OK!\n",
},
{
name: "nil bundle without summary with 1 error and 1 warning",
bundle: nil,
diags: diag.Diagnostics{
diag.Diagnostic{
Severity: diag.Error,
Summary: "error (1)",
Detail: "detail (1)",
Location: dyn.Location{
File: "foo.py",
Line: 1,
Column: 1,
},
},
diag.Diagnostic{
Severity: diag.Warning,
Summary: "warning (2)",
Detail: "detail (2)",
Location: dyn.Location{
File: "foo.py",
Line: 3,
Column: 1,
},
},
},
opts: RenderOptions{RenderSummaryTable: false},
expected: "Error: error (1)\n" +
" in foo.py:1:1\n" +
"\n" +
"detail (1)\n" +
"\n" +
"Warning: warning (2)\n" +
" in foo.py:3:1\n" +
"\n" +
"detail (2)\n" +
"\n",
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
writer := &bytes.Buffer{}

err := RenderTextOutput(writer, tc.bundle, tc.diags)
err := RenderTextOutput(writer, tc.bundle, tc.diags, tc.opts)
require.NoError(t, err)

assert.Equal(t, tc.expected, writer.String())
Expand Down
9 changes: 7 additions & 2 deletions bundle/scripts/scripts.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (m *script) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {

cmd, out, err := executeHook(ctx, executor, b, m.scriptHook)
if err != nil {
return diag.FromErr(err)
return diag.FromErr(fmt.Errorf("failed to execute script: %w", err))
}
if cmd == nil {
log.Debugf(ctx, "No script defined for %s, skipping", m.scriptHook)
Expand All @@ -53,7 +53,12 @@ func (m *script) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
line, err = reader.ReadString('\n')
}

return diag.FromErr(cmd.Wait())
err = cmd.Wait()
if err != nil {
return diag.FromErr(fmt.Errorf("failed to execute script: %w", err))
}

return nil
}

func executeHook(ctx context.Context, executor *exec.Executor, b *bundle.Bundle, hook config.ScriptHook) (exec.Command, io.Reader, error) {
Expand Down
57 changes: 34 additions & 23 deletions cmd/bundle/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package bundle

import (
"context"
"fmt"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/phases"
"github.com/databricks/cli/bundle/render"
"github.com/databricks/cli/cmd/bundle/utils"
"github.com/databricks/cli/cmd/root"
"github.com/databricks/cli/libs/diag"
Expand All @@ -30,32 +32,41 @@ func newDeployCommand() *cobra.Command {
cmd.RunE = func(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()
b, diags := utils.ConfigureBundleWithVariables(cmd)
if err := diags.Error(); err != nil {
return diags.Error()

if !diags.HasError() {
bundle.ApplyFunc(ctx, b, func(context.Context, *bundle.Bundle) diag.Diagnostics {
b.Config.Bundle.Force = force
b.Config.Bundle.Deployment.Lock.Force = forceLock
if cmd.Flag("compute-id").Changed {
b.Config.Bundle.ComputeID = computeID
}

if cmd.Flag("fail-on-active-runs").Changed {
b.Config.Bundle.Deployment.FailOnActiveRuns = failOnActiveRuns
}

return nil
})

diags = diags.Extend(
bundle.Apply(ctx, b, bundle.Seq(
phases.Initialize(),
phases.Build(),
phases.Deploy(),
)),
)
}

bundle.ApplyFunc(ctx, b, func(context.Context, *bundle.Bundle) diag.Diagnostics {
b.Config.Bundle.Force = force
b.Config.Bundle.Deployment.Lock.Force = forceLock
if cmd.Flag("compute-id").Changed {
b.Config.Bundle.ComputeID = computeID
}

if cmd.Flag("fail-on-active-runs").Changed {
b.Config.Bundle.Deployment.FailOnActiveRuns = failOnActiveRuns
}

return nil
})

diags = bundle.Apply(ctx, b, bundle.Seq(
phases.Initialize(),
phases.Build(),
phases.Deploy(),
))
if err := diags.Error(); err != nil {
return err
renderOpts := render.RenderOptions{RenderSummaryTable: false}
err := render.RenderTextOutput(cmd.OutOrStdout(), b, diags, renderOpts)
if err != nil {
return fmt.Errorf("failed to render output: %w", err)
}

if diags.HasError() {
return root.ErrAlreadyPrinted
}

return nil
}

Expand Down
3 changes: 2 additions & 1 deletion cmd/bundle/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ func newValidateCommand() *cobra.Command {

switch root.OutputType(cmd) {
case flags.OutputText:
err := render.RenderTextOutput(cmd.OutOrStdout(), b, diags)
renderOpts := render.RenderOptions{RenderSummaryTable: true}
err := render.RenderTextOutput(cmd.OutOrStdout(), b, diags, renderOpts)
if err != nil {
return fmt.Errorf("failed to render output: %w", err)
}
Expand Down

0 comments on commit af975ca

Please sign in to comment.