slfan1989 commented on code in PR #7016:
URL: https://github.com/apache/ozone/pull/7016#discussion_r1734903355
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/scm/GetScmRatisRolesSubcommand.java:
##########
@@ -50,13 +52,54 @@ public class GetScmRatisRolesSubcommand extends
ScmSubcommand {
description = "Format output as JSON")
private boolean json;
+ @CommandLine.Option(names = { "--table" },
+ defaultValue = "false",
+ description = "Format output as Table")
+ private boolean table;
+
+ private static final String SCM_ROLES_TITLE = "Storage Container Manager
Roles";
+
+ private static final List<String> RATIS_SCM_ROLES_HEADER = Arrays.asList(
+ "Host Name", "Ratis Port", "Node ID", "Role", "Host Address");
+
+ private static final List<String> STANDALONE_SCM_ROLES_HEADER =
Arrays.asList("Host Name", "Port");
+
@Override
protected void execute(ScmClient scmClient) throws IOException {
List<String> ratisRoles = scmClient.getScmRatisRoles();
+ boolean isRatisEnabled = scmClient.isSCMRatisEnable();
if (json) {
Map<String, Map<String, String>> scmRoles = parseScmRoles(ratisRoles);
System.out.print(
JsonUtils.toJsonStringWithDefaultPrettyPrinter(scmRoles));
+ } else if (table) {
+ FormattingCLIUtils formattingCLIUtils = new
FormattingCLIUtils(SCM_ROLES_TITLE);
+
+ // Determine which header to use based on whether Ratis is enabled or
not.
+ if (isRatisEnabled) {
+ formattingCLIUtils.addHeaders(RATIS_SCM_ROLES_HEADER);
Review Comment:
@whbing @ChenSammi
Thank you very much for reviewing this PR! I’d like to explain why I added
the `scmHAEnable`:
When using `--table` to output SCM information, we need to first determine
the length of the table headers.
For non-HA SCMs, we will display it in the following format:
```
+-----------------------------------------------+
| Storage Container Manager Roles |
+---------------------------------+-------------+
| Host Name | Port |
+---------------------------------+-------------+
```
For HA SCMs, we will display it in the following format:
```
+---------------------------------------------------------------------------------------------------------------+
| Storage Container Manager Roles
|
+---------------------------------+------------+----------+--------------------------------------+--------------+
| Host Name | Ratis Port | Role |
Node ID | Host Address |
+---------------------------------+------------+----------+--------------------------------------+--------------+
```
I need a clear method to determine if SCM has Ratis enabled.
I am considering two approaches:
Approach 1: Use the `ratisRole` length of the returned data for judgment.
Approach 2: Add a return attribute that explicitly indicates whether Ratis
is being used.
From my perspective, Approach 2 is better, as the code looks clearer.
--
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]
