accumulo-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] ctubbsii commented on a change in pull request #419: #408 - Removed ClientConfiguration from ClientOpts
Date Tue, 10 Apr 2018 22:28:51 GMT
ctubbsii commented on a change in pull request #419:  #408 - Removed ClientConfiguration from
ClientOpts
URL: https://github.com/apache/accumulo/pull/419#discussion_r180586097
 
 

 ##########
 File path: server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnRequiredTable.java
 ##########
 @@ -36,6 +35,10 @@ synchronized public Instance getInstance() {
     if (instance == null) {
       return cachedInstance = HdfsZooInstance.getInstance();
     }
-    return cachedInstance = new ZooKeeperInstance(getClientConfiguration());
+    try {
+      return cachedInstance = getConnector().getInstance();
+    } catch (Exception e) {
+      throw new RuntimeException(e);
 
 Review comment:
   This is a bad pattern to follow, for a few reasons:
   
   1. We have multi-catch blocks now, so there's no reason to do blanket catch like this,
   2. This will unnecessarily wrap `RuntimeException`s with another `RuntimeException`, and
   3. You should never call `new RuntimeException(...)`; instead, pick a specific runtime
exception appropriate to the circumstance.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message