[GitHub] [ratis] szetszwo commented on a change in pull request #540: RATIS-1436.Add ratis-shell GroupInfo command

[GitBox <gi...@apache.org>]

szetszwo commented on a change in pull request #540:
URL: https://github.com/apache/ratis/pull/540#discussion_r761725391



##########
File path: ratis-shell/src/main/java/org/apache/ratis/shell/cli/sh/command/GroupCommand.java
##########
@@ -57,91 +58,45 @@ public String getCommandName() {
   }
 
   @Override
-  public int run(CommandLine cl) throws IOException {
-    super.run(cl);
-    final Map<RaftPeerId, InetSocketAddress> peersInfo = new HashMap<>();
-    final List<RaftPeerId> toRemove = getIds(cl.getOptionValues(REMOVE_OPTION_NAME), (a, b) -> {});
-    final List<RaftPeerId> toAdd = getIds(cl.getOptionValues(ADD_OPTION_NAME), peersInfo::put);
-    if (toRemove.isEmpty() && toAdd.isEmpty()) {
-      throw new IllegalArgumentException(String.format("Both -%s and -%s options are empty",
-          REMOVE_OPTION_NAME, ADD_OPTION_NAME));
-    }
-
-    try (RaftClient client = RaftUtils.createClient(getRaftGroup())) {
-      final Stream<RaftPeer> remaining = getRaftGroup().getPeers().stream()
-          .filter(raftPeer -> !toRemove.contains(raftPeer.getId()))
-          .filter(raftPeer -> !toAdd.contains(raftPeer.getId()));
-      final Stream<RaftPeer> adding = toAdd.stream().map(raftPeerId -> RaftPeer.newBuilder()
-          .setId(raftPeerId)
-          .setAddress(peersInfo.get(raftPeerId))
-          .setPriority(0)
-          .build());
-      final List<RaftPeer> peers = Stream.concat(remaining, adding).collect(Collectors.toList());
-      System.out.println("New peer list: " + peers);
-      RaftClientReply reply = client.admin().setConfiguration(peers);
-      processReply(reply, () -> "failed to change raft peer");
-    }
-    return 0;
-  }
+  public String getUsage() {
 
-  private static List<RaftPeerId> getIds(String[] optionValues, BiConsumer<RaftPeerId, InetSocketAddress> consumer) {
-    if (optionValues == null) {
-      return Collections.emptyList();
-    }
-    final List<RaftPeerId> ids = new ArrayList<>();
-    for (String address : optionValues) {
-      final String[] str = parse(address);
-      final InetSocketAddress serverAddress = InetSocketAddress.createUnresolved(str[0], Integer.parseInt(str[1]));
-      final RaftPeerId peerId = RaftUtils.getPeerId(serverAddress);
-      consumer.accept(peerId, serverAddress);
-      ids.add(peerId);
+    StringBuilder usage = new StringBuilder(getCommandName());
+    for (String cmd : subs.keySet()) {
+      usage.append(" [").append(cmd).append("]");
     }
-    return ids;
+    return usage.toString();
   }
 
-  private static String[] parse(String address) {
-    String[] str = address.split(":");
-    if(str.length < 2) {
-      throw new IllegalArgumentException("Failed to parse the address parameter \"" + address + "\".");
-    }
-    return str;
+  @Override
+  public String getDescription() {
+    return description();
   }
 
   @Override
-  public String getUsage() {
-    return String.format("%s"
-                    + " -%s <PEER0_HOST:PEER0_PORT,PEER1_HOST:PEER1_PORT,PEER2_HOST:PEER2_PORT>"
-                    + " [-%s <RAFT_GROUP_ID>]"
-                    + " -%s <PEER_HOST:PEER_PORT>"
-                    + " -%s <PEER_HOST:PEER_PORT>",
-            getCommandName(), PEER_OPTION_NAME, GROUPID_OPTION_NAME,
-            REMOVE_OPTION_NAME, ADD_OPTION_NAME);
+  public boolean hasSubCommand() {
+    return Optional.ofNullable(getSubCommands()).filter(sub -> !subs.isEmpty()).isPresent();
   }

Review comment:
       @codings-dan , We don't have to override hasSubCommand() anymore since the default will always work.  Please remove this.  Sorry that I did not see this last time.




-- 
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: issues-unsubscribe@ratis.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org