ctubbsii commented on code in PR #3440:
URL: https://github.com/apache/accumulo/pull/3440#discussion_r1223305274


##########
shell/src/main/java/org/apache/accumulo/shell/Shell.java:
##########
@@ -189,6 +189,16 @@
  */
 @AutoService(KeywordExecutable.class)
 public class Shell extends ShellOptions implements KeywordExecutable {
+
+  public static interface Authenticator {
+    default boolean authenticateUser(AccumuloClient client, String principal,
+        AuthenticationToken token) throws AccumuloException, 
AccumuloSecurityException {
+      return client.securityOperations().authenticateUser(principal, token);
+    }
+  }
+
+  public static Authenticator AUTHENTICATOR = new Authenticator() {};

Review Comment:
   Note: FateCommandTest is removed in 3.0, since the FateCommand is deprecated 
in 2.1 and removed in 3.0.
   
   I think that making this static is a confusing way to implement this for 
testing, though. I think it would be better to use a protected method and 
create a subclass test shell in FateCommandTest.createShell that overrides this 
method for testing. That way, there's less public and less static stuff leaking 
out of the shell's API for users to shoot themselves in the foot with.
   
   I can add a commit to this PR or submit a PR to your branch to demonstrate 
my suggestion, if you want. Whichever you'd prefer.



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