ashishkumar50 commented on code in PR #5099:
URL: https://github.com/apache/ozone/pull/5099#discussion_r1271729775


##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/TopologySubcommand.java:
##########
@@ -180,16 +204,119 @@ private String getAdditionNodeOutput(HddsProtos.Node 
node) {
 
   // Format "ipAddress(hostName):PortName1=PortValue1    OperationalState
   //     networkLocation
-  private void printNodesWithLocation(Collection<HddsProtos.Node> nodes) {
-    nodes.forEach(node -> {
-      System.out.print(" " + getAdditionNodeOutput(node) +
-          node.getNodeID().getIpAddress() + "(" +
-          node.getNodeID().getHostName() + ")" +
-          ":" + formatPortOutput(node.getNodeID().getPortsList()));
-      System.out.println("    "
-          + node.getNodeOperationalStates(0) + "    " +
-          (node.getNodeID().getNetworkLocation() != null ?
-              node.getNodeID().getNetworkLocation() : "NA"));
-    });
+  private void printNodesWithLocation(Collection<HddsProtos.Node> nodes,
+                                      String state) throws IOException {
+    if (json) {

Review Comment:
   Please merge multiple if else.
   
   if (json && fullInfo) {.... return;}
   else if(json && !fullInfo {... return;}



##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/TopologySubcommand.java:
##########
@@ -180,16 +204,119 @@ private String getAdditionNodeOutput(HddsProtos.Node 
node) {
 
   // Format "ipAddress(hostName):PortName1=PortValue1    OperationalState
   //     networkLocation
-  private void printNodesWithLocation(Collection<HddsProtos.Node> nodes) {
-    nodes.forEach(node -> {
-      System.out.print(" " + getAdditionNodeOutput(node) +
-          node.getNodeID().getIpAddress() + "(" +
-          node.getNodeID().getHostName() + ")" +
-          ":" + formatPortOutput(node.getNodeID().getPortsList()));
-      System.out.println("    "
-          + node.getNodeOperationalStates(0) + "    " +
-          (node.getNodeID().getNetworkLocation() != null ?
-              node.getNodeID().getNetworkLocation() : "NA"));
-    });
+  private void printNodesWithLocation(Collection<HddsProtos.Node> nodes,
+                                      String state) throws IOException {
+    if (json) {
+      if (fullInfo) {
+        ArrayList<NodeTopologyFull> nodesJson = new ArrayList<>();
+        nodes.forEach(node -> {
+          NodeTopologyFull nodeJson =
+              new NodeTopologyFull(
+                  DatanodeDetails.getFromProtoBuf(node.getNodeID()), state);
+          nodesJson.add(nodeJson);
+        });
+        System.out.println(
+            JsonUtils.toJsonStringWithDefaultPrettyPrinter(nodesJson));
+      } else {
+        ArrayList<NodeTopologyDefault> nodesJson = new ArrayList<>();
+        nodes.forEach(node -> {
+          NodeTopologyDefault nodeJson = new NodeTopologyDefault(
+              DatanodeDetails.getFromProtoBuf(node.getNodeID()), state);
+          nodesJson.add(nodeJson);
+        });
+        System.out.println(
+            JsonUtils.toJsonStringWithDefaultPrettyPrinter(nodesJson));
+      }
+    } else {
+      // show node state
+      System.out.println("State = " + state);
+      nodes.forEach(node -> {
+        System.out.print(" " + getAdditionNodeOutput(node) +
+            node.getNodeID().getIpAddress() + "(" +
+            node.getNodeID().getHostName() + ")" +
+            ":" + formatPortOutput(node.getNodeID().getPortsList()));
+        System.out.println("    "
+            + node.getNodeOperationalStates(0) + "    " +
+            (node.getNodeID().getNetworkLocation() != null ?
+                node.getNodeID().getNetworkLocation() : "NA"));
+      });
+    }
+  }
+
+  private static class ListJsonSerializer extends
+      JsonSerializer<List<DatanodeDetails.Port>> {
+    @Override
+    public void serialize(List<DatanodeDetails.Port> value, JsonGenerator jgen,
+                          SerializerProvider provider)
+        throws IOException {
+      jgen.writeStartObject();
+      for (DatanodeDetails.Port port : value) {
+        jgen.writeNumberField(port.getName().toString(), port.getValue());
+      }
+      jgen.writeEndObject();
+    }
+  }
+
+  private static class NodeTopologyOrder {
+    private String ipAddress;
+    private String hostName;
+    private String nodeState;
+    private String operationalState;
+    private String networkLocation;
+
+    NodeTopologyOrder(DatanodeDetails node, String nState, String opState) {
+      ipAddress = node.getIpAddress();
+      hostName = node.getHostName();
+      nodeState = nState;
+      operationalState = opState;
+      networkLocation = node.getNetworkLocation();

Review Comment:
   Keep consistent as without json output
   ```
   networkLocation = (node.getNodeID().getNetworkLocation() != null ?
       node.getNodeID().getNetworkLocation() : "NA")
   ```



##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/TopologySubcommand.java:
##########
@@ -180,16 +204,119 @@ private String getAdditionNodeOutput(HddsProtos.Node 
node) {
 
   // Format "ipAddress(hostName):PortName1=PortValue1    OperationalState
   //     networkLocation
-  private void printNodesWithLocation(Collection<HddsProtos.Node> nodes) {
-    nodes.forEach(node -> {
-      System.out.print(" " + getAdditionNodeOutput(node) +
-          node.getNodeID().getIpAddress() + "(" +
-          node.getNodeID().getHostName() + ")" +
-          ":" + formatPortOutput(node.getNodeID().getPortsList()));
-      System.out.println("    "
-          + node.getNodeOperationalStates(0) + "    " +
-          (node.getNodeID().getNetworkLocation() != null ?
-              node.getNodeID().getNetworkLocation() : "NA"));
-    });
+  private void printNodesWithLocation(Collection<HddsProtos.Node> nodes,
+                                      String state) throws IOException {
+    if (json) {
+      if (fullInfo) {
+        ArrayList<NodeTopologyFull> nodesJson = new ArrayList<>();

Review Comment:
   ```suggestion
           List<NodeTopologyFull> nodesJson = new ArrayList<>();
   ```



##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/TopologySubcommand.java:
##########
@@ -180,16 +204,119 @@ private String getAdditionNodeOutput(HddsProtos.Node 
node) {
 
   // Format "ipAddress(hostName):PortName1=PortValue1    OperationalState
   //     networkLocation
-  private void printNodesWithLocation(Collection<HddsProtos.Node> nodes) {
-    nodes.forEach(node -> {
-      System.out.print(" " + getAdditionNodeOutput(node) +
-          node.getNodeID().getIpAddress() + "(" +
-          node.getNodeID().getHostName() + ")" +
-          ":" + formatPortOutput(node.getNodeID().getPortsList()));
-      System.out.println("    "
-          + node.getNodeOperationalStates(0) + "    " +
-          (node.getNodeID().getNetworkLocation() != null ?
-              node.getNodeID().getNetworkLocation() : "NA"));
-    });
+  private void printNodesWithLocation(Collection<HddsProtos.Node> nodes,
+                                      String state) throws IOException {
+    if (json) {
+      if (fullInfo) {
+        ArrayList<NodeTopologyFull> nodesJson = new ArrayList<>();
+        nodes.forEach(node -> {
+          NodeTopologyFull nodeJson =
+              new NodeTopologyFull(
+                  DatanodeDetails.getFromProtoBuf(node.getNodeID()), state);
+          nodesJson.add(nodeJson);
+        });
+        System.out.println(
+            JsonUtils.toJsonStringWithDefaultPrettyPrinter(nodesJson));
+      } else {
+        ArrayList<NodeTopologyDefault> nodesJson = new ArrayList<>();

Review Comment:
   ```suggestion
           List<NodeTopologyDefault> nodesJson = new ArrayList<>();
   ```



##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/TopologySubcommand.java:
##########
@@ -136,7 +144,8 @@ public Class<?> getParentType() {
   // Format
   // Location: rack1
   //  ipAddress(hostName) OperationalState
-  private void printOrderedByLocation(List<HddsProtos.Node> nodes) {
+  private void printOrderedByLocation(List<HddsProtos.Node> nodes,
+                                      String nState) throws IOException {

Review Comment:
   ```suggestion
                                         String state) throws IOException {
   ```



##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/TopologySubcommand.java:
##########
@@ -153,13 +162,28 @@ private void printOrderedByLocation(List<HddsProtos.Node> 
nodes) {
     ArrayList<String> locations = new ArrayList<>(tree.keySet());
     Collections.sort(locations);
 
-    locations.forEach(location -> {
-      System.out.println("Location: " + location);
-      tree.get(location).forEach(n -> {
-        System.out.println(" " + n.getIpAddress() + "(" + n.getHostName()
-            + ") " + state.get(n));
+    if (json) {
+      List<NodeTopologyOrder> nodesJson = new ArrayList<>();
+      locations.forEach(location -> {
+        tree.get(location).forEach(n -> {
+          NodeTopologyOrder nodeJson = new NodeTopologyOrder(n, nState,
+              state.get(n).toString());
+          nodesJson.add(nodeJson);
+        });
+      });
+      System.out.println(
+          JsonUtils.toJsonStringWithDefaultPrettyPrinter(nodesJson));
+    } else {

Review Comment:
   Remove else, return from if statement.



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to