Refactor statsd_exporter class to support version >= 0.5.0; bump from 0.3.0->0.8.0 #271
Conversation
|
I dont have enough time anymore, i'll fix the small style issues next week. Probably Tuesday. |
| prometheus::statsd_exporter::package_ensure: 'latest' | ||
| prometheus::statsd_exporter::package_name: 'statsd_exporter' | ||
| prometheus::statsd_exporter::user: 'statsd-exporter' | ||
| prometheus::statsd_exporter::version: '0.3.0' | ||
| prometheus::statsd_exporter::version: '0.8.0' |
There was a problem hiding this comment.
Can you please keep the old version? Otherwise it would be a breaking change. We want to do that in a later release for all exporters.
There was a problem hiding this comment.
It is a breaking change anyways. The configuration format changed between v0.4.0 and v0.5.0. It won't download and configure v0.3.0 or older correctly, it would require more/duplicate logic to also support the old version.
There was a problem hiding this comment.
The style issues are fixed now and saw that i didn't see that versions older than v0.7.0 still used the single dash options so i added some logic for that too.
There was a problem hiding this comment.
I also did an experiment to see how difficult it would be to support older versions as well but it gets quite funky and i think more confusing with the different file format and names..
There was a problem hiding this comment.
We did a release of this module a few days ago. In the current merge window we want to update all exporters, so this is fine now.
Signed-off-by: Wiebe Verweij <wiebe@wiebelt.nl>
bastelfreak
changed the title
templates/statsd_mapping.yaml.erb
Outdated
| @@ -0,0 +1,3 @@ | |||
| <% require 'yaml' -%> | |||
| <% full_config = { 'mappings'=>@mappings } -%> | |||
| <%= full_config.to_yaml -%> | |||
There was a problem hiding this comment.
Can't we use this puppet function to generate the yaml? https://github.com/puppetlabs/puppetlabs-stdlib#to_yaml
and provide that to the file as content? That would allow us to not use a template at all.
There was a problem hiding this comment.
|
Can you please add an acceptance test for this? |
Is there any information somewhere on how i can even run the acceptance tests myself?, that is what held me back the most (i already have docker and everything installed i think, just need the command really).. |
|
You can execute them locally with: |
bastelfreak
added
enhancement
|
Thanks for the PR! |
Refactor statsd_exporter class to support version >= 0.5.0; bump from 0.3.0->0.8.0
Refactor statsd_exporter class to support version >= 0.5.0; bump from 0.3.0->0.8.0
Signed-off-by: Wiebe Verweij wiebe@wiebelt.nl
Pull Request (PR) description
Refactor statsd_exporter class to support version >= 0.5.0
This Pull Request (PR) fixes the following issues
Fixes #248