simbadzina commented on code in PR #5554:
URL: https://github.com/apache/hadoop/pull/5554#discussion_r1173987391


##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableManager.java:
##########
@@ -48,6 +50,17 @@ public interface MountTableManager {
   AddMountTableEntryResponse addMountTableEntry(
       AddMountTableEntryRequest request) throws IOException;
 
+  /**
+   * All multiple entries to the mount table.

Review Comment:
   Typo "Add" vs "All"



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java:
##########
@@ -132,15 +135,15 @@ public void printUsage() {
     System.out.println(usage);
   }
 
-  private void printUsage(String cmd) {
+  private static void printUsage(String cmd) {
     String usage = getUsage(cmd);
     System.out.println(usage);
   }
 
-  private String getUsage(String cmd) {
+  private static String getUsage(String cmd) {
     if (cmd == null) {
       String[] commands =
-          {"-add", "-update", "-rm", "-ls", "-getDestination", "-setQuota",
+          {"-add", "-addall", "-update", "-rm", "-ls", "-getDestination", 
"-setQuota",

Review Comment:
   Can you use camelCase like the other two word commands.



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterAdminServer.java:
##########
@@ -358,6 +360,25 @@ public AddMountTableEntryResponse addMountTableEntry(
     return getMountTableStore().addMountTableEntry(request);
   }
 
+  @Override
+  public AddMountTableEntriesResponse 
addMountTableEntries(AddMountTableEntriesRequest request)
+      throws IOException {
+    List<MountTable> mountTables = request.getEntries();
+    for (MountTable mountTable : mountTables) {
+      verifyMaxComponentLength(mountTable);
+    }
+    if (this.mountTableCheckDestination) {
+      for (MountTable mountTable : mountTables) {
+        List<String> nsIds = verifyFileInDestinations(mountTable);
+        if (!nsIds.isEmpty()) {
+          throw new IllegalArgumentException(
+              "File not found in downstream " + "nameservices: " + 
StringUtils.join(",", nsIds));
+        }

Review Comment:
   This code is now duplicated in three places. Can we move it into a small 
separate function.



-- 
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: common-issues-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to