Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default read ZK servers from cfg file #18

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alexbb
Copy link

@alexbb alexbb commented Mar 8, 2017

If no servers are specified and a config /etc/zookeeper/conf/zoo.cfg exists, read the config file for a list of servers instead of defaulting to localhost:2181. Config location is default for Cloudera installed ZK.

@phunt
Copy link
Owner

phunt commented Mar 22, 2017

Thanks @alexbb this looks like a great feature. I'm torn though, while I like the idea (and would even be open for it to scan a list of directories/files rather than a single set file, i.e. it could then have a few common default locations listed) I don't like the fact that it changes default behavior. Also even if you specify the server option on the command line it might still override.

Possible to add a config option, or perhaps if the user specifies "-c" without an argument then it looks for the file? I think that would be a better way to go.

@alexbb
Copy link
Author

alexbb commented Mar 22, 2017

I had many of the same concerns when writing it, so thank you for your feedback. I think I can work around the override of a command line specified server (would only happen with exactly localhost:2181, but still) by setting default=None on the server option then explicitly checking it later. I'll try to have a revised branch ready for you in the next couple days for this, and will await your feedback on the rest of the items below.

Checking a list of common locations is a great idea. Do you have suggestions for other locations to check, and what you would like to see in terms of check order? I've only ever used CDH installed ZK, but I can imagine looking for configs under /usr/local/zookeeper/conf/ or /opt/zookeeper/conf/.

Changing the default behavior was my biggest concern. I reasoned thusly, let me know if you find it compelling or if you'd rather leave the config search behavior as a non-default option:

  1. In the case of no config files it falls back to the old behavior.
  2. In the case of a config file but a single server (testing/dev installs) the effective change is nil. Config gets read but the config only has a single ZK in it.
  3. In the case of a config file and a ZK cluster, non-default ports, or remote hosts, I feel that the new behavior is nicer for administrators.

@phunt
Copy link
Owner

phunt commented Mar 22, 2017

I think I can work around the override of a command line specified server

Instead of any workarounds the simplest approach would be to just add a new config option "--scanForConfigs" or somesuch thing. Why not do that? Or as I mentioned overload "-c", where no args mean scan. I really like the "zktop -c" approach, not sure if optparse supports it though...

other locations?

I don't know. I've only ever really used CDH myself. :-)

defaults

the one i thought about was where someone was using it locally, but for whatever reason the individual, or someone else, has installed an unrelated config file into one of the default locations. Or maybe they are just logging into a particular zk server and want to check that particular server. Basically I was going back to "least surprize" esp if the alternative is to expose it as another option...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants