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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
