pkuwm commented on a change in pull request #819: Make RealmAwareZkClient 
implementations use HttpRoutingDataReader for routing data
URL: https://github.com/apache/helix/pull/819#discussion_r387445166
 
 

 ##########
 File path: 
zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/FederatedZkClient.java
 ##########
 @@ -80,19 +83,17 @@
   private PathBasedZkSerializer _pathBasedZkSerializer;
 
   // TODO: support capacity of ZkClient number in one FederatedZkClient and do 
garbage collection.
-  public FederatedZkClient(RealmAwareZkClient.RealmAwareZkClientConfig 
clientConfig,
-      MetadataStoreRoutingData metadataStoreRoutingData) {
-    if (metadataStoreRoutingData == null) {
-      throw new IllegalArgumentException("MetadataStoreRoutingData cannot be 
null!");
-    }
+  public FederatedZkClient(RealmAwareZkClient.RealmAwareZkClientConfig 
clientConfig)
+      throws IOException, InvalidRoutingDataException {
 
 Review comment:
   Like I said, personally, hiding failure type in this case is more 
user-friendly than floating these exceptions up. And I also understand and 
respect your opinion.
   
    I am not sure how Helix Java APIs could handle these 2 exceptions 
differently when constructing this FederatedZkClient. Regarding a few other 
users, we could get opinions from them. I will be OK if we can get options from 
other users/committers and they think floating exceptions are better :)

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to