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 scope problem for gmetad config #27

Merged
merged 1 commit into from
Nov 25, 2014
Merged

Fix scope problem for gmetad config #27

merged 1 commit into from
Nov 25, 2014

Conversation

NoodlesNZ
Copy link
Contributor

With future parser there is a scope problem in the gmetad.conf.erb. Because the variable "clusters" is defined in ganglia::gmetad and the template is in ganglia::gmetad::config it is not in the same scope. By inheriting ganglia::gmetad this fixes the problem.

@NoodlesNZ
Copy link
Contributor Author

Also found the problem in ganglia::web::config (port/ip from ganglia::web not in the scope that the template is in)

@jhoblitt
Copy link
Owner

Inheriting the base class to get at it's variables is a bit of an anti-pattern in puppet. A better approach would be to use scope.lookupvar in the ERB templates. See: https://docs.puppetlabs.com/guides/templating.html#out-of-scope-variables

EPP templates won't have this problem but it's going to be awhile before we can rely on puppet 4 features.

@jhoblitt
Copy link
Owner

BTW - the scope[] form would be fine. I'm OK with dropping 2.7.x support.

@NoodlesNZ
Copy link
Contributor Author

I'm open to suggestions, I wasn't keen on using inherits, but I figured it's already being used elsewhere in puppet-ganglia so it should be ok for now.

I can refactor with scope[] if that's the way you want to go

@NoodlesNZ
Copy link
Contributor Author

I started rewriting it to use scope[], but EPEL6 is still on puppet 2.7.x. Might be worth hanging on to 2.7.x support for a while.

@jhoblitt
Copy link
Owner

Sounds completely reasonable. Supposedly EPEL6 is going to stay with
2.7.x forever.

@NoodlesNZ
Copy link
Contributor Author

Ok, I'll use scope.lookupvar() instead.

@jhoblitt
Copy link
Owner

Thanks for working on this!

jhoblitt pushed a commit that referenced this pull request Nov 25, 2014
Fix scope problem for gmetad config
@jhoblitt jhoblitt merged commit 63cad00 into jhoblitt:master Nov 25, 2014
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