[GitHub] jmark99 commented on a change in pull request #370: ACCUMULO-4772 Update shell to use NewTableConfiguration methods

[GitBox <gi...@apache.org>]

jmark99 commented on a change in pull request #370: ACCUMULO-4772 Update shell to use NewTableConfiguration methods
URL: https://github.com/apache/accumulo/pull/370#discussion_r165247235
 
 

 ##########
 File path: shell/src/main/java/org/apache/accumulo/shell/commands/CreateTableCommand.java
 ##########
 @@ -150,9 +169,83 @@ public int execute(final String fullCommand, final CommandLine cl, final Shell s
     return 0;
   }
 
+  /**
+   * Add supplied locality groups information to a NewTableConfiguration object.
+   *
+   * Used in conjunction with createtable shell command to allow locality groups to be configured upon table creation.
+   */
+  private NewTableConfiguration setLocalityForNewTable(CommandLine cl, NewTableConfiguration ntc) {
+    HashMap<String,Set<Text>> localityGroupMap = new HashMap<>();
+    String[] options = cl.getOptionValues(createTableOptLocalityProps.getOpt());
+    for (String localityInfo : options) {
+      final String parts[] = localityInfo.split("=", 2);
+      if (parts.length < 2)
+        throw new IllegalArgumentException("Missing '=' or there are spaces between entries");
+      final String groupName = parts[0];
+      final HashSet<Text> colFams = new HashSet<>();
+      for (String family : parts[1].split(","))
+        colFams.add(new Text(family.getBytes(Shell.CHARSET)));
+      if (localityGroupMap.containsKey(groupName))
+        throw new IllegalArgumentException("Duplicate locality group name found. Group names must be unique");
+      localityGroupMap.put(groupName, colFams);
+    }
+    ntc.setLocalityGroups(localityGroupMap);
+    return ntc;
+  }
+
+  /**
+   * Add supplied iterator information to NewTableConfiguration object.
+   *
+   * Used in conjunction with createtable shell command to allow an iterator to be configured upon table creation.
+   */
+  private NewTableConfiguration attachIteratorToNewTable(CommandLine cl, Shell shellState, NewTableConfiguration ntc) {
+    if (shellState.iteratorProfiles.size() == 0)
+      throw new IllegalArgumentException("No shell iterator profiles have been created.");
+    String[] options = cl.getOptionValues(createTableOptIteratorProps.getOpt());
+    for (String profileInfo : options) {
+      String[] parts = profileInfo.split(":", 2);
+      if (parts.length < 2)
+        throw new IllegalArgumentException("Missing scope or there are spaces between parameters");
+      // get profile name
+      String profileName = parts[0];
+      IteratorSetting iteratorSetting = shellState.iteratorProfiles.get(profileName).get(0);
+      if (iteratorSetting == null)
+        throw new IllegalArgumentException("Provided iterator profile, " + profileName + ", does not exist");
+      // parse scope info
+      List<String> scopeList = Arrays.asList(parts[1].split(","));
 
 Review comment:
   @keith-turner, I refactored the attachIteratorToNewTable method . Take a look and let me know your thoughts. I made use of IteratorUtil.IteratorScope.valueOf() for the scopeList check and rearranged the order of several checks to handle more error conditions. I didn't make use lowercase as all arguments for other shell methods use lowercase and It seemed more consistent to require users to provide lowercase parameters. If you feel strongly about handling mixed case args you can let me know and I can update.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services