narendly commented on a change in pull request #1183:
URL: https://github.com/apache/helix/pull/1183#discussion_r463228808
##########
File path: helix-core/src/main/java/org/apache/helix/HelixManagerFactory.java
##########
@@ -65,4 +64,39 @@ public static HelixManager getZKHelixManager(String
clusterName, String instance
return new ZKHelixManager(clusterName, instanceName, type, zkAddr,
stateListener);
}
+ /**
+ * Construct a ZkHelixManager using the HelixManagerProperty instance given.
+ * HelixManagerProperty given must contain a valid ZkConnectionConfig.
+ * @param clusterName
+ * @param instanceName
+ * @param type
+ * @param stateListener
+ * @param helixManagerProperty must contain a valid ZkConnectionConfig
+ * @return
+ */
+ public static HelixManager getZKHelixManager(String clusterName, String
instanceName,
+ InstanceType type, HelixManagerStateListener stateListener,
+ HelixManagerProperty helixManagerProperty) {
+ return new ZKHelixManager(clusterName, instanceName, type, null,
stateListener,
+ helixManagerProperty);
+ }
+
+ /**
+ * Construct a ZkHelixManager using the HelixManagerProperty instance given.
+ * NOTE: if both zkAddr and a valid ZkConnectionConfig are given in
HelixManagerProperty, the
+ * instantiation will fail - only one is required.
+ * @param clusterName
+ * @param instanceName
+ * @param type
+ * @param zkAddr
+ * @param stateListener
+ * @param helixManagerProperty
+ * @return
+ */
+ public static HelixManager getZKHelixManager(String clusterName, String
instanceName,
+ InstanceType type, String zkAddr, HelixManagerStateListener
stateListener,
+ HelixManagerProperty helixManagerProperty) {
Review comment:
As to whether we should "move" `zkAddress` into helixManagerProperty -
this idea is not ideal. It is what we have now and we should keep it in the
constructor for backward-compatibility, and giving users multiple ways to input
`zkAddress` will undoubtedly confuse users.
So I think the right thing to do here is to keep zkAddress in
HelixManager/HelixManagerFactory as a parameter. It won't continue to increase
getZkHelixManager() methods. All future configs that are needed to generate an
instance of HelixManager could go into `helixManagerProperty`.
----------------------------------------------------------------
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
