goiri commented on code in PR #6199:
URL: https://github.com/apache/hadoop/pull/6199#discussion_r1364211542
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/RouterCLI.java:
##########
@@ -77,25 +79,70 @@ public class RouterCLI extends Configured implements Tool {
private static final Logger LOG = LoggerFactory.getLogger(RouterCLI.class);
- protected final static Map<String, UsageInfo> ADMIN_USAGE =
- ImmutableMap.<String, UsageInfo>builder()
- // Command1: deregisterSubCluster
- .put("-deregisterSubCluster", new UsageInfo(
- "[-sc|--subClusterId [subCluster Id]]",
- "Deregister SubCluster, If the interval between the heartbeat time of
the subCluster " +
- "and the current time exceeds the timeout period, " +
- "set the state of the subCluster to SC_LOST."))
- // Command2: policy
- .put("-policy", new UsageInfo(
- "[-s|--save [queue;router weight;amrm weight;headroomalpha]] " +
- "[-bs|--batch-save [--format xml] [-f|--input-file fileName]]" +
- "[-l|--list [--pageSize][--currentPage][--queue][--queues]]",
- "We provide a set of commands for Policy:" +
- " Include list policies, save policies, batch save policies. " +
- " (Note: The policy type will be directly read from the" +
- " yarn.federation.policy-manager in the local yarn-site.xml.)" +
- " eg. (routeradmin -policy [-s|--save]
root.a;SC-1:0.7,SC-2:0.3;SC-1:0.7,SC-2:0.3;1.0)"))
- .build();
+ protected final static UsageInfo SUBCLUSTER_ID = new
UsageInfo("<-sc|--subClusterId>",
+ "'-sc' option allows you to specify the sub-cluster to operate on, " +
+ "while the '--subClusterId' option is the long format of -sc and serves
the same purpose.");
+ protected final static String DSEXAMPLE_1 = "yarn routeradmin
-deregisterSubCluster -sc SC-1";
+ protected final static String DSEXAMPLE_2 = "yarn routeradmin
-deregisterSubCluster --subClusterId SC-1";
+
+ protected final static RouterExampleUsageInfos DSEXAMPLEUSAGEINFOS_1 = new
RouterExampleUsageInfos(
+ Arrays.asList(DSEXAMPLE_1, DSEXAMPLE_2),
Review Comment:
I don't think this is the right spacing.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/RouterCLI.java:
##########
@@ -77,25 +79,70 @@ public class RouterCLI extends Configured implements Tool {
private static final Logger LOG = LoggerFactory.getLogger(RouterCLI.class);
- protected final static Map<String, UsageInfo> ADMIN_USAGE =
- ImmutableMap.<String, UsageInfo>builder()
- // Command1: deregisterSubCluster
- .put("-deregisterSubCluster", new UsageInfo(
- "[-sc|--subClusterId [subCluster Id]]",
- "Deregister SubCluster, If the interval between the heartbeat time of
the subCluster " +
- "and the current time exceeds the timeout period, " +
- "set the state of the subCluster to SC_LOST."))
- // Command2: policy
- .put("-policy", new UsageInfo(
- "[-s|--save [queue;router weight;amrm weight;headroomalpha]] " +
- "[-bs|--batch-save [--format xml] [-f|--input-file fileName]]" +
- "[-l|--list [--pageSize][--currentPage][--queue][--queues]]",
- "We provide a set of commands for Policy:" +
- " Include list policies, save policies, batch save policies. " +
- " (Note: The policy type will be directly read from the" +
- " yarn.federation.policy-manager in the local yarn-site.xml.)" +
- " eg. (routeradmin -policy [-s|--save]
root.a;SC-1:0.7,SC-2:0.3;SC-1:0.7,SC-2:0.3;1.0)"))
- .build();
+ protected final static UsageInfo SUBCLUSTER_ID = new
UsageInfo("<-sc|--subClusterId>",
+ "'-sc' option allows you to specify the sub-cluster to operate on, " +
+ "while the '--subClusterId' option is the long format of -sc and serves
the same purpose.");
+ protected final static String DSEXAMPLE_1 = "yarn routeradmin
-deregisterSubCluster -sc SC-1";
+ protected final static String DSEXAMPLE_2 = "yarn routeradmin
-deregisterSubCluster --subClusterId SC-1";
+
+ protected final static RouterExampleUsageInfos DSEXAMPLEUSAGEINFOS_1 = new
RouterExampleUsageInfos(
+ Arrays.asList(DSEXAMPLE_1, DSEXAMPLE_2),
+ Arrays.asList("At this point we can use the following command:"));
+ protected final static RouterUsageInfos DEREGISTER_SUBCLUSTER =
+ new RouterUsageInfos(Arrays.asList(SUBCLUSTER_ID),
+ Arrays.asList("deregister subCluster, " +
Review Comment:
This doesn't look much more readable to be honest.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/cli/RouterCLI.java:
##########
@@ -159,43 +214,59 @@ public RouterCLI(Configuration conf) {
}
private static void buildHelpMsg(String cmd, StringBuilder builder) {
- UsageInfo usageInfo = ADMIN_USAGE.get(cmd);
- if (usageInfo == null) {
+ RouterUsageInfos routerUsageInfo = ADMIN_USAGE.get(cmd);
+
+ if (routerUsageInfo == null) {
return;
}
+ builder.append("[").append(cmd).append("]\n");
- if (usageInfo.args != null) {
- String space = (usageInfo.args == "") ? "" : " ";
- builder.append(" ")
- .append(cmd)
- .append(space)
- .append(usageInfo.args)
- .append(": ")
- .append(usageInfo.help);
- } else {
- builder.append(" ")
- .append(cmd)
- .append(": ")
- .append(usageInfo.help);
+ if (!routerUsageInfo.helpInfos.isEmpty()) {
+ builder.append("\t Description: \n");
+ for (String helpInfo : routerUsageInfo.helpInfos) {
+ builder.append("\t\t").append(helpInfo).append("\n\n");
+ }
}
- }
- private static void buildIndividualUsageMsg(String cmd, StringBuilder
builder) {
- UsageInfo usageInfo = ADMIN_USAGE.get(cmd);
- if (usageInfo == null) {
- return;
+ if (!routerUsageInfo.usageInfos.isEmpty()) {
+ builder.append("\t UsageInfos: \n");
+ for (UsageInfo usageInfo : routerUsageInfo.usageInfos) {
+ builder.append("\t\t").append(usageInfo.args)
+ .append(": ")
+ .append("\n\t\t")
+ .append(usageInfo.help).append("\n\n");
+ }
}
- if (usageInfo.args == null) {
- builder.append("Usage: routeradmin [")
- .append(cmd)
- .append("]\n");
- } else {
- String space = (usageInfo.args == "") ? "" : " ";
- builder.append("Usage: routeradmin [")
- .append(cmd)
- .append(space)
- .append(usageInfo.args)
- .append("]\n");
+
+ if (MapUtils.isNotEmpty(routerUsageInfo.examples)) {
+ builder.append("\t Examples: \n");
+ int count = 1;
+ for (Map.Entry<String, RouterExampleUsageInfos> example :
+ routerUsageInfo.examples.entrySet()) {
+ String key = example.getKey();
+ builder.append("\t\t").append("Cmd:").append(count).append(".
").append(key).append(": \n\n");
+ RouterExampleUsageInfos values = example.getValue();
+ if (values == null) {
+ return;
+ }
+ // Print Command Description
+ if (CollectionUtils.isNotEmpty(values.desc)) {
+ builder.append("\t\t").append("Cmd Requirement Description:\n");
+ for (String value : values.desc) {
+ builder.append("\t\t").append(value).append("\n");
+ }
+ }
+ builder.append("\n");
+ // Print Command example
+ if (CollectionUtils.isNotEmpty(values.examples)) {
Review Comment:
Can we add unit tests for these?
--
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]
