Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 215
Make bundle validation print text output by default#1335
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
740cfc696a2b39e1b12376ab9d107df82a355d9ef547d27f1927f27f2741b157e640c6File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -2,15 +2,129 @@ package bundle | ||
| import ( | ||
| "encoding/json" | ||
| "fmt" | ||
| "path/filepath" | ||
| "strings" | ||
| "text/template" | ||
| "github.com/databricks/cli/bundle" | ||
| "github.com/databricks/cli/bundle/phases" | ||
| "github.com/databricks/cli/cmd/bundle/utils" | ||
| "github.com/databricks/cli/cmd/root" | ||
| "github.com/databricks/cli/libs/log" | ||
| "github.com/databricks/cli/libs/diag" | ||
| "github.com/databricks/cli/libs/flags" | ||
| "github.com/fatih/color" | ||
| "github.com/spf13/cobra" | ||
| ) | ||
| var validateFuncMap = template.FuncMap{ | ||
| "red": color.RedString, | ||
| "green": color.GreenString, | ||
| "blue": color.BlueString, | ||
| "yellow": color.YellowString, | ||
| "magenta": color.MagentaString, | ||
| "cyan": color.CyanString, | ||
| "bold": func(format string, a ...interface{}) string { | ||
| return color.New(color.Bold).Sprintf(format, a...) | ||
| }, | ||
| "italic": func(format string, a ...interface{}) string { | ||
| return color.New(color.Italic).Sprintf(format, a...) | ||
| }, | ||
| } | ||
| const errorTemplate = `{{ "Error" | red }}: {{ .Summary }} | ||
| {{ "at " }}{{ .Path.String | green }} | ||
| {{ "in " }}{{ .Location.String | cyan }} | ||
| ` | ||
| const warningTemplate = `{{ "Warning" | yellow }}: {{ .Summary }} | ||
| {{ "at " }}{{ .Path.String | green }} | ||
| {{ "in " }}{{ .Location.String | cyan }} | ||
| ` | ||
| const summaryTemplate = `Name: {{ .Config.Bundle.Name | bold }} | ||
| Target: {{ .Config.Bundle.Target | bold }} | ||
| Workspace: | ||
| Host: {{ .Config.Workspace.Host | bold }} | ||
| User: {{ .Config.Workspace.CurrentUser.UserName | bold }} | ||
| Path: {{ .Config.Workspace.RootPath | bold }} | ||
| {{ .Trailer }} | ||
| ` | ||
| func pluralize(n int, singular, plural string) string { | ||
| if n == 1 { | ||
| return fmt.Sprintf("%d %s", n, singular) | ||
| } | ||
| return fmt.Sprintf("%d %s", n, plural) | ||
| } | ||
| func buildTrailer(diags diag.Diagnostics) string { | ||
| parts := []string{} | ||
| if errors := len(diags.Filter(diag.Error)); errors > 0 { | ||
| parts = append(parts, color.RedString(pluralize(errors, "error", "errors"))) | ||
| } | ||
| if warnings := len(diags.Filter(diag.Warning)); warnings > 0 { | ||
| parts = append(parts, color.YellowString(pluralize(warnings, "warning", "warnings"))) | ||
| } | ||
| if len(parts) > 0 { | ||
| return fmt.Sprintf("Found %s", strings.Join(parts, " and ")) | ||
| } else { | ||
| return color.GreenString("Validation OK!") | ||
| } | ||
| } | ||
| func renderTextOutput(cmd *cobra.Command, b *bundle.Bundle, diags diag.Diagnostics) error { | ||
| errorT := template.Must(template.New("error").Funcs(validateFuncMap).Parse(errorTemplate)) | ||
| warningT := template.Must(template.New("warning").Funcs(validateFuncMap).Parse(warningTemplate)) | ||
| // Print errors and warnings. | ||
| for _, d := range diags { | ||
| var t *template.Template | ||
| switch d.Severity { | ||
| case diag.Error: | ||
| t = errorT | ||
| case diag.Warning: | ||
| t = warningT | ||
| } | ||
| // Make file relative to bundle root | ||
| if d.Location.File != "" { | ||
| out, _ := filepath.Rel(b.RootPath, d.Location.File) | ||
| d.Location.File = out | ||
| } | ||
| // Render the diagnostic with the appropriate template. | ||
| err := t.Execute(cmd.OutOrStdout(), d) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
| // Print validation summary. | ||
| t := template.Must(template.New("summary").Funcs(validateFuncMap).Parse(summaryTemplate)) | ||
| err := t.Execute(cmd.OutOrStdout(), map[string]any{ | ||
| "Config": b.Config, | ||
| "Trailer": buildTrailer(diags), | ||
| }) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| return diags.Error() | ||
| } | ||
| func renderJsonOutput(cmd *cobra.Command, b *bundle.Bundle, diags diag.Diagnostics) error { | ||
| buf, err := json.MarshalIndent(b.Config, "", " ") | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we include diagnostics in the json output as well? ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, we should, but would like to think about the JSON-serialization of those before we do. We don't externalize the type yet, and doing so establishes the contract. | ||
| if err != nil { | ||
| return err | ||
| } | ||
| cmd.OutOrStdout().Write(buf) | ||
| return diags.Error() | ||
| } | ||
| func newValidateCommand() *cobra.Command { | ||
| cmd := &cobra.Command{ | ||
| Use: "validate", | ||
| @@ -25,23 +139,19 @@ func newValidateCommand() *cobra.Command { | ||
| return diags.Error() | ||
| } | ||
| diags = bundle.Apply(ctx, b, phases.Initialize()) | ||
| diags = diags.Extend(bundle.Apply(ctx, b, phases.Initialize())) | ||
| if err := diags.Error(); err != nil { | ||
| return err | ||
| } | ||
| // Until we change up the output of this command to be a text representation, | ||
| // we'll just output all diagnostics as debug logs. | ||
| for _, diag := range diags { | ||
| log.Debugf(cmd.Context(), "[%s]: %s", diag.Location, diag.Summary) | ||
| } | ||
| buf, err := json.MarshalIndent(b.Config, "", " ") | ||
| if err != nil { | ||
| return err | ||
| switch root.OutputType(cmd) { | ||
| case flags.OutputText: | ||
| return renderTextOutput(cmd, b, diags) | ||
| case flags.OutputJSON: | ||
| return renderJsonOutput(cmd, b, diags) | ||
| default: | ||
| return fmt.Errorf("unknown output type %s", root.OutputType(cmd)) | ||
| } | ||
| cmd.OutOrStdout().Write(buf) | ||
| return nil | ||
| } | ||
| return cmd | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a superset of this mapping in the
cmdiolibrary. Can you make that a public global and use that instead?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point -- will do in a follow-up. Wanted to use a subset here, hence the copy.