Skip to content
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

feat: --format=json #7

Merged
merged 5 commits into from
Jan 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added auth0
Binary file not shown.
14 changes: 12 additions & 2 deletions internal/cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"io/ioutil"
"os"
"path"
"strings"
"sync"

"github.com/auth0/auth0-cli/internal/display"
Expand Down Expand Up @@ -33,6 +34,7 @@ type cli struct {

verbose bool
tenant string
format string
Copy link
Contributor Author

@jfatta jfatta Jan 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to use display.OutputFormat here directly (instead of string) but I didn't know how to make it work case insensitive


initOnce sync.Once
path string
Expand Down Expand Up @@ -96,9 +98,17 @@ func (c *cli) init() error {
c.tenant = c.data.DefaultTenant
}

format := strings.ToLower(c.format)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

json, JSON it should work for the user

if format != "" && format != string(display.OutputFormatJSON) {
err = fmt.Errorf("Invalid format. Use `--format=json` or omit this option to use the default format.")
return
}

c.renderer = &display.Renderer{
Tenant: c.tenant,
Writer: os.Stdout,
Tenant: c.tenant,
MessageWriter: os.Stderr,
ResultWriter: os.Stdout,
Comment on lines +109 to +110
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

intermediate messages use Stderr to keep the stdout clean for the final result (json or not), this way the output can be piped to another command (like jq)

Format: display.OutputFormat(format),
}
})

Expand Down
5 changes: 4 additions & 1 deletion internal/cli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,14 @@ func Execute() {

rootCmd.SetUsageTemplate(namespaceUsageTemplate())
rootCmd.PersistentFlags().StringVar(&cli.tenant,
"tenant", "", "Specific tenant to use")
"tenant", "", "Specific tenant to use.")

rootCmd.PersistentFlags().BoolVar(&cli.verbose,
"verbose", false, "Enable verbose mode.")

rootCmd.PersistentFlags().StringVar(&cli.format,
"format", "", "Command output format. Options: json.")

rootCmd.AddCommand(clientsCmd(cli))
rootCmd.AddCommand(logsCmd(cli))

Expand Down
28 changes: 22 additions & 6 deletions internal/display/clients.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,37 @@ import (
"gopkg.in/auth0.v5/management"
)

type clientView struct {
Name string
Type string
ClientID string
}

func (v *clientView) AsTableHeader() []string {
return []string{"Name", "Type", "ClientID"}
}

func (v *clientView) AsTableRow() []string {
return []string{v.Name, v.Type, ansi.Faint(v.ClientID)}
}

func (r *Renderer) ClientList(clients []*management.Client) {
r.Heading(ansi.Bold(r.Tenant), "clients\n")

var rows [][]string
var res []View
for _, c := range clients {
if auth0.StringValue(c.Name) == deprecatedAppName {
continue
}
rows = append(rows, []string{
auth0.StringValue(c.Name),
appTypeFor(c.AppType),
ansi.Faint(auth0.StringValue(c.ClientID)),
res = append(res, &clientView{
Name: auth0.StringValue(c.Name),
Type: appTypeFor(c.AppType),
ClientID: auth0.StringValue(c.ClientID),
})
Comment on lines +31 to 35
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if for JSON we just dumped the whole object? let users do whatever filtering/processing/formatting they need with jq or similar? There's a lot of info there it could be really useful to have (https://github.com/go-auth0/auth0/blob/master/management/client.go#L3)

Copy link
Contributor Author

@jfatta jfatta Jan 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of having --full or --deep mode. not completely sure if the format should also imply the level of details. As a user, I would expect the same columns and transformations (like appTypeFor()) instead of having the internals exposed, makes sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imagine if this command includes everything available here 😅: https://docs.github.com/en/rest/reference/pulls#list-pull-request

❯ gh pr list --state closed

Showing 17 of 17 pull requests in auth0/pkg that match your search

#17  This bumps go-instrumentation to 0.3.2 which bumps sentry to 0.0.9                  sentry-v0.0.9-bump
#16  Bumping lightstep tracer to 0.23.0                                                  bump-lightstep-tracer-to-0.23.0
#15  loggertest: ExpectLogged now takes in a proto.Message                               change-expect-log-to-use-proto
#14  Log the Addr to which the http server binds                                         log-bound-addr
#13  Bump datadag to 4.2.0                                                               bump-datadog-to-4-2-0
#12  Add error reporter via sentry                                                       add-error-reporter-via-sentry
#11  Add secretbox with aes+gcm implementation                                           add-crypto-aes-gcm-package
#10  Setup CI with Jenkins                                                               jenkins
#9   Update version of statds library, with fixed typo for With{out}AggregationInterval  ja30278:master
#8   Use auth0/go-instrumentation for logging                                            go-instrumentation
#7   Support defining a namespace for service metrics                                    support-metric-namespace
#6   Add Timing to metrics interface                                                     add-timing-to-interface
#5   Add Timing to metrics                                                               timing
#4   Adds lightstep tracer                                                               add-tracer
#3   enable client side aggregation                                                      use-client-side-aggregation
#2   service: extract standard service concept                                           extract-service-concept
#1   Add github actions                                                                  add-github-actions-ci

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for text mode it makes sense to curate what the user sees, but for JSON mode it's likely just being piped to something else and the full object probably makes more sense IMO.

But in any case, absolutely not a blocker and can be discussed (bikeshedded 😆) later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is a pretty valid point, eventually we'll need to show more details. we can go back to this as we move on with the commands 🙇


}
r.Table([]string{"Name", "Type", "ClientID"}, rows)

r.Results(res)
}

// TODO(cyx): determine if there's a better way to filter this out.
Expand Down
67 changes: 55 additions & 12 deletions internal/display/display.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package display

import (
"encoding/json"
"fmt"
"io"
"os"
Expand All @@ -13,48 +14,90 @@ import (
"github.com/olekukonko/tablewriter"
)

type OutputFormat string

const (
OutputFormatJSON OutputFormat = "json"
)

type Renderer struct {
Tenant string

Writer io.Writer
// MessageWriter receives the renderer messages (typically os.Stderr)
MessageWriter io.Writer

// ResultWriter writes the final result of the commands (typically os.Stdout which can be piped to other commands)
ResultWriter io.Writer

// Format indicates how the results are rendered. Default (empty) will write as table
Format OutputFormat

initOnce sync.Once
}

func (r *Renderer) init() {
r.initOnce.Do(func() {
if r.Writer == nil {
r.Writer = os.Stdout
if r.MessageWriter == nil {
r.MessageWriter = os.Stderr
}
if r.ResultWriter == nil {
r.ResultWriter = os.Stdout
}
})
}

func (r *Renderer) Infof(format string, a ...interface{}) {
r.init()

fmt.Fprint(r.Writer, aurora.Green(" ▸ "))
fmt.Fprintf(r.Writer, format+"\n", a...)
fmt.Fprint(r.MessageWriter, aurora.Green(" ▸ "))
fmt.Fprintf(r.MessageWriter, format+"\n", a...)
}

func (r *Renderer) Warnf(format string, a ...interface{}) {
r.init()

fmt.Fprint(r.Writer, aurora.Yellow(" ▸ "))
fmt.Fprintf(r.Writer, format+"\n", a...)
fmt.Fprint(r.MessageWriter, aurora.Yellow(" ▸ "))
fmt.Fprintf(r.MessageWriter, format+"\n", a...)
}

func (r *Renderer) Errorf(format string, a ...interface{}) {
r.init()

fmt.Fprint(r.Writer, aurora.BrightRed(" ▸ "))
fmt.Fprintf(r.Writer, format+"\n", a...)
fmt.Fprint(r.MessageWriter, aurora.BrightRed(" ▸ "))
fmt.Fprintf(r.MessageWriter, format+"\n", a...)
}

func (r *Renderer) Heading(text ...string) {
fmt.Fprintf(r.Writer, "%s %s\n", ansi.Faint("==="), strings.Join(text, " "))
fmt.Fprintf(r.MessageWriter, "%s %s\n", ansi.Faint("==="), strings.Join(text, " "))
}

type View interface {
AsTableHeader() []string
AsTableRow() []string
}
Comment on lines +74 to +77
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I partially agree with @paddycarey regarding full results eventually. For the purposes of this PR, let's keep it as it is.

The only change I think we need to do in the future is maybe:

  1. Make View have a Raw interface{} which we can dump the entire SDK's response to.
  2. For the JSON format, we just use the Raw instead of the View directly.

Either way great progress!


func (r *Renderer) Results(data []View) {
if len(data) > 0 {
switch r.Format {
case OutputFormatJSON:
b, err := json.MarshalIndent(data, "", " ")
if err != nil {
r.Errorf("couldn't marshal results as JSON: %v", err)
return
}
fmt.Fprint(r.ResultWriter, string(b))

default:
rows := make([][]string, len(data))
for i, d := range data {
rows[i] = d.AsTableRow()
}
writeTable(r.ResultWriter, data[0].AsTableHeader(), rows)
}
}
}

func (r *Renderer) Table(header []string, data [][]string) {
func writeTable(w io.Writer, header []string, data [][]string) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private now in favor of .Result(). The cli setup will know when to render as table, or json, YAML, etc

tableString := &strings.Builder{}
table := tablewriter.NewWriter(tableString)
table.SetHeader(header)
Expand All @@ -74,7 +117,7 @@ func (r *Renderer) Table(header []string, data [][]string) {
}

table.Render()
fmt.Fprint(r.Writer, tableString.String())
fmt.Fprint(w, tableString.String())
}

func timeAgo(ts time.Time) string {
Expand Down
8 changes: 4 additions & 4 deletions internal/display/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func (r *Renderer) LogList(logs []*management.Log) {
}

fmt.Fprintf(
r.Writer,
r.ResultWriter,
"[%s] (%s) client_name=%q client_id=%q",
l.Date.Format(time.RFC3339),
logType,
Expand All @@ -35,7 +35,7 @@ func (r *Renderer) LogList(logs []*management.Log) {
userAgent, _ := reqMap["userAgent"].(string)
if userAgent != "" {
fmt.Fprintf(
r.Writer,
r.ResultWriter,
" user_agent=%q",
userAgent,
)
Expand All @@ -47,13 +47,13 @@ func (r *Renderer) LogList(logs []*management.Log) {
errType, _ := errMap["type"].(string)
if errType != "" || errMsg != "" {
fmt.Fprintf(
r.Writer,
r.ResultWriter,
" error_type=%q error_message=%q",
errType,
errMsg,
)
}

fmt.Fprint(r.Writer, "\n")
fmt.Fprint(r.ResultWriter, "\n")
}
}