ZOOKEEPER-4656: Allow ZooKeeperAdmin creation with custom HostProvider - #1947
ZOOKEEPER-4656: Allow ZooKeeperAdmin creation with custom HostProvider#1947jowiho wants to merge 1 commit into
Conversation
eolivelli
left a comment
There was a problem hiding this comment.
I support this idea.
I have a couple of requests on this patch:
- I created this JIRA, can you please update the commit message and the PR title ? https://issues.apache.org/jira/browse/ZOOKEEPER-4656
- can you please add a test ?
- Add a new overloaded constructor will add some tech debt. Would you please think about adding a Builder API for ZooKeeperAdmin ? this way in the future it will be simpler to add more customisation points
4220dfe to
551ea5a
Compare
|
Thanks for your feedback @eolivelli, and for creating the JIRA issue. I've added the JIRA ID to the PR title and commit message.
I've considered it, but I couldn't come up with a good way to test it. There currently are no other tests for
I agree that a Builder API would be a very useful addition. But perhaps, in order to keep the PR small, that would better be done in a separate PR? |
|
I would prefer to not commit the new constructor and then remove it. For the test, we can a very simple test that bootstraps an instance using the new builder. |
|
Superseded by #2001 |
It's currently not possible to create a ZooKeeperAdmin instance with a custom HostProvider because the ZooKeeperAdmin doesn't expose the necessary ZooKeeper base class constructor. To make this possible, this PR adds ZooKeeperAdmin constructor that passes a given HostProvider parameter to the ZooKeeper base class constructor.