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

kafka_consumer : added an option to change the kafka path in zookeeper #776

Closed
wants to merge 2 commits into from

Conversation

prune998
Copy link
Contributor

@prune998 prune998 commented Mar 1, 2016

added a plugin option zookeeper_chroot to set up the kafka endpoint in zookeeper, which may not be / (default).
This chroot is then configured in the consumergroup config.Zookeeper.Chroot

This is workaround the fact that this plugins does not handle the urls like "zookeeper_server:port/chroot"
As the peers are stored in an array, it makes no sens to have them beeing URL. Peers should all be members of the same cluster, so they all have the same chroot.

added a plugin option zookeeper_chroot to set up the kafka endpoint in zookeeper, which may not be / (default).
This chroot is then configured in the consumergroup config.Zookeeper.Chroot

This is workaround the fact that this plugins does not handle the urls like    "zookeeper_server:port/chroot"
As the peers are stored in an array, it makes no sens to have them beeing URL. Peers should all be members of the same cluster, so they all have the same chroot.
@prune998
Copy link
Contributor Author

prune998 commented Mar 2, 2016

Can someone please help with the failed check ?
I'm no GO expert and I just changed 2 lines in the code...

This is a critical bug for us as our zookeeper is not dedicated to Kafka, which is chrooted under "/kafka".

@tripledes
Copy link

@prune998 The test failed because of go fmt, after your 2nd commit where you fixed the issues, the test passes.

@sparrc sparrc closed this in bd3d0c3 Mar 7, 2016
geodimm pushed a commit to miketonks/telegraf that referenced this pull request Mar 10, 2016
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