You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "crossoverJie (via GitHub)" <gi...@apache.org> on 2024/03/19 13:29:27 UTC
[PR] [improve][cli] PIP-343: Use picocli instead of jcommander in pulsar-perf [pulsar]
crossoverJie opened a new pull request, #22303:
URL: https://github.com/apache/pulsar/pull/22303
PIP: #22181
Improve CLI: `pulsar-perf` user experience.
### Motivation
<!-- Explain here the context, and why you're making that change. What is the problem you're trying to solve. -->
### Modifications
- Refactor the `pulsar-perf` code.
### Verifying this change
- [x] Make sure that the change passes the CI checks.
### Does this pull request potentially affect one of the following parts:
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
*If the box was checked, please highlight the changes*
- [ ] Dependencies (add or upgrade a dependency)
- [ ] The public API
- [ ] The schema
- [ ] The default values of configurations
- [ ] The threading model
- [ ] The binary protocol
- [ ] The REST endpoints
- [ ] The admin CLI options
- [ ] The metrics
- [ ] Anything that affects deployment
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `doc` <!-- Your PR contains doc changes. -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
### Matching PR in forked repository
PR in forked repository: https://github.com/crossoverJie/pulsar/pull/21
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [improve][cli] PIP-343: Use picocli instead of jcommander in pulsar-perf [pulsar]
Posted by "nodece (via GitHub)" <gi...@apache.org>.
nodece commented on PR #22303:
URL: https://github.com/apache/pulsar/pull/22303#issuecomment-2009556865
/pulsarbot rerun-failure-checks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [improve][cli] PIP-343: Use picocli instead of jcommander in pulsar-perf [pulsar]
Posted by "crossoverJie (via GitHub)" <gi...@apache.org>.
crossoverJie commented on code in PR #22303:
URL: https://github.com/apache/pulsar/pull/22303#discussion_r1531545778
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/CmdGenerateDocumentation.java:
##########
@@ -80,36 +81,36 @@ public static void main(String[] args) throws Exception {
Class<?> clazz = entry.getValue();
Constructor<?> constructor = clazz.getDeclaredConstructor();
constructor.setAccessible(true);
- jc.addCommand(cmd, constructor.newInstance());
+ commander.addSubcommand(cmd, constructor.newInstance());
}
if (arguments.commandNames.size() == 0) {
- for (Map.Entry<String, JCommander> cmd : jc.getCommands().entrySet()) {
- generateDocument(cmd.getKey(), jc);
+ for (Map.Entry<String, CommandLine> cmd : commander.getSubcommands().entrySet()) {
+ generateDocument(cmd.getKey(), commander);
}
} else {
for (String commandName : arguments.commandNames) {
- generateDocument(commandName, jc);
+ generateDocument(commandName, commander);
}
}
}
- private static String generateDocument(String module, JCommander parentCmd) {
+ private static String generateDocument(String module, CommandLine parentCmd) {
StringBuilder sb = new StringBuilder();
- JCommander cmd = parentCmd.getCommands().get(module);
+ CommandLine cmd = parentCmd.getSubcommands().get(module);
sb.append("## ").append(module).append("\n\n");
- sb.append(parentCmd.getUsageFormatter().getCommandDescription(module)).append("\n");
+ sb.append(cmd.getCommandName()).append("\n");
sb.append("\n\n```shell\n")
.append("$ pulsar-perf ").append(module).append(" [options]")
.append("\n```");
sb.append("\n\n");
sb.append("|Flag|Description|Default|\n");
sb.append("|---|---|---|\n");
- List<ParameterDescription> options = cmd.getParameters();
- options.stream().filter(ele -> !ele.getParameterAnnotation().hidden()).forEach((option) ->
- sb.append("| `").append(option.getNames())
- .append("` | ").append(option.getDescription().replace("\n", " "))
- .append("|").append(option.getDefault()).append("|\n")
+ List<CommandLine.Model.OptionSpec> options = cmd.getCommandSpec().options();
+ options.stream().filter(ele -> !ele.hidden()).forEach((option) ->
+ sb.append("| `").append(String.join(", ", option.names()))
+ .append("` | ").append(option.description()[0].replace("\n", " "))
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [improve][cli] PIP-343: Use picocli instead of jcommander in pulsar-perf [pulsar]
Posted by "crossoverJie (via GitHub)" <gi...@apache.org>.
crossoverJie commented on code in PR #22303:
URL: https://github.com/apache/pulsar/pull/22303#discussion_r1530656768
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/CmdGenerateDocumentation.java:
##########
@@ -80,36 +81,36 @@ public static void main(String[] args) throws Exception {
Class<?> clazz = entry.getValue();
Constructor<?> constructor = clazz.getDeclaredConstructor();
constructor.setAccessible(true);
- jc.addCommand(cmd, constructor.newInstance());
+ commander.addSubcommand(cmd, constructor.newInstance());
}
if (arguments.commandNames.size() == 0) {
- for (Map.Entry<String, JCommander> cmd : jc.getCommands().entrySet()) {
- generateDocument(cmd.getKey(), jc);
+ for (Map.Entry<String, CommandLine> cmd : commander.getSubcommands().entrySet()) {
+ generateDocument(cmd.getKey(), commander);
}
} else {
for (String commandName : arguments.commandNames) {
- generateDocument(commandName, jc);
+ generateDocument(commandName, commander);
}
}
}
- private static String generateDocument(String module, JCommander parentCmd) {
+ private static String generateDocument(String module, CommandLine parentCmd) {
StringBuilder sb = new StringBuilder();
- JCommander cmd = parentCmd.getCommands().get(module);
+ CommandLine cmd = parentCmd.getSubcommands().get(module);
sb.append("## ").append(module).append("\n\n");
- sb.append(parentCmd.getUsageFormatter().getCommandDescription(module)).append("\n");
+ sb.append(cmd.getCommandName()).append("\n");
Review Comment:
I can't seem to find a function that can get the command description, only a function that gets the command name.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [improve][cli] PIP-343: Use picocli instead of jcommander in pulsar-perf [pulsar]
Posted by "crossoverJie (via GitHub)" <gi...@apache.org>.
crossoverJie commented on code in PR #22303:
URL: https://github.com/apache/pulsar/pull/22303#discussion_r1530653033
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/BrokerMonitor.java:
##########
@@ -434,17 +434,17 @@ private synchronized void printBrokerData(final String broker, final LocalBroker
}
}
- // JCommander arguments class.
- @Parameters(commandDescription = "Monitors brokers and prints to the console information about their system "
+ // picocli arguments class.
+ @Command(description = "Monitors brokers and prints to the console information about their system "
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [improve][cli] PIP-343: Use picocli instead of jcommander in pulsar-perf [pulsar]
Posted by "crossoverJie (via GitHub)" <gi...@apache.org>.
crossoverJie commented on code in PR #22303:
URL: https://github.com/apache/pulsar/pull/22303#discussion_r1531545906
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/CmdGenerateDocumentation.java:
##########
@@ -80,36 +81,36 @@ public static void main(String[] args) throws Exception {
Class<?> clazz = entry.getValue();
Constructor<?> constructor = clazz.getDeclaredConstructor();
constructor.setAccessible(true);
- jc.addCommand(cmd, constructor.newInstance());
+ commander.addSubcommand(cmd, constructor.newInstance());
}
if (arguments.commandNames.size() == 0) {
- for (Map.Entry<String, JCommander> cmd : jc.getCommands().entrySet()) {
- generateDocument(cmd.getKey(), jc);
+ for (Map.Entry<String, CommandLine> cmd : commander.getSubcommands().entrySet()) {
+ generateDocument(cmd.getKey(), commander);
}
} else {
for (String commandName : arguments.commandNames) {
- generateDocument(commandName, jc);
+ generateDocument(commandName, commander);
}
}
}
- private static String generateDocument(String module, JCommander parentCmd) {
+ private static String generateDocument(String module, CommandLine parentCmd) {
StringBuilder sb = new StringBuilder();
- JCommander cmd = parentCmd.getCommands().get(module);
+ CommandLine cmd = parentCmd.getSubcommands().get(module);
sb.append("## ").append(module).append("\n\n");
- sb.append(parentCmd.getUsageFormatter().getCommandDescription(module)).append("\n");
+ sb.append(cmd.getCommandName()).append("\n");
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [improve][cli] PIP-343: Use picocli instead of jcommander in pulsar-perf [pulsar]
Posted by "nodece (via GitHub)" <gi...@apache.org>.
nodece commented on code in PR #22303:
URL: https://github.com/apache/pulsar/pull/22303#discussion_r1531511259
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/CmdGenerateDocumentation.java:
##########
@@ -80,36 +81,36 @@ public static void main(String[] args) throws Exception {
Class<?> clazz = entry.getValue();
Constructor<?> constructor = clazz.getDeclaredConstructor();
constructor.setAccessible(true);
- jc.addCommand(cmd, constructor.newInstance());
+ commander.addSubcommand(cmd, constructor.newInstance());
}
if (arguments.commandNames.size() == 0) {
- for (Map.Entry<String, JCommander> cmd : jc.getCommands().entrySet()) {
- generateDocument(cmd.getKey(), jc);
+ for (Map.Entry<String, CommandLine> cmd : commander.getSubcommands().entrySet()) {
+ generateDocument(cmd.getKey(), commander);
}
} else {
for (String commandName : arguments.commandNames) {
- generateDocument(commandName, jc);
+ generateDocument(commandName, commander);
}
}
}
- private static String generateDocument(String module, JCommander parentCmd) {
+ private static String generateDocument(String module, CommandLine parentCmd) {
StringBuilder sb = new StringBuilder();
- JCommander cmd = parentCmd.getCommands().get(module);
+ CommandLine cmd = parentCmd.getSubcommands().get(module);
sb.append("## ").append(module).append("\n\n");
- sb.append(parentCmd.getUsageFormatter().getCommandDescription(module)).append("\n");
+ sb.append(cmd.getCommandName()).append("\n");
sb.append("\n\n```shell\n")
.append("$ pulsar-perf ").append(module).append(" [options]")
.append("\n```");
sb.append("\n\n");
sb.append("|Flag|Description|Default|\n");
sb.append("|---|---|---|\n");
- List<ParameterDescription> options = cmd.getParameters();
- options.stream().filter(ele -> !ele.getParameterAnnotation().hidden()).forEach((option) ->
- sb.append("| `").append(option.getNames())
- .append("` | ").append(option.getDescription().replace("\n", " "))
- .append("|").append(option.getDefault()).append("|\n")
+ List<CommandLine.Model.OptionSpec> options = cmd.getCommandSpec().options();
+ options.stream().filter(ele -> !ele.hidden()).forEach((option) ->
+ sb.append("| `").append(String.join(", ", option.names()))
+ .append("` | ").append(option.description()[0].replace("\n", " "))
Review Comment:
Please check the description() length.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [improve][cli] PIP-343: Use picocli instead of jcommander in pulsar-perf [pulsar]
Posted by "crossoverJie (via GitHub)" <gi...@apache.org>.
crossoverJie commented on code in PR #22303:
URL: https://github.com/apache/pulsar/pull/22303#discussion_r1530653734
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/LoadSimulationClient.java:
##########
@@ -170,19 +170,19 @@ public void start() throws Exception {
}
}
- // JCommander arguments for starting a LoadSimulationClient.
- @Parameters(commandDescription = "Simulate client load by maintaining producers and consumers for topics.")
+ // picocli arguments for starting a LoadSimulationClient.
+ @Command(description = "Simulate client load by maintaining producers and consumers for topics.")
Review Comment:
Done.
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/CmdGenerateDocumentation.java:
##########
@@ -18,46 +18,47 @@
*/
package org.apache.pulsar.testclient;
-import com.beust.jcommander.JCommander;
-import com.beust.jcommander.Parameter;
-import com.beust.jcommander.ParameterDescription;
-import com.beust.jcommander.ParameterException;
-import com.beust.jcommander.Parameters;
import java.lang.reflect.Constructor;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import lombok.extern.slf4j.Slf4j;
+import picocli.CommandLine;
+import picocli.CommandLine.Command;
+import picocli.CommandLine.Option;
+import picocli.CommandLine.ParameterException;
@Slf4j
public class CmdGenerateDocumentation {
- @Parameters(commandDescription = "Generate documentation automatically.")
+ @Command(description = "Generate documentation automatically.")
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [improve][cli] PIP-343: Use picocli instead of jcommander in pulsar-perf [pulsar]
Posted by "nodece (via GitHub)" <gi...@apache.org>.
nodece commented on code in PR #22303:
URL: https://github.com/apache/pulsar/pull/22303#discussion_r1530449831
##########
pulsar-cli-utils/src/main/java/org/apache/pulsar/cli/converters/ByteUnitToLongConverter.java:
##########
@@ -18,22 +18,16 @@
*/
package org.apache.pulsar.cli.converters;
-import static org.apache.pulsar.cli.ValueValidationUtil.emptyCheck;
-import com.beust.jcommander.converters.BaseConverter;
+import picocli.CommandLine;
-public class ByteUnitToLongConverter extends BaseConverter<Long> {
-
- public ByteUnitToLongConverter(String optionName) {
- super(optionName);
- }
+public class ByteUnitToLongConverter implements CommandLine.ITypeConverter<Long> {
Review Comment:
You can remove this class, and use `org.apache.pulsar.cli.converters.picocli.ByteUnitToLongConverter` instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [improve][cli] PIP-343: Use picocli instead of jcommander in pulsar-perf [pulsar]
Posted by "Technoboy- (via GitHub)" <gi...@apache.org>.
Technoboy- merged PR #22303:
URL: https://github.com/apache/pulsar/pull/22303
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [improve][cli] PIP-343: Use picocli instead of jcommander in pulsar-perf [pulsar]
Posted by "crossoverJie (via GitHub)" <gi...@apache.org>.
crossoverJie commented on code in PR #22303:
URL: https://github.com/apache/pulsar/pull/22303#discussion_r1531419005
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/LoadSimulationController.java:
##########
@@ -82,50 +83,50 @@ public class LoadSimulationController {
private static final ExecutorService threadPool = Executors.newCachedThreadPool();
- // JCommander arguments for starting a controller via main.
- @Parameters(commandDescription = "Provides a shell for the user to dictate how simulation clients should "
+ // picocli arguments for starting a controller via main.
+ @Command(description = "Provides a shell for the user to dictate how simulation clients should "
+ "incur load.")
private static class MainArguments {
- @Parameter(names = { "-h", "--help" }, description = "Help message", help = true)
+ @Option(names = { "-h", "--help" }, description = "Help message", help = true)
boolean help;
- @Parameter(names = { "--cluster" }, description = "Cluster to test on", required = true)
+ @Option(names = { "--cluster" }, description = "Cluster to test on", required = true)
String cluster;
- @Parameter(names = { "--clients" }, description = "Comma separated list of client hostnames", required = true)
+ @Option(names = { "--clients" }, description = "Comma separated list of client hostnames", required = true)
String clientHostNames;
- @Parameter(names = { "--client-port" }, description = "Port that the clients are listening on", required = true)
+ @Option(names = { "--client-port" }, description = "Port that the clients are listening on", required = true)
int clientPort;
}
- // JCommander arguments for accepting user input.
+ // picocli arguments for accepting user input.
private static class ShellArguments {
- @Parameter(description = "Command arguments:\n" + "trade tenant namespace topic\n"
+ @Parameters(description = "Command arguments:\n" + "trade tenant namespace topic\n"
+ "change tenant namespace topic\n" + "stop tenant namespace topic\n"
+ "trade_group tenant group_name num_namespaces\n" + "change_group tenant group_name\n"
+ "stop_group tenant group_name\n" + "script script_name\n" + "copy tenant_name source_zk target_zk\n"
- + "stream source_zk\n" + "simulate zk\n", required = true)
+ + "stream source_zk\n" + "simulate zk\n")
Review Comment:
Add ` arity = "1"`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [improve][cli] PIP-343: Use picocli instead of jcommander in pulsar-perf [pulsar]
Posted by "nodece (via GitHub)" <gi...@apache.org>.
nodece commented on code in PR #22303:
URL: https://github.com/apache/pulsar/pull/22303#discussion_r1530455656
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/CmdGenerateDocumentation.java:
##########
@@ -80,36 +81,36 @@ public static void main(String[] args) throws Exception {
Class<?> clazz = entry.getValue();
Constructor<?> constructor = clazz.getDeclaredConstructor();
constructor.setAccessible(true);
- jc.addCommand(cmd, constructor.newInstance());
+ commander.addSubcommand(cmd, constructor.newInstance());
}
if (arguments.commandNames.size() == 0) {
- for (Map.Entry<String, JCommander> cmd : jc.getCommands().entrySet()) {
- generateDocument(cmd.getKey(), jc);
+ for (Map.Entry<String, CommandLine> cmd : commander.getSubcommands().entrySet()) {
+ generateDocument(cmd.getKey(), commander);
}
} else {
for (String commandName : arguments.commandNames) {
- generateDocument(commandName, jc);
+ generateDocument(commandName, commander);
}
}
}
- private static String generateDocument(String module, JCommander parentCmd) {
+ private static String generateDocument(String module, CommandLine parentCmd) {
StringBuilder sb = new StringBuilder();
- JCommander cmd = parentCmd.getCommands().get(module);
+ CommandLine cmd = parentCmd.getSubcommands().get(module);
sb.append("## ").append(module).append("\n\n");
- sb.append(parentCmd.getUsageFormatter().getCommandDescription(module)).append("\n");
+ sb.append(cmd.getCommandName()).append("\n");
Review Comment:
Should be command description, not command name.
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/CmdGenerateDocumentation.java:
##########
@@ -80,36 +81,36 @@ public static void main(String[] args) throws Exception {
Class<?> clazz = entry.getValue();
Constructor<?> constructor = clazz.getDeclaredConstructor();
constructor.setAccessible(true);
- jc.addCommand(cmd, constructor.newInstance());
+ commander.addSubcommand(cmd, constructor.newInstance());
}
if (arguments.commandNames.size() == 0) {
- for (Map.Entry<String, JCommander> cmd : jc.getCommands().entrySet()) {
- generateDocument(cmd.getKey(), jc);
+ for (Map.Entry<String, CommandLine> cmd : commander.getSubcommands().entrySet()) {
+ generateDocument(cmd.getKey(), commander);
}
} else {
for (String commandName : arguments.commandNames) {
- generateDocument(commandName, jc);
+ generateDocument(commandName, commander);
}
}
}
- private static String generateDocument(String module, JCommander parentCmd) {
+ private static String generateDocument(String module, CommandLine parentCmd) {
StringBuilder sb = new StringBuilder();
- JCommander cmd = parentCmd.getCommands().get(module);
+ CommandLine cmd = parentCmd.getSubcommands().get(module);
sb.append("## ").append(module).append("\n\n");
- sb.append(parentCmd.getUsageFormatter().getCommandDescription(module)).append("\n");
+ sb.append(cmd.getCommandName()).append("\n");
sb.append("\n\n```shell\n")
.append("$ pulsar-perf ").append(module).append(" [options]")
.append("\n```");
sb.append("\n\n");
sb.append("|Flag|Description|Default|\n");
sb.append("|---|---|---|\n");
- List<ParameterDescription> options = cmd.getParameters();
- options.stream().filter(ele -> !ele.getParameterAnnotation().hidden()).forEach((option) ->
- sb.append("| `").append(option.getNames())
- .append("` | ").append(option.getDescription().replace("\n", " "))
- .append("|").append(option.getDefault()).append("|\n")
+ List<CommandLine.Model.OptionSpec> options = cmd.getCommandSpec().options();
+ options.stream().filter(ele -> !ele.hidden()).forEach((option) ->
+ sb.append("| `").append(Arrays.toString(option.names()))
+ .append("` | ").append(option.description()[0].replace("\n", " "))
+ .append("|").append(option.defaultValue()).append("|\n")
Review Comment:
```suggestion
.append("|").append(option.defaultValueString()).append("|\n")
```
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/CmdGenerateDocumentation.java:
##########
@@ -80,36 +81,36 @@ public static void main(String[] args) throws Exception {
Class<?> clazz = entry.getValue();
Constructor<?> constructor = clazz.getDeclaredConstructor();
constructor.setAccessible(true);
- jc.addCommand(cmd, constructor.newInstance());
+ commander.addSubcommand(cmd, constructor.newInstance());
}
if (arguments.commandNames.size() == 0) {
- for (Map.Entry<String, JCommander> cmd : jc.getCommands().entrySet()) {
- generateDocument(cmd.getKey(), jc);
+ for (Map.Entry<String, CommandLine> cmd : commander.getSubcommands().entrySet()) {
+ generateDocument(cmd.getKey(), commander);
}
} else {
for (String commandName : arguments.commandNames) {
- generateDocument(commandName, jc);
+ generateDocument(commandName, commander);
}
}
}
- private static String generateDocument(String module, JCommander parentCmd) {
+ private static String generateDocument(String module, CommandLine parentCmd) {
StringBuilder sb = new StringBuilder();
- JCommander cmd = parentCmd.getCommands().get(module);
+ CommandLine cmd = parentCmd.getSubcommands().get(module);
sb.append("## ").append(module).append("\n\n");
- sb.append(parentCmd.getUsageFormatter().getCommandDescription(module)).append("\n");
+ sb.append(cmd.getCommandName()).append("\n");
sb.append("\n\n```shell\n")
.append("$ pulsar-perf ").append(module).append(" [options]")
.append("\n```");
sb.append("\n\n");
sb.append("|Flag|Description|Default|\n");
sb.append("|---|---|---|\n");
- List<ParameterDescription> options = cmd.getParameters();
- options.stream().filter(ele -> !ele.getParameterAnnotation().hidden()).forEach((option) ->
- sb.append("| `").append(option.getNames())
- .append("` | ").append(option.getDescription().replace("\n", " "))
- .append("|").append(option.getDefault()).append("|\n")
+ List<CommandLine.Model.OptionSpec> options = cmd.getCommandSpec().options();
+ options.stream().filter(ele -> !ele.hidden()).forEach((option) ->
+ sb.append("| `").append(Arrays.toString(option.names()))
Review Comment:
Use `String.join(", ", option.names())` instead of `Arrays.toString(option.names())`.
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/LoadSimulationClient.java:
##########
@@ -170,19 +170,19 @@ public void start() throws Exception {
}
}
- // JCommander arguments for starting a LoadSimulationClient.
- @Parameters(commandDescription = "Simulate client load by maintaining producers and consumers for topics.")
+ // picocli arguments for starting a LoadSimulationClient.
+ @Command(description = "Simulate client load by maintaining producers and consumers for topics.")
Review Comment:
Miss show default value, and scope.
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/PerformanceReader.java:
##########
@@ -59,30 +59,30 @@ public class PerformanceReader {
private static Recorder recorder = new Recorder(TimeUnit.DAYS.toMillis(10), 5);
private static Recorder cumulativeRecorder = new Recorder(TimeUnit.DAYS.toMillis(10), 5);
- @Parameters(commandDescription = "Test pulsar reader performance.")
+ @Command(description = "Test pulsar reader performance.")
Review Comment:
```suggestion
@Command(description = "Test pulsar reader performance.", showDefaultValues = true, scope = ScopeType.INHERIT)
```
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/LoadSimulationController.java:
##########
@@ -82,50 +83,50 @@ public class LoadSimulationController {
private static final ExecutorService threadPool = Executors.newCachedThreadPool();
- // JCommander arguments for starting a controller via main.
- @Parameters(commandDescription = "Provides a shell for the user to dictate how simulation clients should "
+ // picocli arguments for starting a controller via main.
+ @Command(description = "Provides a shell for the user to dictate how simulation clients should "
Review Comment:
Miss show default value, and scope.
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/PerformanceConsumer.java:
##########
@@ -82,105 +82,107 @@ public class PerformanceConsumer {
private static final Recorder recorder = new Recorder(MAX_LATENCY, 5);
private static final Recorder cumulativeRecorder = new Recorder(MAX_LATENCY, 5);
- @Parameters(commandDescription = "Test pulsar consumer performance.")
+ @Command(description = "Test pulsar consumer performance.")
Review Comment:
```suggestion
@Command(description = "Test pulsar consumer performance.", showDefaultValues = true, scope = ScopeType.INHERIT)
```
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/BrokerMonitor.java:
##########
@@ -434,17 +434,17 @@ private synchronized void printBrokerData(final String broker, final LocalBroker
}
}
- // JCommander arguments class.
- @Parameters(commandDescription = "Monitors brokers and prints to the console information about their system "
+ // picocli arguments class.
+ @Command(description = "Monitors brokers and prints to the console information about their system "
Review Comment:
Miss show default value, and scope.
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/PerformanceTransaction.java:
##########
@@ -89,84 +89,84 @@ public class PerformanceTransaction {
private static final Recorder messageSendRCumulativeRecorder =
new Recorder(TimeUnit.SECONDS.toMicros(120000), 5);
- @Parameters(commandDescription = "Test pulsar transaction performance.")
+ @Command(description = "Test pulsar transaction performance.")
Review Comment:
```suggestion
@Command(description = "Test pulsar transaction performance.", showDefaultValues = true, scope = ScopeType.INHERIT)
```
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/LoadSimulationController.java:
##########
@@ -82,50 +83,50 @@ public class LoadSimulationController {
private static final ExecutorService threadPool = Executors.newCachedThreadPool();
- // JCommander arguments for starting a controller via main.
- @Parameters(commandDescription = "Provides a shell for the user to dictate how simulation clients should "
+ // picocli arguments for starting a controller via main.
+ @Command(description = "Provides a shell for the user to dictate how simulation clients should "
+ "incur load.")
private static class MainArguments {
- @Parameter(names = { "-h", "--help" }, description = "Help message", help = true)
+ @Option(names = { "-h", "--help" }, description = "Help message", help = true)
boolean help;
- @Parameter(names = { "--cluster" }, description = "Cluster to test on", required = true)
+ @Option(names = { "--cluster" }, description = "Cluster to test on", required = true)
String cluster;
- @Parameter(names = { "--clients" }, description = "Comma separated list of client hostnames", required = true)
+ @Option(names = { "--clients" }, description = "Comma separated list of client hostnames", required = true)
String clientHostNames;
- @Parameter(names = { "--client-port" }, description = "Port that the clients are listening on", required = true)
+ @Option(names = { "--client-port" }, description = "Port that the clients are listening on", required = true)
int clientPort;
}
- // JCommander arguments for accepting user input.
+ // picocli arguments for accepting user input.
private static class ShellArguments {
- @Parameter(description = "Command arguments:\n" + "trade tenant namespace topic\n"
+ @Parameters(description = "Command arguments:\n" + "trade tenant namespace topic\n"
+ "change tenant namespace topic\n" + "stop tenant namespace topic\n"
+ "trade_group tenant group_name num_namespaces\n" + "change_group tenant group_name\n"
+ "stop_group tenant group_name\n" + "script script_name\n" + "copy tenant_name source_zk target_zk\n"
- + "stream source_zk\n" + "simulate zk\n", required = true)
+ + "stream source_zk\n" + "simulate zk\n")
Review Comment:
Miss `required`.
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/PositiveNumberParameterConvert.java:
##########
@@ -18,15 +18,15 @@
*/
package org.apache.pulsar.testclient;
-import com.beust.jcommander.IParameterValidator;
-import com.beust.jcommander.ParameterException;
-
-public class PositiveNumberParameterValidator implements IParameterValidator {
+import picocli.CommandLine;
+public class PositiveNumberParameterConvert implements CommandLine.ITypeConverter<Integer> {
@Override
- public void validate(String name, String value) throws ParameterException {
- if (Integer.parseInt(value) <= 0) {
- throw new ParameterException("Parameter " + name + " should be > 0 (found " + value + ")");
+ public Integer convert(String value) {
+ int result = Integer.parseInt(value);
+ if (result <= 0) {
+ throw new IllegalArgumentException("Parameter should be > 0 (found " + value + ")");
Review Comment:
```suggestion
throw new TypeConversionException("Parameter should be > 0 (found " + value + ")");
```
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/CmdGenerateDocumentation.java:
##########
@@ -18,46 +18,47 @@
*/
package org.apache.pulsar.testclient;
-import com.beust.jcommander.JCommander;
-import com.beust.jcommander.Parameter;
-import com.beust.jcommander.ParameterDescription;
-import com.beust.jcommander.ParameterException;
-import com.beust.jcommander.Parameters;
import java.lang.reflect.Constructor;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import lombok.extern.slf4j.Slf4j;
+import picocli.CommandLine;
+import picocli.CommandLine.Command;
+import picocli.CommandLine.Option;
+import picocli.CommandLine.ParameterException;
@Slf4j
public class CmdGenerateDocumentation {
- @Parameters(commandDescription = "Generate documentation automatically.")
+ @Command(description = "Generate documentation automatically.")
Review Comment:
Miss show default value, and scope.
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/ManagedLedgerWriter.java:
##########
@@ -78,61 +78,61 @@ public class ManagedLedgerWriter {
private static Recorder recorder = new Recorder(TimeUnit.SECONDS.toMillis(120000), 5);
private static Recorder cumulativeRecorder = new Recorder(TimeUnit.SECONDS.toMillis(120000), 5);
- @Parameters(commandDescription = "Write directly on managed-ledgers")
+ @Command(description = "Write directly on managed-ledgers")
Review Comment:
```suggestion
@Command(description = "Write directly on managed-ledgers", showDefaultValues = true, scope = ScopeType.INHERIT)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [improve][cli] PIP-343: Use picocli instead of jcommander in pulsar-perf [pulsar]
Posted by "crossoverJie (via GitHub)" <gi...@apache.org>.
crossoverJie commented on code in PR #22303:
URL: https://github.com/apache/pulsar/pull/22303#discussion_r1530653407
##########
pulsar-cli-utils/src/main/java/org/apache/pulsar/cli/converters/ByteUnitToLongConverter.java:
##########
@@ -18,22 +18,16 @@
*/
package org.apache.pulsar.cli.converters;
-import static org.apache.pulsar.cli.ValueValidationUtil.emptyCheck;
-import com.beust.jcommander.converters.BaseConverter;
+import picocli.CommandLine;
-public class ByteUnitToLongConverter extends BaseConverter<Long> {
-
- public ByteUnitToLongConverter(String optionName) {
- super(optionName);
- }
+public class ByteUnitToLongConverter implements CommandLine.ITypeConverter<Long> {
Review Comment:
Done.
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/CmdGenerateDocumentation.java:
##########
@@ -80,36 +81,36 @@ public static void main(String[] args) throws Exception {
Class<?> clazz = entry.getValue();
Constructor<?> constructor = clazz.getDeclaredConstructor();
constructor.setAccessible(true);
- jc.addCommand(cmd, constructor.newInstance());
+ commander.addSubcommand(cmd, constructor.newInstance());
}
if (arguments.commandNames.size() == 0) {
- for (Map.Entry<String, JCommander> cmd : jc.getCommands().entrySet()) {
- generateDocument(cmd.getKey(), jc);
+ for (Map.Entry<String, CommandLine> cmd : commander.getSubcommands().entrySet()) {
+ generateDocument(cmd.getKey(), commander);
}
} else {
for (String commandName : arguments.commandNames) {
- generateDocument(commandName, jc);
+ generateDocument(commandName, commander);
}
}
}
- private static String generateDocument(String module, JCommander parentCmd) {
+ private static String generateDocument(String module, CommandLine parentCmd) {
StringBuilder sb = new StringBuilder();
- JCommander cmd = parentCmd.getCommands().get(module);
+ CommandLine cmd = parentCmd.getSubcommands().get(module);
sb.append("## ").append(module).append("\n\n");
- sb.append(parentCmd.getUsageFormatter().getCommandDescription(module)).append("\n");
+ sb.append(cmd.getCommandName()).append("\n");
sb.append("\n\n```shell\n")
.append("$ pulsar-perf ").append(module).append(" [options]")
.append("\n```");
sb.append("\n\n");
sb.append("|Flag|Description|Default|\n");
sb.append("|---|---|---|\n");
- List<ParameterDescription> options = cmd.getParameters();
- options.stream().filter(ele -> !ele.getParameterAnnotation().hidden()).forEach((option) ->
- sb.append("| `").append(option.getNames())
- .append("` | ").append(option.getDescription().replace("\n", " "))
- .append("|").append(option.getDefault()).append("|\n")
+ List<CommandLine.Model.OptionSpec> options = cmd.getCommandSpec().options();
+ options.stream().filter(ele -> !ele.hidden()).forEach((option) ->
+ sb.append("| `").append(Arrays.toString(option.names()))
Review Comment:
Done.
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/LoadSimulationController.java:
##########
@@ -82,50 +83,50 @@ public class LoadSimulationController {
private static final ExecutorService threadPool = Executors.newCachedThreadPool();
- // JCommander arguments for starting a controller via main.
- @Parameters(commandDescription = "Provides a shell for the user to dictate how simulation clients should "
+ // picocli arguments for starting a controller via main.
+ @Command(description = "Provides a shell for the user to dictate how simulation clients should "
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [improve][cli] PIP-343: Use picocli instead of jcommander in pulsar-perf [pulsar]
Posted by "nodece (via GitHub)" <gi...@apache.org>.
nodece commented on code in PR #22303:
URL: https://github.com/apache/pulsar/pull/22303#discussion_r1531508942
##########
pulsar-testclient/src/main/java/org/apache/pulsar/testclient/CmdGenerateDocumentation.java:
##########
@@ -80,36 +81,36 @@ public static void main(String[] args) throws Exception {
Class<?> clazz = entry.getValue();
Constructor<?> constructor = clazz.getDeclaredConstructor();
constructor.setAccessible(true);
- jc.addCommand(cmd, constructor.newInstance());
+ commander.addSubcommand(cmd, constructor.newInstance());
}
if (arguments.commandNames.size() == 0) {
- for (Map.Entry<String, JCommander> cmd : jc.getCommands().entrySet()) {
- generateDocument(cmd.getKey(), jc);
+ for (Map.Entry<String, CommandLine> cmd : commander.getSubcommands().entrySet()) {
+ generateDocument(cmd.getKey(), commander);
}
} else {
for (String commandName : arguments.commandNames) {
- generateDocument(commandName, jc);
+ generateDocument(commandName, commander);
}
}
}
- private static String generateDocument(String module, JCommander parentCmd) {
+ private static String generateDocument(String module, CommandLine parentCmd) {
StringBuilder sb = new StringBuilder();
- JCommander cmd = parentCmd.getCommands().get(module);
+ CommandLine cmd = parentCmd.getSubcommands().get(module);
sb.append("## ").append(module).append("\n\n");
- sb.append(parentCmd.getUsageFormatter().getCommandDescription(module)).append("\n");
+ sb.append(cmd.getCommandName()).append("\n");
Review Comment:
public static String getCommandDescription(CommandLine commandLine) {
String[] description = commandLine.getCommandSpec().usageMessage().description();
if (description != null && description.length != 0) {
return description[0];
}
return "";
}
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [improve][cli] PIP-343: Use picocli instead of jcommander in pulsar-perf [pulsar]
Posted by "crossoverJie (via GitHub)" <gi...@apache.org>.
crossoverJie commented on PR #22303:
URL: https://github.com/apache/pulsar/pull/22303#issuecomment-2007183915
@nodece PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org