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

rpk/api: Return back friendly errors #362

Merged
merged 1 commit into from
Jan 3, 2021

Conversation

c4milo
Copy link
Member

@c4milo c4milo commented Dec 31, 2020

when positional arguments are missing in all subcommands of rpk api. Instead of
Cobra's generic errors. Fixes half of #300.

Before

$ rpk api topic create
Error: accepts 1 arg(s), received 0

After

$ rpk api topic create
Error: topic's name is missing.

Usage:
  rpk api topic create <topic name> [flags]

Flags:
      --compact            Enable topic compaction
  -h, --help               help for create
  -p, --partitions int32   Number of partitions (default 1)
  -r, --replicas int16     Replication factor. If it's negative or is left unspecified, it will use the cluster's default topic replication factor. (default -1)

Global Flags:
      --brokers strings   Comma-separated list of broker ip:port pairs
      --config string     Redpanda config file, if not set the file will be searched for in the default locations
  -v, --verbose           enable verbose logging (default false)

@c4milo c4milo marked this pull request as draft December 31, 2020 19:14
@emaxerrno
Copy link
Contributor

@c4milo really great idea! thank you.

looks like some simple test strings need to be updated. waiting for the go mod patch to finish building and I'll merge that too.

when positional arguments are missing. Instead of
Cobra's generic errors.
@c4milo c4milo marked this pull request as ready for review January 3, 2021 20:11
@c4milo
Copy link
Member Author

c4milo commented Jan 3, 2021

@0x5d, @senior7515, this is ready for review. FWIW, I got caught up by topic_test.go trailing spaces, my IDE trims them by default and I had to restore them to get the tablewriter tests passing again 😬

Args: cobra.ExactArgs(1),
Args: exactArgs(1, "topic's name is missing."),
// We don't want Cobra printing CLI usage help if the error isn't about CLI usage.
SilenceUsage: true,
Copy link
Member Author

@c4milo c4milo Jan 3, 2021

Choose a reason for hiding this comment

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

Because we are using Cobra's RunE function, any errors returned from it will make Cobra to automatically print the Usage help, even if the error has nothing to do with CLI usage. We may want to use Cobra differently after GA, to have more flexibility on CLI UX.

@c4milo
Copy link
Member Author

c4milo commented Jan 3, 2021

@c4milo really great idea! thank you.

@senior7515, it was @rkruze idea :)

Copy link
Contributor

@emaxerrno emaxerrno left a comment

Choose a reason for hiding this comment

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

LGTM

@emaxerrno emaxerrno merged commit 99ccd6f into redpanda-data:dev Jan 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants