errose28 commented on code in PR #8050:
URL: https://github.com/apache/ozone/pull/8050#discussion_r1989770192
##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ListSubcommand.java:
##########
@@ -90,11 +95,35 @@ public class ListSubcommand extends ScmSubcommand {
}
- private void outputContainerInfo(ContainerInfo containerInfo)
- throws IOException {
- // Print container report info.
+ private void outputContainerInfo(ContainerInfo containerInfo) throws
IOException {
+ // Original behavior - just print the container JSON
System.out.println(WRITER.writeValueAsString(containerInfo));
}
+
+ private void outputContainerInfoAsJsonMember(ContainerInfo containerInfo,
boolean isFirst,
+ boolean isLast) throws IOException {
+ // JSON array format with proper brackets and commas
+ if (isFirst) {
+ // Start of array
+ System.out.print("[");
Review Comment:
Let's use Jackson's `SequenceWriter` for this similar to what is done in
#7944. We may need to duplicate some minor changes to `JsonUtils` here but that
should be ok and we can resolve it on the next reverse merge into reconciler
branch.
##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ListSubcommand.java:
##########
@@ -76,6 +76,11 @@ public class ListSubcommand extends ScmSubcommand {
description = "Container replication (ONE, THREE for Ratis, " +
"rs-6-3-1024k for EC)")
private String replication;
+
+ @Option(names = {"--json"},
+ description = "Output the entire list in JSON array format",
Review Comment:
The original is still json (and can still be streamed into jq), it just
doesn't represent a valid json file if it is saved that way. In that sense I
think the flag here is confusing. I would just make json list the default and
only behavior. We did something similar in HDDS-5824 and did not consider it a
breaking change, since the original was a "bug". With 2.0 coming up such a
change makes sense as well.
##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ListSubcommand.java:
##########
@@ -76,6 +76,11 @@ public class ListSubcommand extends ScmSubcommand {
description = "Container replication (ONE, THREE for Ratis, " +
"rs-6-3-1024k for EC)")
private String replication;
+
+ @Option(names = {"--json"},
+ description = "Output the entire list in JSON array format",
Review Comment:
Existing acceptance tests would need to be updated to account for this.
##########
hadoop-ozone/dist/src/main/smoketest/admincli/container.robot:
##########
@@ -96,6 +96,33 @@ List all containers from a particular container ID
${output} = Execute ozone admin container list --all
--start 1
Should contain ${output} OPEN
+List containers in JSON array format
+ ${output} = Execute ozone admin container list --json |
jq -r '.'
+ Should Start With ${output} [
Review Comment:
This way of testing works, but a jq query to do the same test might be more
robust (although failure messages may be more confusing):
`ozone admin container list --json | jq '.[] | .containerID'` then ensure
that we actually get content in the output.
--
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]
