ctubbsii commented on a change in pull request #2273:
URL: https://github.com/apache/accumulo/pull/2273#discussion_r717159758



##########
File path: 
core/src/main/java/org/apache/accumulo/core/rpc/UGIAssumingTransportFactory.java
##########
@@ -46,7 +47,13 @@ public UGIAssumingTransportFactory(TTransportFactory 
wrapped, UserGroupInformati
   }
 
   @Override
-  public TTransport getTransport(final TTransport trans) {
-    return ugi.doAs((PrivilegedAction<TTransport>) () -> 
wrapped.getTransport(trans));
+  public TTransport getTransport(final TTransport trans) throws 
TTransportException {
+    return ugi.doAs((PrivilegedAction<TTransport>) () -> {
+      try {
+        return wrapped.getTransport(trans);
+      } catch (TTransportException e) {
+        throw new RuntimeException(e);
+      }
+    });

Review comment:
       Does this method even throw TTransportException now? It was added to the 
method signature, but seems like it doesn't throw, because it's always wrapped 
with RTE if it happens.

##########
File path: test/src/main/java/org/apache/accumulo/test/rpc/Mocket.java
##########
@@ -141,13 +143,13 @@ public MocketServerTransport(MocketTransport 
mocketTransport) {
     public void listen() {}
 
     @Override
-    public void close() {
-      acceptImpl().close();
+    public TTransport accept() throws TTransportException {
+      return null;

Review comment:
       Shouldn't this return `servTrans`?




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