ddanielr commented on code in PR #5749:
URL: https://github.com/apache/accumulo/pull/5749#discussion_r2243499131


##########
assemble/bin/accumulo-cluster:
##########


Review Comment:
   Confirmed  `accumulo-cluster start --local` dies immediately on an undefined 
resource group
   ```
   ./accumulo-cluster start --local
   2025-07-30T18:02:29,282 [conf.SiteConfiguration] INFO : Found Accumulo 
configuration on classpath at 
/workspace/fluo-uno/install/accumulo-4.0.0-SNAPSHOT/conf/accumulo.properties
   Processing error: Resource group configured that does not exist in 
ZooKeeper. ZK: [default], configured: test
   Thread 'org.apache.accumulo.server.conf.cluster.ClusterConfigParser' died.
   java.lang.IllegalStateException: Resource group configured that does not 
exist in ZooKeeper. ZK: [default], configured: test
        at 
org.apache.accumulo.server.conf.cluster.ClusterConfigParser.validateConfiguredGroups(ClusterConfigParser.java:167)
        at 
org.apache.accumulo.server.conf.cluster.ClusterConfigParser.outputShellVariables(ClusterConfigParser.java:229)
        at 
org.apache.accumulo.server.conf.cluster.ClusterConfigParser.main(ClusterConfigParser.java:278)
        at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
        at 
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:569)
        at org.apache.accumulo.start.Main.lambda$execMainClass$1(Main.java:118)
        at java.base/java.lang.Thread.run(Thread.java:840)
   ```



##########
server/base/src/main/java/org/apache/accumulo/server/conf/util/ZooPropEditor.java:
##########


Review Comment:
   If a resource group doesn't exist then the following lines throw NPE 
exceptions
   ```
   writer.printf(": Data version: %d\n", props.getDataVersion());
   writer.printf(": Timestamp: %s\n", props.getTimestampISO());
   ```
   Suggest moving them past the empty props check on line 189 or adding a null 
check to indicate the resource group doesn't exist. 
   



##########
shell/src/main/java/org/apache/accumulo/shell/commands/ConfigCommand.java:
##########
@@ -436,6 +480,8 @@ public Options getOptions() {
     outputFileOpt = new Option("o", "output", true, "local file to write the 
scan output to");
     namespaceOpt = new Option(ShellOptions.namespaceOption, "namespace", true,
         "namespace to display/set/delete properties for");
+    resourceGroupOpt = new Option(ShellOptions.resourceGroupOption, 
"resourceGroup", true,

Review Comment:
   Yeah I dropped a comment about this as users are going to expect the 
resource group properties to behave similar to the table properties.
   
   If rg is set without `-d` or `-s` then at minimum, a message should be 
inform the user that what they are doing is incorrect. 
   example: "option rg has no effect on printed results`
   
   I did test the dumpConfig command and it did produce a nice rg.cfg file 
   ```
   $ cat test_rg.cfg 
   resourcegroup -c test
   config -rg test -s general.custom.test=passing
   ```



##########
shell/src/main/java/org/apache/accumulo/shell/commands/ResourceGroupCommand.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.shell.commands;
+
+import java.util.Set;
+import java.util.TreeSet;
+
+import org.apache.accumulo.core.client.admin.ResourceGroupOperations;
+import org.apache.accumulo.core.data.ResourceGroupId;
+import org.apache.accumulo.shell.Shell;
+import org.apache.accumulo.shell.Shell.Command;
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.Option;
+import org.apache.commons.cli.Options;
+
+public class ResourceGroupCommand extends Command {
+
+  private Option createOpt;
+  private Option listOpt;
+  private Option deleteOpt;
+
+  @Override
+  public int execute(String fullCommand, CommandLine cl, Shell shellState) 
throws Exception {
+
+    final ResourceGroupOperations ops = 
shellState.getAccumuloClient().resourceGroupOperations();
+
+    if (cl.hasOption(createOpt.getOpt())) {
+      ops.create(ResourceGroupId.of(cl.getOptionValue(createOpt)));
+    } else if (cl.hasOption(deleteOpt.getOpt())) {
+      ops.remove(ResourceGroupId.of(cl.getOptionValue(deleteOpt)));
+    } else if (cl.hasOption(listOpt.getOpt())) {
+      Set<String> sorted = new TreeSet<>();
+      ops.list().forEach(rg -> sorted.add(rg.canonical()));
+      shellState.printLines(sorted.iterator(), false);
+    }
+    return 0;
+  }
+
+  @Override
+  public Options getOptions() {
+    final Options o = new Options();
+
+    createOpt = new Option("c", "create", true, "create a resource group");
+    createOpt.setArgName("group");
+    o.addOption(createOpt);
+
+    listOpt = new Option("l", "list", false, "display resource group names");
+    o.addOption(listOpt);
+
+    deleteOpt = new Option("d", "delete", true, "delete a resource group");
+    deleteOpt.setArgName("group");
+    o.addOption(deleteOpt);
+
+    return o;
+  }
+
+  @Override
+  public String description() {
+    return "create, list, or remove resource groups";

Review Comment:
   I believe what keith is asking for is possible with the refactored help flag 
work that was done. 



-- 
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