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

pulsar-perf: improve error message for unrecognized options #18866

Closed
2 tasks done
pgier opened this issue Dec 10, 2022 · 2 comments · Fixed by #18889
Closed
2 tasks done

pulsar-perf: improve error message for unrecognized options #18866

pgier opened this issue Dec 10, 2022 · 2 comments · Fixed by #18889

Comments

@pgier
Copy link
Contributor

pgier commented Dec 10, 2022

Search before asking

  • I searched in the issues and found nothing similar.

Motivation

If I accidentally type a bad option when running the pulsar-perf command, I get an unhelpful error message.

> bin/pulsar-perf produce --foo mytopic
The size of topics list should be equal to --num-topic
Usage: pulsar-perf produce [options] persistent://prop/ns/my-topic
  Options:
    -am, --access-mode
      Producer access mode
      Default: Shared
...

I would expect pulsar-perf to provide an error that points out the bad option.

unrecognized option `--foo`

Solution

pulsar-perf should be able to determine if the user typed an option that is not valid.
I would think this would be handled automatically by the CLI library (jcommander in this case), but it seems to be interpreting unrecognized options as names of topics. If the CLI library can't handle this case correctly, pulsar should either switch to a different library, or add some additional error checking to see if an invalid option has been entered.

Alternatives

No response

Anything else?

No response

Are you willing to submit a PR?

  • I'm willing to submit a PR!
@pgier pgier changed the title Improve error message for unrecognized options pulsar-perf: Improve error message for unrecognized options Dec 10, 2022
@pgier pgier changed the title pulsar-perf: Improve error message for unrecognized options pulsar-perf: improve error message for unrecognized options Dec 10, 2022
@gaoran10
Copy link
Contributor

gaoran10 commented Dec 12, 2022

Thanks, this is a nice suggestion.

Maybe the problem is the topics parameter doesn't need a name such as -t, --topics, so the application can't distinguish whether it's an optional param or not.

For example, if we use this command, the result is the tool will publish messages to the topic public/default/--foo.

bin/pulsar-perf produce --foo

If we use this command, the tool wants to publish messages to topics public/default/--foo and public/default/mytopic, so it prints the tip, The size of topics list should be equal to --num-topic.

bin/pulsar-perf produce --foo mytopic

I think if we want to resolve this problem, we need to require the topics parameter to provide a param name, but this will change the using habit.

@pgier
Copy link
Contributor Author

pgier commented Dec 12, 2022

Yes, that nicely summarizes the issue. The issue is partly due to the fact that pulsar allows topic names which start with weird characters such as '-', and also JCommander is pretty limited in terms of validation on options vs. args.
I think a relatively easy solution is just to manually check for args that start with a '-', and then fail with an error in that case. I'll work on a PR for this.

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 a pull request may close this issue.

2 participants