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

fix role specification for elasticsearch and graphite #22

Merged
merged 1 commit into from
Sep 16, 2014

Conversation

jwmarshall
Copy link

This change fixes the ability to dynamically configure the remote end points for graphite and elasticsearch. I could not figure out how the original method was supposed to work given that the es_server and graphite_server had default variables defined in attributes/default.rb

@JonathanTron
Copy link
Contributor

Thanks for the pull-request.
What is expected is that once you want to use the roles you set:

override['grafana']['es_server'] = nil
override['grafana']['graphite_server'] = nil

and either use the defaults node['grafana']['es_role']/node['grafana']['graphite_role'] or change them to match yours.

I think your change is fine and will apply it with a warning in the README that it will now search for the es_role/graphite_role by default even if you set the es_server/graphite_server and will replace the defaults.

JonathanTron added a commit that referenced this pull request Sep 16, 2014
fix role specification for elasticsearch and graphite
@JonathanTron JonathanTron merged commit 134878f into sous-chefs:master Sep 16, 2014
@jwmarshall
Copy link
Author

Jonathan, that makes a little more sense as setting the default to nil did not work for me. I guess using override or normal even would have worked. However the override of each server to nil feels very strange when you're reading the environment/role attributes. I think the update you've merge makes the most sense, but I'm probably biased to that as I wrote the pull request.

Never-the-less thank you for a great cookbook and merging my updates. 👍

@lock
Copy link

lock bot commented Jul 24, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jul 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants