narendly commented on a change in pull request #899: Use Java Generics and
inheritance to reduce duplicate code in Helix API Builders
URL: https://github.com/apache/helix/pull/899#discussion_r395846744
##########
File path:
helix-core/src/main/java/org/apache/helix/tools/ClusterVerifiers/ZkHelixClusterVerifier.java
##########
@@ -384,14 +373,26 @@ protected void validate() {
+ _zkAddress + " RealmAwareZkConnectionConfig: " +
_realmAwareZkConnectionConfig);
}
}
+ initializeConfigsIfNull();
+ }
- // Resolve all default values
- if (_realmAwareZkConnectionConfig == null) {
- _realmAwareZkConnectionConfig =
- new
RealmAwareZkClient.RealmAwareZkConnectionConfig.Builder().build();
- }
- if (_realmAwareZkClientConfig == null) {
- _realmAwareZkClientConfig = new
RealmAwareZkClient.RealmAwareZkClientConfig();
+ /**
+ * Creates a RealmAwareZkClient for ZkHelixClusterVerifiers.
+ * Note that DedicatedZkClient is used whether it's multi-realm or
single-realm.
+ * @return
+ */
+ private RealmAwareZkClient createZkClientFromBuilderForVerifier() {
Review comment:
@jiajunwang Because this would cause the constructor to take in a list of
parameters defined by its parent builder's createZkClient(). I'd argue that
it's not confusing because the method name makes is 100% clear - there really
is no ambiguity.
For ZkHelixClusterVerifier and its implementations, they, by definition,
only operate on single realm mode on a dedicated zk client. As such, that
degree of customization that would be required in the parent builder's
createZkClient() is not necessary.
Is it a good idea to have unnecessary parameters in a method just for the
sake of overriding a parent method, especially when it could be avoided?
Again, this is a stylistic choice, and for the sake of moving this forward,
I will make that change to make the method override its parent's method - that
way we would be on the same page.
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
