[GitHub] [helix] dasahcc commented on a change in pull request #1127: Fix validation logic in GenericApiBuilders and add usage tests

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

dasahcc commented on a change in pull request #1127:
URL: https://github.com/apache/helix/pull/1127#discussion_r448541950



##########
File path: helix-core/src/main/java/org/apache/helix/manager/zk/GenericBaseDataAccessorBuilder.java
##########
@@ -82,38 +82,28 @@ protected RealmAwareZkClient createZkClient(RealmAwareZkClient.RealmMode realmMo
     switch (realmMode) {
       case MULTI_REALM:
         try {
-          if (_zkClientType == ZkClientType.DEDICATED) {
-            // Use a realm-aware dedicated zk client
-            zkClient = DedicatedZkClientFactory.getInstance()
-                .buildZkClient(connectionConfig, clientConfig);
-          } else if (_zkClientType == ZkClientType.SHARED) {
-            // Use a realm-aware shared zk client
-            zkClient =
-                SharedZkClientFactory.getInstance().buildZkClient(connectionConfig, clientConfig);
-          } else {
-            zkClient = new FederatedZkClient(connectionConfig, clientConfig);
-          }
-        } catch (IOException | InvalidRoutingDataException | IllegalStateException e) {
+          zkClient = new FederatedZkClient(connectionConfig, clientConfig);

Review comment:
       If you think this is a bug we should fix, then we need to make clear definition of what is single realm and what is multi realm. My original thought was if you are using sharding key to determine the ZK address then it is mutli realm.
   
   Then for dedicated zkclient, and shared zkclient, they can still use sharding key for searching zk address. Then it will be meaningless for dedicated/shared Zkclient to have the sharding key and they will only accept the ZK address. This conflict with what we have changed.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org