This is an automated email from the ASF dual-hosted git repository.
ctubbsii pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/2.1 by this push:
new ac2886c34e Remove direct use of System.out in shell (#4154)
ac2886c34e is described below
commit ac2886c34ee5b28b704bce2877d3952cb265a40b
Author: Christopher Tubbs <ctubb...@apache.org>
AuthorDate: Tue Jan 23 16:56:18 2024 -0500
Remove direct use of System.out in shell (#4154)
* Show shell config load info as a log message, so it can be suppressed
from output more easily (depending on logging config)
* Throw command line parse exception when an invalid shell option is
used in FateCommand, instead of merely printing to System.out and
returning
* Use shellState.getWriter() for command-output messages instead of
hard-coded System.out (use System.out for related unit tests)
---
.../org/apache/accumulo/shell/ShellOptionsJC.java | 2 +-
.../accumulo/shell/commands/FateCommand.java | 23 +++++++++++-----------
.../accumulo/shell/commands/FateCommandTest.java | 14 +++++++------
3 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/shell/src/main/java/org/apache/accumulo/shell/ShellOptionsJC.java
b/shell/src/main/java/org/apache/accumulo/shell/ShellOptionsJC.java
index cb006fd438..0b4822caa1 100644
--- a/shell/src/main/java/org/apache/accumulo/shell/ShellOptionsJC.java
+++ b/shell/src/main/java/org/apache/accumulo/shell/ShellOptionsJC.java
@@ -201,7 +201,7 @@ public class ShellOptionsJC {
File file = new File(path);
if (file.isFile() && file.canRead()) {
clientConfigFile = file.getAbsolutePath();
- System.out.println("Loading configuration from " + clientConfigFile);
+ Shell.log.info("Loading configuration from {}", clientConfigFile);
break;
}
}
diff --git
a/shell/src/main/java/org/apache/accumulo/shell/commands/FateCommand.java
b/shell/src/main/java/org/apache/accumulo/shell/commands/FateCommand.java
index d0e82b8b88..9ac7342d97 100644
--- a/shell/src/main/java/org/apache/accumulo/shell/commands/FateCommand.java
+++ b/shell/src/main/java/org/apache/accumulo/shell/commands/FateCommand.java
@@ -22,6 +22,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
import static org.apache.accumulo.core.fate.FateTxId.parseTidFromUserInput;
import java.io.IOException;
+import java.io.PrintWriter;
import java.lang.reflect.Type;
import java.util.ArrayList;
import java.util.Base64;
@@ -159,25 +160,25 @@ public class FateCommand extends Command {
if (cl.hasOption(cancel.getOpt())) {
String[] txids = cl.getOptionValues(cancel.getOpt());
validateArgs(txids);
- System.out.println(
+ throw new ParseException(
"Option not available. Use 'accumulo admin fate -c " + String.join("
", txids) + "'");
} else if (cl.hasOption(fail.getOpt())) {
String[] txids = cl.getOptionValues(fail.getOpt());
validateArgs(txids);
- failedCommand = failTx(admin, zs, zk, managerLockPath, txids);
+ failedCommand = failTx(shellState.getWriter(), admin, zs, zk,
managerLockPath, txids);
} else if (cl.hasOption(delete.getOpt())) {
String[] txids = cl.getOptionValues(delete.getOpt());
validateArgs(txids);
- failedCommand = deleteTx(admin, zs, zk, managerLockPath, txids);
+ failedCommand = deleteTx(shellState.getWriter(), admin, zs, zk,
managerLockPath, txids);
} else if (cl.hasOption(list.getOpt())) {
printTx(shellState, admin, zs, zk, tableLocksPath,
cl.getOptionValues(list.getOpt()), cl);
} else if (cl.hasOption(print.getOpt())) {
printTx(shellState, admin, zs, zk, tableLocksPath,
cl.getOptionValues(print.getOpt()), cl);
} else if (cl.hasOption(summary.getOpt())) {
- System.out.println("Option not available. Use 'accumulo admin fate
--summary'");
+ throw new ParseException("Option not available. Use 'accumulo admin fate
--summary'");
} else if (cl.hasOption(dump.getOpt())) {
String output = dumpTx(zs, cl.getOptionValues(dump.getOpt()));
- System.out.println(output);
+ shellState.getWriter().println(output);
} else {
throw new ParseException("Invalid command option");
}
@@ -234,14 +235,14 @@ public class FateCommand extends Command {
!cl.hasOption(disablePaginationOpt.getOpt()));
}
- protected boolean deleteTx(AdminUtil<FateCommand> admin,
ZooStore<FateCommand> zs,
- ZooReaderWriter zk, ServiceLockPath zLockManagerPath, String[] args)
+ protected boolean deleteTx(PrintWriter out, AdminUtil<FateCommand> admin,
+ ZooStore<FateCommand> zs, ZooReaderWriter zk, ServiceLockPath
zLockManagerPath, String[] args)
throws InterruptedException, KeeperException {
for (int i = 1; i < args.length; i++) {
if (admin.prepDelete(zs, zk, zLockManagerPath, args[i])) {
admin.deleteLocks(zk, zLockManagerPath, args[i]);
} else {
- System.out.printf("Could not delete transaction: %s%n", args[i]);
+ out.printf("Could not delete transaction: %s%n", args[i]);
return false;
}
}
@@ -254,12 +255,12 @@ public class FateCommand extends Command {
}
}
- public boolean failTx(AdminUtil<FateCommand> admin, ZooStore<FateCommand>
zs, ZooReaderWriter zk,
- ServiceLockPath managerLockPath, String[] args) {
+ public boolean failTx(PrintWriter out, AdminUtil<FateCommand> admin,
ZooStore<FateCommand> zs,
+ ZooReaderWriter zk, ServiceLockPath managerLockPath, String[] args) {
boolean success = true;
for (int i = 1; i < args.length; i++) {
if (!admin.prepFail(zs, zk, managerLockPath, args[i])) {
- System.out.printf("Could not fail transaction: %s%n", args[i]);
+ out.printf("Could not fail transaction: %s%n", args[i]);
return !success;
}
}
diff --git
a/shell/src/test/java/org/apache/accumulo/shell/commands/FateCommandTest.java
b/shell/src/test/java/org/apache/accumulo/shell/commands/FateCommandTest.java
index 379dd5b5b1..856c7cb7e9 100644
---
a/shell/src/test/java/org/apache/accumulo/shell/commands/FateCommandTest.java
+++
b/shell/src/test/java/org/apache/accumulo/shell/commands/FateCommandTest.java
@@ -33,6 +33,7 @@ import java.io.FileDescriptor;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.PrintStream;
+import java.io.PrintWriter;
import java.nio.file.Files;
import java.util.List;
import java.util.concurrent.TimeUnit;
@@ -96,15 +97,15 @@ public class FateCommandTest {
}
@Override
- protected boolean deleteTx(AdminUtil<FateCommand> admin,
ZooStore<FateCommand> zs,
- ZooReaderWriter zk, ServiceLockPath zLockManagerPath, String[] args)
- throws InterruptedException, KeeperException {
+ protected boolean deleteTx(PrintWriter out, AdminUtil<FateCommand> admin,
+ ZooStore<FateCommand> zs, ZooReaderWriter zk, ServiceLockPath
zLockManagerPath,
+ String[] args) throws InterruptedException, KeeperException {
deleteCalled = true;
return true;
}
@Override
- public boolean failTx(AdminUtil<FateCommand> admin, ZooStore<FateCommand>
zs,
+ public boolean failTx(PrintWriter out, AdminUtil<FateCommand> admin,
ZooStore<FateCommand> zs,
ZooReaderWriter zk, ServiceLockPath managerLockPath, String[] args) {
failCalled = true;
return true;
@@ -154,9 +155,10 @@ public class FateCommandTest {
FateCommand cmd = new FateCommand();
// require number for Tx
- assertFalse(cmd.failTx(helper, zs, zk, managerLockPath, new String[]
{"fail", "tx1"}));
+ var out = new PrintWriter(System.out);
+ assertFalse(cmd.failTx(out, helper, zs, zk, managerLockPath, new String[]
{"fail", "tx1"}));
// fail the long configured above
- assertTrue(cmd.failTx(helper, zs, zk, managerLockPath, new String[]
{"fail", "12345"}));
+ assertTrue(cmd.failTx(out, helper, zs, zk, managerLockPath, new String[]
{"fail", "12345"}));
verify(zs);
}
