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.

Sign up for GitHub

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

Jump to bottom

Improve exception info for invaild time-related option #12828

Merged
merged 1 commit into from
Nov 17, 2021
Merged

Improve exception info for invaild time-related option #12828

merged 1 commit into from
Nov 17, 2021

Conversation

yuruguo
Copy link
Contributor

@yuruguo yuruguo commented Nov 16, 2021

Motivation

Check the validity of the time-related option in CLI tools and give a more friendly / concise prompt message when time invaild. Currently, it directly display the exception stack info as below:

bin/pulsar tokens create -sk file:///x/y/my-secret.key -s role -e 7z
Exception in thread "main" java.lang.IllegalArgumentException: Invalid time unit 'z'
	at org.apache.pulsar.common.util.RelativeTimeUtil.parseRelativeTimeInSeconds(RelativeTimeUtil.java:63)
	at org.apache.pulsar.utils.auth.tokens.TokensCliUtils$CommandCreateToken.run(TokensCliUtils.java:158)
	at org.apache.pulsar.utils.auth.tokens.TokensCliUtils.main(TokensCliUtils.java:327)

Modifications

  • Throw ParameterException when IllegalArgumentException occurs when executing RelativeTimeUtil.parseRelativeTimeInSeconds()

Documentation

Automatically generate doc through code

  • doc

@github-actions github-actions bot added the doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. label Nov 16, 2021
@yuruguo yuruguo changed the title Improve exception info for invaild time Improve exception info for invaild time-related option Nov 16, 2021
eolivelli
eolivelli reviewed Nov 16, 2021
View reviewed changes
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdNamespaces.java
try {
offloadAfterElapsed = TimeUnit.SECONDS.toMillis(
RelativeTimeUtil.parseRelativeTimeInSeconds(offloadAfterElapsedStr));
} catch (IllegalArgumentException exception) {
Copy link
Contributor

@eolivelli eolivelli Nov 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Why this case is different?

Copy link
Contributor Author

@yuruguo yuruguo Nov 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry, it is my mistake and I have corrected (PulsarAdminException -> ParameterException), PTAL again

eolivelli
eolivelli approved these changes Nov 16, 2021
View reviewed changes
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Lgtm

@codelipenghui codelipenghui added this to the 2.10.0 milestone Nov 17, 2021
@codelipenghui codelipenghui added release/2.9.1 doc-not-needed Your PR changes do not impact docs type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages and removed doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. labels Nov 17, 2021
@codelipenghui codelipenghui merged commit 60f5475 into apache:master Nov 17, 2021
dlg99 pushed a commit to dlg99/pulsar that referenced this pull request Nov 23, 2021
@yuruguo @dlg99
Improve exception info for invaild time-related option (apache#12828)
cdefa43
eolivelli pushed a commit to eolivelli/pulsar that referenced this pull request Nov 29, 2021
@yuruguo @eolivelli
Improve exception info for invaild time-related option (apache#12828)
c239d00
fxbing pushed a commit to fxbing/pulsar that referenced this pull request Dec 19, 2021
@yuruguo
Improve exception info for invaild time-related option (apache#12828)
0f21c8c
codelipenghui pushed a commit that referenced this pull request Dec 21, 2021
@yuruguo @codelipenghui
Improve exception info for invaild time-related option (#12828)
67d210d 
(cherry picked from commit 60f5475)
@codelipenghui codelipenghui added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codelipenghui codelipenghui codelipenghui approved these changes

@eolivelli eolivelli eolivelli approved these changes

Assignees

@yuruguo yuruguo

Labels
area/tool cherry-picked/branch-2.9 Archived: 2.9 is end of life doc-not-needed Your PR changes do not impact docs release/2.9.2 type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Milestone
2.10.0
Development

Successfully merging this pull request may close these issues.
4 participants
@yuruguo @eolivelli @codelipenghui @gaoran10