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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to