adoroszlai commented on code in PR #8050:
URL: https://github.com/apache/ozone/pull/8050#discussion_r2007126389
##########
hadoop-ozone/dist/src/main/smoketest/admincli/container.robot:
##########
@@ -87,17 +97,54 @@ Report containers as JSON
List all containers
${output} = Execute ozone admin container list --all
Should contain ${output} OPEN
+ Should Start With ${output} [
+ Should End With ${output} ]
List all containers according to count (batchSize)
${output} = Execute ozone admin container list --all
--count 10
Should contain ${output} OPEN
+ Should Start With ${output} [
+ Should End With ${output} ]
List all containers from a particular container ID
- ${output} = Execute ozone admin container list --all
--start 1
+ ${output} = Execute ozone admin container list --all
--start 2
Should contain ${output} OPEN
+ Should Start With ${output} [
+ Should End With ${output} ]
+
+Check JSON array parsing
+ ${output} = Execute ozone admin container list
+ Should Start With ${output} [
+ Should Contain ${output} containerID
+ Should End With ${output} ]
+ ${containerIDs} = Execute echo '${output}' | jq -r
'.[].containerID'
+ Should Not Be Empty ${containerIDs}
+
+Check state filtering with JSON array format
+ ${output} = Execute ozone admin container list
--state=OPEN
+ Should Start With ${output} [
+ Should End With ${output} ]
+ ${states} = Execute echo '${output}' | jq -r '.[].state'
+ Should Contain ${states} OPEN
+ Should Not Contain ${states} CLOSED
+
+Check count limit with JSON array format
+ ${output} = Execute ozone admin container create
+ Should contain ${output} is created
+ ${output} = Execute ozone admin container create
+ Should contain ${output} is created
+ ${output} = Execute ozone admin container create
+ Should contain ${output} is created
+ ${output} = Execute ozone admin container create
+ Should contain ${output} is created
+ ${output} = Execute ozone admin container create
+ Should contain ${output} is created
Review Comment:
I would like to eliminate this duplication, as well as the duplicated
start/end checks, in a follow-up.
##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ListSubcommand.java:
##########
@@ -114,44 +116,104 @@ public void execute(ScmClient scmClient) throws
IOException {
.getInt(ScmConfigKeys.OZONE_SCM_CONTAINER_LIST_MAX_COUNT,
ScmConfigKeys.OZONE_SCM_CONTAINER_LIST_MAX_COUNT_DEFAULT);
- ContainerListResult containerListAndTotalCount;
+ // Use SequenceWriter to output JSON array format for all cases
+ SequenceWriter sequenceWriter = WRITER.writeValues(new
NonClosingOutputStream(System.out));
+ sequenceWriter.init(true); // Initialize as a JSON array
if (!all) {
+ // Regular listing with count limit
if (count > maxCountAllowed) {
System.err.printf("Attempting to list the first %d records of
containers." +
" However it exceeds the cluster's current limit of %d. The
results will be capped at the" +
- " maximum allowed count.%n", count,
ScmConfigKeys.OZONE_SCM_CONTAINER_LIST_MAX_COUNT_DEFAULT);
+ " maximum allowed count.%n", count, maxCountAllowed);
count = maxCountAllowed;
}
- containerListAndTotalCount = scmClient.listContainer(startId, count,
state, type, repConfig);
- for (ContainerInfo container :
containerListAndTotalCount.getContainerInfoList()) {
- outputContainerInfo(container);
- }
- if (containerListAndTotalCount.getTotalCount() > count) {
+ ContainerListResult containerListResult =
+ scmClient.listContainer(startId, count, state, type, repConfig);
+
+ writeContainers(sequenceWriter,
containerListResult.getContainerInfoList());
+
+ closeStream(sequenceWriter);
+ if (containerListResult.getTotalCount() > count) {
System.err.printf("Displaying %d out of %d containers. " +
- "Container list has more containers.%n",
- count, containerListAndTotalCount.getTotalCount());
+ "Container list has more containers.%n",
+ count, containerListResult.getTotalCount());
}
} else {
- // Batch size is either count passed through cli or maxCountAllowed
+ // List all containers by fetching in batches
int batchSize = (count > 0) ? count : maxCountAllowed;
- long currentStartId = startId;
- int fetchedCount;
-
- do {
- // Fetch containers in batches of 'batchSize'
- containerListAndTotalCount = scmClient.listContainer(currentStartId,
batchSize, state, type, repConfig);
- fetchedCount =
containerListAndTotalCount.getContainerInfoList().size();
-
- for (ContainerInfo container :
containerListAndTotalCount.getContainerInfoList()) {
- outputContainerInfo(container);
- }
-
- if (fetchedCount > 0) {
- currentStartId =
containerListAndTotalCount.getContainerInfoList().get(fetchedCount -
1).getContainerID() + 1;
- }
- } while (fetchedCount > 0);
+ listAllContainers(scmClient, sequenceWriter, batchSize, repConfig);
+ closeStream(sequenceWriter);
+ }
+ }
+
+ private void writeContainers(SequenceWriter writer, List<ContainerInfo>
containers)
+ throws IOException {
+ for (ContainerInfo container : containers) {
+ writer.write(container);
+ }
+ }
+
+ private void closeStream(SequenceWriter writer) throws IOException {
+ writer.flush();
+ writer.close();
+ // Add the final newline
+ System.out.println();
+ }
+
+ private void listAllContainers(ScmClient scmClient, SequenceWriter writer,
+ int batchSize, ReplicationConfig repConfig)
Review Comment:
nit: Please do not format method signature like this. Whenever visibility /
return type / method name / other modifiers are changed, we would have to
reindent parameters.
--
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]
