haiyang1987 commented on PR #5667:
URL: https://github.com/apache/hadoop/pull/5667#issuecomment-1554608527
> > a miss on the actual PR. "Period"
>
> I agree.
>
> @haiyang1987 for this PR, since you already have the opportunity, I would
like to propose these changes so that any new argument in future will not have
to go through the same fate:
>
> ```
> diff --git
a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSAdmin.java
b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSAdmin.java
> index d717476dded..c25e2cf3579 100644
> ---
a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSAdmin.java
> +++
b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSAdmin.java
> @@ -489,7 +489,11 @@ public DFSAdmin(Configuration conf) {
> protected DistributedFileSystem getDFS() throws IOException {
> return AdminHelper.checkAndGetDFS(getFS(), getConf());
> }
> -
> +
> + public static final String[] DFS_REPORT_ARGS =
> + new String[] {"-live", "-dead", "-decommissioning",
"-enteringmaintenance", "-inmaintenance",
> + "-slownodes"};
> +
> /**
> * Gives a report on how the FileSystem is doing.
> * @exception IOException if the filesystem does not exist.
> @@ -581,16 +585,16 @@ public void report(String[] argv, int i) throws
IOException {
> List<String> args = Arrays.asList(argv);
> // Truncate already handled arguments before parsing
report()-specific ones
> args = new ArrayList<String>(args.subList(i, args.size()));
> - final boolean listLive = StringUtils.popOption("-live", args);
> - final boolean listDead = StringUtils.popOption("-dead", args);
> + final boolean listLive = StringUtils.popOption(DFS_REPORT_ARGS[0],
args);
> + final boolean listDead = StringUtils.popOption(DFS_REPORT_ARGS[1],
args);
> final boolean listDecommissioning =
> - StringUtils.popOption("-decommissioning", args);
> + StringUtils.popOption(DFS_REPORT_ARGS[2], args);
> final boolean listEnteringMaintenance =
> - StringUtils.popOption("-enteringmaintenance", args);
> + StringUtils.popOption(DFS_REPORT_ARGS[3], args);
> final boolean listInMaintenance =
> - StringUtils.popOption("-inmaintenance", args);
> + StringUtils.popOption(DFS_REPORT_ARGS[4], args);
> final boolean listSlowNodes =
> - StringUtils.popOption("-slownodes", args);
> + StringUtils.popOption(DFS_REPORT_ARGS[5], args);
>
>
> // If no filter flags are found, then list all DN types
> @@ -2399,7 +2403,7 @@ public int run(String[] argv) {
> return exitCode;
> }
> } else if ("-report".equals(cmd)) {
> - if (argv.length > 6) {
> + if (argv.length > 7) {
> printUsage(cmd);
> return exitCode;
> }
> diff --git
a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/TestDFSAdmin.java
b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/TestDFSAdmin.java
> index d81aebf3c2e..eaa7a88ca0d 100644
> ---
a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/TestDFSAdmin.java
> +++
b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/TestDFSAdmin.java
> @@ -795,6 +795,16 @@ public void testReportCommand() throws Exception {
> resetStream();
> assertEquals(0, ToolRunner.run(dfsAdmin, new String[] {"-report"}));
> verifyNodesAndCorruptBlocks(numDn, numDn - 1, 1, 1, client, 0L, 0L);
> +
> + // verify report command for list all DN types
> + resetStream();
> + String[] reportWithArg = new String[DFSAdmin.DFS_REPORT_ARGS.length
+ 1];
> + reportWithArg[0] = "-report";
> + int k=1;
> + for (int i = 0; i < DFSAdmin.DFS_REPORT_ARGS.length; i++) {
> + reportWithArg[k++] = DFSAdmin.DFS_REPORT_ARGS[i];
> + }
> + assertEquals(0, ToolRunner.run(dfsAdmin, reportWithArg));
> }
> }
> ```
>
> once again, thanks for the PR.
Thanks for the suggestion, I think this change will solve the problem better.
I'll update the pr later.
--
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]
