kaisun2000 commented on a change in pull request #770: WIP: Add SharedZkClient and update SharedZkClientFactory URL: https://github.com/apache/helix/pull/770#discussion_r380868468
########## File path: zookeeper-api/src/main/java/org/apache/helix/zookeeper/api/client/HelixZkClient.java ########## @@ -89,32 +88,17 @@ public int getSessionTimeout() { * Deprecated - please use RealmAwareZkClient and RealmAwareZkClientConfig instead. * * Configuration for creating a new HelixZkClient with serializer and monitor. - * - * TODO: If possible, try to merge with RealmAwareZkClient's RealmAwareZkClientConfig to reduce duplicate logic/code (without breaking backward-compatibility). - * Simply making this a subclass of RealmAwareZkClientConfig will break backward-compatiblity. */ @Deprecated - class ZkClientConfig { - // For client to init the connection - private long _connectInitTimeout = DEFAULT_CONNECTION_TIMEOUT; - - // Data access configs - private long _operationRetryTimeout = DEFAULT_OPERATION_TIMEOUT; - - // Others - private PathBasedZkSerializer _zkSerializer; - - // Monitoring - private String _monitorType; - private String _monitorKey; - private String _monitorInstanceName = null; - private boolean _monitorRootPathOnly = true; + class ZkClientConfig extends RealmAwareZkClientConfig { + @Override Review comment: Why do we need to re-implement these methods if you inherit from the RealmAwareZkClientConfig? ---------------------------------------------------------------- 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