[GitHub] [accumulo] ctubbsii commented on a change in pull request #2273: Upgrade Thrift to 0.15.0

[GitBox <gi...@apache.org>]

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: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org