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 indent bug in jmx.yaml.erb, add test #93

Merged
merged 3 commits into from Mar 19, 2014

Conversation

Projects
None yet
3 participants
@clofresh
Copy link
Contributor

commented Jan 28, 2014

domain, bean and attribute should be children of include and exclude, not siblings. See http://docs.datadoghq.com/integrations/java/

@ghost ghost assigned miketheman Jan 28, 2014

@miketheman

This comment has been minimized.

Copy link
Collaborator

commented Jan 28, 2014

Looks to be a dupe of #88 (without the tests part) - can you confirm?

@clofresh

This comment has been minimized.

Copy link
Contributor Author

commented Jan 28, 2014

Yeah, #88 fixes the jmx indent. The tests are good to have. Also, I made name, username and password optional attributes

@miketheman

This comment has been minimized.

Copy link
Collaborator

commented Jan 28, 2014

Ok, I'll merge that in first, then grab this one and get the goods. when I work on this next week.
Thanks!

@clofresh

This comment has been minimized.

Copy link
Contributor Author

commented Jan 28, 2014

Big thanks to @ajohnstone for helping debug this! 🍔

@ajohnstone

This comment has been minimized.

Copy link

commented Jan 28, 2014

Whilst looking into this, I was curious why the yaml templates are hard coded pre-formatted templates.
Surely this makes them more error prone? Granted writing out yaml files strips comments, however comments could easily be injected.

@clofresh

This comment has been minimized.

Copy link
Contributor Author

commented Jan 29, 2014

Yeah ideally we'd just do a node['jmx'].to_yaml and write it to the file, but unfortunately ruby 1.8 doesn't have a stable sort order for their hashes, so to_yaml can potentially be different for the same set of attributes, causing chef to think it's new config and restart the agent when it doesn't need to.

@miketheman miketheman merged commit 829fda3 into master Mar 19, 2014

1 check failed

default The Travis CI build failed
Details
@miketheman

This comment has been minimized.

Copy link
Collaborator

commented Mar 19, 2014

Thanks @clofresh and @ajohnstone for working through this - the changes will be in the next cookbook release.

miketheman added a commit that referenced this pull request Mar 19, 2014

@miketheman miketheman deleted the fix-jmx-yaml branch Mar 20, 2014

n1koo pushed a commit to Shopify/chef-datadog that referenced this pull request Oct 20, 2014

miketheman added a commit that referenced this pull request Dec 30, 2014

Render jmx.yaml template based on data input
After multiple passes at attempting to cover all cases of inclusion of instance
details into a given YAML file, this lays the foundation for leaving the details
up to the end-user to implement the instance hash, instead of trying to capture
each possible flag.
This should help future-proof the recipe and template, so any new flags can be
appended as data.

See #88, #93 and fixes #116, #121.

Tests and examples updated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.