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

Add support for any config options #375

Merged
merged 1 commit into from Oct 24, 2016

Conversation

Projects
None yet
2 participants
@degemer
Copy link
Member

degemer commented Oct 20, 2016

It is annoying to have to wait for a new cookbook release when an agent
is released with a new config option.
This commit aims to fix it by allowing any config options to be added.

Fix #336

@degemer degemer added this to the 2.7.0 milestone Oct 20, 2016

@degemer degemer force-pushed the quentin/support-any-config-key branch 2 times, most recently from ed56583 to cec18ac Oct 20, 2016

@degemer degemer added the feature label Oct 20, 2016

@olivielpeau
Copy link
Member

olivielpeau left a comment

Had one "important" comment, everything else looks 👌

@@ -221,6 +221,11 @@
# extra_packages to install
default['datadog']['extra_packages'] = {}

# extra config options
# If an agent is released with a new config option which is not yet supported by this cookbook
# you can use this attribute to set it. Will be ignored if falsy.

This comment has been minimized.

Copy link
@olivielpeau

olivielpeau Oct 21, 2016

Member

I think we should ignore the value only if it's nil. We have agent configuration options that are true by default so setting them to false is meaningful. Thoughts?

This comment has been minimized.

Copy link
@olivielpeau

olivielpeau Oct 21, 2016

Member

(also, if you agree, a test on that would be perfect :) )

This comment has been minimized.

Copy link
@degemer

degemer Oct 21, 2016

Author Member

Makes sense. I didn't think of false, only of '', which is useless since ini requires key: value.

@@ -221,6 +221,11 @@
# extra_packages to install
default['datadog']['extra_packages'] = {}

# extra config options

This comment has been minimized.

Copy link
@olivielpeau

olivielpeau Oct 21, 2016

Member

nit: could you put this right under default['datadog']['histogram_percentiles'] in this file, so that it's visually regrouped with the other agent config attribute defaults?

@degemer degemer force-pushed the quentin/support-any-config-key branch from cec18ac to 42c058f Oct 21, 2016

Add support for any config options
It is annoying to have to wait for a new cookbook release when an agent
is released with a new config option.
This commit aims to fix it by allowing any config options to be added.

@degemer degemer force-pushed the quentin/support-any-config-key branch from 42c058f to f712ee6 Oct 21, 2016

@degemer

This comment has been minimized.

Copy link
Member Author

degemer commented Oct 21, 2016

Updated @olivielpeau, ready for round 2!

@olivielpeau
Copy link
Member

olivielpeau left a comment

👌
🙇

@degemer degemer merged commit 32bef19 into master Oct 24, 2016

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@degemer degemer deleted the quentin/support-any-config-key branch Oct 24, 2016

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.