[GitHub] [accumulo] ivakegg commented on a diff in pull request #3562: Fixes 3233. Add copy-properties option for createnamespace and createtable shell commands

["ivakegg (via GitHub)" <gi...@apache.org>]

ivakegg commented on code in PR #3562:
URL: https://github.com/apache/accumulo/pull/3562#discussion_r1254641316


##########
shell/src/main/java/org/apache/accumulo/shell/commands/CreateNamespaceCommand.java:
##########
@@ -32,42 +32,55 @@
 import org.apache.accumulo.shell.Shell.Command;
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.Option;
-import org.apache.commons.cli.OptionGroup;
 import org.apache.commons.cli.Options;
 
 public class CreateNamespaceCommand extends Command {
   private Option createNamespaceOptCopyConfig;
+  private Option createNamesapceOptExcludeParentProps;
 
   @Override
   public int execute(final String fullCommand, final CommandLine cl, final Shell shellState)
       throws AccumuloException, AccumuloSecurityException, TableExistsException,
       TableNotFoundException, IOException, ClassNotFoundException, NamespaceExistsException,
       NamespaceNotFoundException {
 
-    if (createNamespaceOptCopyConfig == null) {
-      getOptions();
+    // validate that copy config and copy properties options are mutually exclusive.
+    if (cl.hasOption(createNamesapceOptExcludeParentProps.getLongOpt())
+        && !cl.hasOption(createNamespaceOptCopyConfig.getOpt())) {
+      throw new IllegalArgumentException(createNamesapceOptExcludeParentProps.getLongOpt()
+          + " only valid when using " + createNamespaceOptCopyConfig.getLongOpt());
     }
 
     String namespace = cl.getArgs()[0];
 
     shellState.getAccumuloClient().namespaceOperations().create(namespace);
+    if (!shellState.getAccumuloClient().namespaceOperations().exists(namespace)) {
+      throw new IllegalArgumentException("Could not create namespace `" + namespace + "`");
+    }
 
-    // Copy options if flag was set
-    Map<String,String> configuration = null;
+    // Copy configuration options if flag was set
     if (cl.hasOption(createNamespaceOptCopyConfig.getOpt())) {
-      String copy = cl.getOptionValue(createNamespaceOptCopyConfig.getOpt());
-      if (shellState.getAccumuloClient().namespaceOperations().exists(namespace)) {
-        configuration = shellState.getAccumuloClient().namespaceOperations().getConfiguration(copy);
+      String srcNs = cl.getOptionValue(createNamespaceOptCopyConfig.getOpt());
+      if (!srcNs.isEmpty() && !shellState.getAccumuloClient().namespaceOperations().exists(srcNs)) {
+        throw new NamespaceNotFoundException(null, srcNs, null);
+      }
+      Map<String,String> userProps;
+      if (cl.hasOption(createNamesapceOptExcludeParentProps.getLongOpt())) {
+        // use namespace props - excludes parents in configuration
+        userProps =
+            shellState.getAccumuloClient().namespaceOperations().getNamespaceProperties(srcNs);
+      } else {
+        // use namespace config - include parents in configuration hierarchy
+        userProps = shellState.getAccumuloClient().namespaceOperations().getConfiguration(srcNs);
+      }
+      if (userProps != null) {

Review Comment:
   can userProps actually be null here?



-- 
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: notifications-unsubscribe@accumulo.apache.org

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