[CDTOOL-1699] Add Log Explorer and Insights CLI commands - #1887
[CDTOOL-1699] Add Log Explorer and Insights CLI commands#1887philippschulte wants to merge 1 commit into
Conversation
|
This pull request depends on fastly/go-fastly#851 and fastly/go-fastly#853. |
0aa1998 to
c910597
Compare
kpfleming
left a comment
kpfleming
left a comment
There was a problem hiding this comment.
Overall this looks good, but I've requested some changes for consistency with other commands in the CLI.
| Dst: &c.serviceName.Value, | ||
| }) | ||
| c.CmdClause.Flag("domain", "Limit data to the specified request domain").Action(c.domain.Set).StringVar(&c.domain.Value) | ||
| c.CmdClause.Flag("domain-exact-match", "Treat --domain as an exact match instead of a suffix match [true, false]").Action(c.domainExactMatch.Set).StringVar(&c.domainExactMatch.Value) |
There was a problem hiding this comment.
The CLI is not consistent in how it handles flags like this, but my preference would be for this to be a bare flag (no parameter provided), which sets the domain-exact-match behavior to true. Since the default is false, there's no need to provide a flag which can set it to false, since the only way it can ever be true is for a second (earlier) flag on the command line to have set it to true. If that change is made, then it can be a boolean in the struct instead of a string.
| }) | ||
| c.CmdClause.Flag("filter", "Filter in FIELD,OPERATOR,VALUE format (repeatable)").StringsVar(&c.filters) | ||
| c.CmdClause.Flag("limit", "Maximum number of rows to return (up to 100)").Action(c.limit.Set).IntVar(&c.limit.Value) | ||
| c.CmdClause.Flag("cursor", "Pagination cursor from a previous response").Action(c.cursor.Set).StringVar(&c.cursor.Value) |
There was a problem hiding this comment.
I don't think this is standard practice for commands in the CLI; pagination should be hidden from the user, either by absorbing all the pages in the CLI itself, or doing that in go-fastly. It's not important where the de-pagination occurs, but we shouldn't require CLI users to have to extract cursor values from the output and supply them to subsequent commands. In particular, the sequence of commands would have to have the same filter, start, and end parameters in order for the cursor to be valid. Since the command already offers start and end, the user can decide how much output they want.
This means we don't need limit or cursor in this command, since the user can get the desired behavior directly as long as the de-pagination is handled for them.
2 participants
Change summary
Adds
fastly service logging log-explorerandfastly service logging insightscommands with service scoping, filtering, pagination, visualization options, JSON/human-readable output, and test coverage.All Submissions:
New Feature Submissions: