narendly commented on a change in pull request #745: [WIP] Create
RealmAwareZkClientFactory interface
URL: https://github.com/apache/helix/pull/745#discussion_r378591083
##########
File path:
zookeeper-api/src/main/java/org/apache/helix/zookeeper/api/api/factory/RealmAwareZkClientFactory.java
##########
@@ -19,5 +19,14 @@
* under the License.
*/
+import org.apache.helix.zookeeper.api.api.client.RealmAwareZkClient;
+
+
+/**
+ * Creates an instance of RealmAwareZkClient.
+ */
public interface RealmAwareZkClientFactory {
+
+ RealmAwareZkClient buildZkClient(RealmAwareZkClient.MODE mode,
+ RealmAwareZkClient.RealmAwareZkClientConfig config);
Review comment:
@kaisun2000
1. Yes, we should add the sessionTimeout field to this. I will add it for
completeness (this was a draft, so I just wanted to first give you an idea). :)
2. Yes and no.
- RealmAwareZkClients produced by RealmAwareZkClientFactory are all
single-realm mode. So yes, mode might not be required as a method parameter for
`buildZkClient()`.
- We should still keep this field internally because not all
RealmAwareZkClients are single-realm mode. For example, if you create a
FederatedZkClient instance, it should still have a `mode = MULTI_REALM`.
So yes, let's follow your suggestion to remove the mode parameter from here,
but we should keep it as an internal variable that we could expose to the user
(for instance, RealmAwareZkClient should support an interface method like
`getMode()` that returns either `SINGLE_REALM` or `MULTI_REALM`)
----------------------------------------------------------------
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]
