desaikomal commented on code in PR #2099:
URL: https://github.com/apache/helix/pull/2099#discussion_r873899781
##########
helix-core/src/test/java/org/apache/helix/integration/manager/ClusterControllerManager.java:
##########
@@ -37,6 +38,10 @@ public ClusterControllerManager(String zkAddr, String
clusterName, String contro
super(zkAddr, clusterName, controllerName, InstanceType.CONTROLLER);
}
+ public ClusterControllerManager(String clusterName, HelixManagerProperty
helixManagerProperty) {
Review Comment:
I see that you added methods to handle the new additional parameter, but I
don't see any tests to see whether we are using the helixManagerProperty values
in the form of ASSERTs etc.
Is there any actual test cases to test it?
##########
helix-core/src/main/java/org/apache/helix/HelixPropertyFactory.java:
##########
@@ -80,20 +80,23 @@ public HelixManagerProperty getHelixManagerProperty(String
zkAddress, String clu
* @param clusterName
* @return
*/
- public static CloudConfig getCloudConfig(String zkAddress, String
clusterName) {
+ public static CloudConfig getCloudConfig(String zkAddress, String
clusterName,
+ RealmAwareZkClient.RealmAwareZkConnectionConfig
realmAwareZkConnectionConfig) {
Review Comment:
Is there a possibility that the supplied realmAwareZkConnectionConfig
doesn't have everything populated as the default builder?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
