Skip to content

Commit

Permalink
Separates help opt check from parseArgs
Browse files Browse the repository at this point in the history
Separates the check for the `help` opt value from the parseArgs command
and creates a separate method for printing the usage statement from the
Help class

Adds method calls to the new printUsage method so logic is preserved

Removes the AdminOpts class
  • Loading branch information
ddanielr committed Dec 30, 2024
1 parent 0e9e8d9 commit 173957f
Show file tree
Hide file tree
Showing 37 changed files with 94 additions and 26 deletions.
8 changes: 6 additions & 2 deletions core/src/main/java/org/apache/accumulo/core/cli/Help.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,12 @@
import com.beust.jcommander.ParameterException;

public class Help {
private final JCommander commander = new JCommander();

@Parameter(names = {"-h", "-?", "--help", "-help"}, help = true)
public boolean help = false;

public void parseArgs(String programName, String[] args, Object... others) {
JCommander commander = new JCommander();
commander.addObject(this);
for (Object other : others) {
commander.addObject(other);
Expand All @@ -39,7 +40,10 @@ public void parseArgs(String programName, String[] args, Object... others) {
commander.usage();
exitWithError(ex.getMessage(), 1);
}
if (help) {
}

public void printUsage(boolean isHelp) {
if (isHelp) {
commander.usage();
exit(0);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ public void execute(String[] args) throws Exception {

Opts opts = new Opts();
opts.parseArgs("accumulo create-empty", args);
opts.printUsage(opts.help);

for (String arg : opts.files) {
Path path = new Path(arg);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ public static void main(String[] args) throws Exception {
public void execute(String[] args) throws Exception {
Opts opts = new Opts();
opts.parseArgs(GenerateSplits.class.getName(), args);
opts.printUsage(opts.help);
if (opts.files.isEmpty()) {
throw new IllegalArgumentException("No files were given");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ protected Class<? extends BiFunction<Key,Value,String>> getFormatter(String form
public void execute(final String[] args) throws Exception {
Opts opts = new Opts();
opts.parseArgs("accumulo rfile-info", args);
opts.printUsage(opts.help);
if (opts.files.isEmpty()) {
System.err.println("No files were given");
System.exit(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ public void execute(String[] args) throws Exception {
FileSystem fs = FileSystem.get(conf);
Opts opts = new Opts();
opts.parseArgs("accumulo split-large", args);
opts.printUsage(opts.help);

for (String file : opts.files) {
AccumuloConfiguration aconf = opts.getSiteConfiguration();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ static class Opts extends ConfigOpts {
public PrintBCInfo(String[] args) throws Exception {
Opts opts = new Opts();
opts.parseArgs("PrintInfo", args);
opts.printUsage(opts.help);
if (opts.file.isEmpty()) {
System.err.println("No files were given");
System.exit(-1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ public String description() {
public void execute(String[] args) {
Opts opts = new Opts();
opts.parseArgs("accumulo create-token", args);
opts.printUsage(opts.help);

String pass = opts.password;
if (pass == null && opts.securePassword != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ static class Opts extends ClientOpts {
public void start(String[] args) throws MergeException {
Opts opts = new Opts();
opts.parseArgs(Merge.class.getName(), args);
opts.printUsage(opts.help);
Span span = TraceUtil.startSpan(Merge.class, "start");
try (Scope scope = span.makeCurrent()) {

Expand Down
20 changes: 20 additions & 0 deletions core/src/test/java/org/apache/accumulo/core/cli/TestHelp.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,12 @@
package org.apache.accumulo.core.cli;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

import org.junit.jupiter.api.Test;

import com.beust.jcommander.Parameter;

public class TestHelp {
protected class HelpStub extends Help {
@Override
Expand All @@ -46,4 +49,21 @@ public void testInvalidArgs() {
}
}

@Test
public void testHelpCommand() {
class TestHelpOpt extends HelpStub {
@Parameter(names = {"--test"})
boolean test = false;
}

String[] args = {"--help", "--test"};
TestHelpOpt opts = new TestHelpOpt();
opts.parseArgs("program", args);
assertTrue(opts.test);
try {
opts.printUsage(opts.help);
} catch (RuntimeException e) {
assertEquals("0", e.getMessage());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ public int run(String[] args) throws Exception {
job.setJarByClass(this.getClass());
RowHash.Opts opts = new RowHash.Opts();
opts.parseArgs(RowHash.class.getName(), args);
opts.printUsage(opts.help);
job.setInputFormatClass(AccumuloInputFormat.class);

String col = opts.column;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ public static class Opts extends Help {
public static void main(String[] args) throws IOException, InterruptedException {
Opts opts = new Opts();
opts.parseArgs(MiniAccumuloRunner.class.getName(), args);
opts.printUsage(opts.help);

if (opts.printProps) {
printProperties();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ protected AbstractServer(String appName, ServerOpts opts, String[] args) {
this.log = LoggerFactory.getLogger(getClass().getName());
this.applicationName = appName;
opts.parseArgs(appName, args);
opts.printUsage(opts.help);
this.hostname = Objects.requireNonNull(opts.getAddress());
var siteConfig = opts.getSiteConfiguration();
SecurityUtil.serverLogin(siteConfig);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ public static void main(String[] args) throws Exception {
public void execute(String[] args) throws Exception {
Opts opts = new Opts();
opts.parseArgs(keyword(), args);
opts.printUsage(opts.help);

if (opts.filePath == null) {
throw new IllegalArgumentException("No properties file was given");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ public void execute(String[] args) throws Exception {

ZooInfoViewer.Opts opts = new ZooInfoViewer.Opts();
opts.parseArgs(ZooInfoViewer.class.getName(), args);
opts.printUsage(opts.help);

log.info("print ids map: {}", opts.printIdMap);
log.info("print properties: {}", opts.printProps);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ public String description() {
public void execute(String[] args) throws Exception {
ZooPropEditor.Opts opts = new ZooPropEditor.Opts();
opts.parseArgs(ZooPropEditor.class.getName(), args);
opts.printUsage(opts.help);

ZooReaderWriter zrw = new ZooReaderWriter(opts.getSiteConfiguration());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,7 @@ public void execute(final String[] args) {
boolean success = true;
Opts opts = new Opts();
opts.parseArgs("accumulo init", args);
opts.printUsage(opts.help);
var siteConfig = SiteConfiguration.auto();
ZooReaderWriter zoo = new ZooReaderWriter(siteConfig);
SecurityUtil.serverLogin(siteConfig);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,26 +96,28 @@
public class Admin implements KeywordExecutable {
private static final Logger log = LoggerFactory.getLogger(Admin.class);

static class AdminOpts extends ServerUtilOpts {
@Parameter(names = {"-f", "--force"},
description = "force the given server to stop by removing its lock")
boolean force = false;
private static class SubCommandOpts {
@Parameter(names = {"-h", "-?", "--help", "-help"}, help = true)
public boolean help = false;
}

@Parameters(commandDescription = "stop the tablet server on the given hosts")
static class StopCommand {
static class StopCommand extends SubCommandOpts {
@Parameter(names = {"-f", "--force"},
description = "force the given server to stop by removing its lock")
boolean force = false;
@Parameter(description = "<host> {<host> ... }")
List<String> args = new ArrayList<>();
}

@Parameters(commandDescription = "Ping tablet servers. If no arguments, pings all.")
static class PingCommand {
static class PingCommand extends SubCommandOpts {
@Parameter(description = "{<host> ... }")
List<String> args = new ArrayList<>();
}

@Parameters(commandDescription = "print tablets that are offline in online tables")
static class CheckTabletsCommand {
static class CheckTabletsCommand extends SubCommandOpts {
@Parameter(names = "--fixFiles", description = "Remove dangling file pointers")
boolean fixFiles = false;

Expand All @@ -125,17 +127,17 @@ static class CheckTabletsCommand {
}

@Parameters(commandDescription = "stop the manager")
static class StopManagerCommand {}
static class StopManagerCommand extends SubCommandOpts {}

@Deprecated(since = "2.1.0")
@Parameters(commandDescription = "stop the master (DEPRECATED -- use stopManager instead)")
static class StopMasterCommand {}

@Parameters(commandDescription = "stop all tablet servers and the manager")
static class StopAllCommand {}
static class StopAllCommand extends SubCommandOpts {}

@Parameters(commandDescription = "list Accumulo instances in zookeeper")
static class ListInstancesCommand {
static class ListInstancesCommand extends SubCommandOpts {
@Parameter(names = "--print-errors", description = "display errors while listing instances")
boolean printErrors = false;
@Parameter(names = "--print-all",
Expand All @@ -144,13 +146,13 @@ static class ListInstancesCommand {
}

@Parameters(commandDescription = "Accumulo volume utility")
static class VolumesCommand {
static class VolumesCommand extends SubCommandOpts {
@Parameter(names = {"-l", "--list"}, description = "list volumes currently in use")
boolean printErrors = false;
}

@Parameters(commandDescription = "print out non-default configuration settings")
static class DumpConfigCommand {
static class DumpConfigCommand extends SubCommandOpts {
@Parameter(names = {"-a", "--all"},
description = "print the system and all table configurations")
boolean allConfiguration = false;
Expand All @@ -173,13 +175,13 @@ static class DumpConfigCommand {
+ "necessary.";

@Parameters(commandDescription = RV_DEPRECATION_MSG)
static class RandomizeVolumesCommand {
static class RandomizeVolumesCommand extends SubCommandOpts {
@Parameter(names = {"-t"}, description = "table to update", required = true)
String tableName = null;
}

@Parameters(commandDescription = "Verify all Tablets are assigned to tablet servers")
static class VerifyTabletAssignmentsCommand {
static class VerifyTabletAssignmentsCommand extends SubCommandOpts {
@Parameter(names = {"-v", "--verbose"},
description = "verbose mode (prints locations of tablets)")
boolean verbose = false;
Expand All @@ -194,14 +196,14 @@ static class ChangeSecretCommand {}

@Parameters(
commandDescription = "List or delete Tablet Server locks. Default with no arguments is to list the locks.")
static class TabletServerLocksCommand {
static class TabletServerLocksCommand extends SubCommandOpts {
@Parameter(names = "-delete", description = "specify a tablet server lock to delete")
String delete = null;
}

@Parameters(
commandDescription = "Deletes specific instance name or id from zookeeper or cleans up all old instances.")
static class DeleteZooInstanceCommand {
static class DeleteZooInstanceCommand extends SubCommandOpts {
@Parameter(names = {"-i", "--instance"}, description = "the instance name or id to delete")
String instance;
@Parameter(names = {"-c", "--clean"},
Expand All @@ -214,7 +216,7 @@ static class DeleteZooInstanceCommand {
}

@Parameters(commandDescription = "Restore Zookeeper data from a file.")
static class RestoreZooCommand {
static class RestoreZooCommand extends SubCommandOpts {
@Parameter(names = "--overwrite")
boolean overwrite = false;

Expand All @@ -224,7 +226,7 @@ static class RestoreZooCommand {

@Parameters(commandNames = "fate",
commandDescription = "Operations performed on the Manager FaTE system.")
static class FateOpsCommand {
static class FateOpsCommand extends SubCommandOpts {
@Parameter(description = "[<txId>...]")
List<String> txList = new ArrayList<>();

Expand Down Expand Up @@ -256,7 +258,7 @@ static class FateOpsCommand {
}

@Parameters(commandDescription = "show service status")
public static class ServiceStatusCmdOpts {
public static class ServiceStatusCmdOpts extends SubCommandOpts {
@Parameter(names = "--json", description = "provide output in json format (--noHosts ignored)")
boolean json = false;
@Parameter(names = "--noHosts",
Expand Down Expand Up @@ -288,7 +290,7 @@ public String description() {
public void execute(final String[] args) {
boolean everything;

AdminOpts opts = new AdminOpts();
ServerUtilOpts opts = new ServerUtilOpts();
JCommander cl = new JCommander(opts);
cl.setProgramName("accumulo admin");

Expand Down Expand Up @@ -346,11 +348,19 @@ public void execute(final String[] args) {

cl.parse(args);

if (opts.help || cl.getParsedCommand() == null) {
if (cl.getParsedCommand() == null) {
cl.usage();
return;
}

if (opts.help || listInstancesOpts.help || pingCommand.help || checkTabletsCommand.help
|| stopOpts.help || dumpConfigCommand.help || volumesCommand.help
|| verifyTabletAssignmentsOpts.help || deleteZooInstOpts.help || restoreZooOpts.help
|| fateOpsCommand.help || tServerLocksOpts.help || serviceStatusCommandOpts.help) {
cl.getCommands().get(cl.getParsedCommand()).usage();
return;
}

ServerContext context = opts.getServerContext();

AccumuloConfiguration conf = context.getConfiguration();
Expand Down Expand Up @@ -389,7 +399,7 @@ public void execute(final String[] args) {
}

} else if (cl.getParsedCommand().equals("stop")) {
stopTabletServer(context, stopOpts.args, opts.force);
stopTabletServer(context, stopOpts.args, stopOpts.force);
} else if (cl.getParsedCommand().equals("dumpConfig")) {
printConfig(context, dumpConfigCommand);
} else if (cl.getParsedCommand().equals("volumes")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ private static void writeLine(BufferedWriter w, String value) {
public void execute(String[] args) throws Exception {
Opts opts = new Opts();
opts.parseArgs("accumulo convert-config", args);
opts.printUsage(opts.help);

File xmlFile = new File(opts.xmlPath);
if (!xmlFile.exists()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ static class Opts extends ConfigOpts {
public void execute(String[] args) throws KeeperException, InterruptedException {
Opts opts = new Opts();
opts.parseArgs(DumpZookeeper.class.getName(), args);
opts.printUsage(opts.help);

PrintStream out = System.out;
zk = new ZooReaderWriter(opts.getSiteConfiguration());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ static class Opts extends Help {

public static void main(String[] args) {
opts.parseArgs(ListInstances.class.getName(), args);
opts.printUsage(opts.help);

if (opts.keepers == null) {
var siteConfig = SiteConfiguration.auto();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ public static void main(String[] args) throws Exception {
Opts opts = new Opts();
opts.principal = "root";
opts.parseArgs(RandomWriter.class.getName(), args);
opts.printUsage(opts.help);

Span span = TraceUtil.startSpan(RandomWriter.class, "main");
try (Scope scope = span.makeCurrent()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ static int checkTable(ServerContext context, String tableName, boolean fix) thro
public static void main(String[] args) throws Exception {
Opts opts = new Opts();
opts.parseArgs(RemoveEntriesForMissingFiles.class.getName(), args);
opts.printUsage(opts.help);
Span span = TraceUtil.startSpan(RemoveEntriesForMissingFiles.class, "main");
try (Scope scope = span.makeCurrent()) {
checkAllTables(opts.getServerContext(), opts.fix);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ static class Opts extends ServerUtilOpts {
public static void main(String[] args) throws Exception {
Opts opts = new Opts();
opts.parseArgs(TableDiskUsage.class.getName(), args);
opts.printUsage(opts.help);
Span span = TraceUtil.startSpan(TableDiskUsage.class, "main");
try (Scope scope = span.makeCurrent()) {
try (AccumuloClient client = Accumulo.newClient().from(opts.getClientProps()).build()) {
Expand Down
Loading

0 comments on commit 173957f

Please sign in to comment.