add explicit parameter for retention #137
Conversation
costela
force-pushed
the
make_retention_configurable
branch
3 times, most recently
from
5ca7c2e
to
061061f
Compare
manifests/config.pp
Outdated
| @@ -7,6 +7,7 @@ | |||
| $remote_read_configs, | |||
| $purge = true, | |||
| $config_template = $::prometheus::params::config_template, | |||
| $retention = $::prometheus::params::storage_retention, | |||
There was a problem hiding this comment.
You are referencing the variable from params.pp. Why not the variable from init.pp?
In your case the value is always fix and not changeable.
There was a problem hiding this comment.
maybe I misunderstood your comment, but the parameter is changeable because it gets passed inside init.pp, in the instantiation of class[::prometheus::config] (and it's also a parameter to init itself).
and I wanted to stick to the current coding style, since $config_template is implemented in exactly the same way (AFAICT)
There was a problem hiding this comment.
@costela ok. I missed the parameter hand over in init.pp
Technically OK. Not my most favorite solution, but working as expected.
There was a problem hiding this comment.
oh, BTW, time for a bit of bikeshedding: retention or storage_retention? I couldn't decide ... 😁
manifests/init.pp
Outdated
| @@ -151,6 +151,7 @@ | |||
| $alerts = $::prometheus::params::alerts, | |||
| Array $alert_relabel_config = $::prometheus::params::alert_relabel_config, | |||
| Array $alertmanagers_config = $::prometheus::params::alertmanagers_config, | |||
| String $retention = $::prometheus::params::storage_retention, | |||
There was a problem hiding this comment.
is this a request for a more specific datatype or just celebrating the current String datatype? 😄
I mean, I could use a regexp type, but it felt a bit overkill...
this way we don't need to use $extra_options
costela
force-pushed
the
make_retention_configurable
branch
from
061061f
to
346e028
Compare
| @@ -151,6 +156,7 @@ | |||
| $alerts = $::prometheus::params::alerts, | |||
| Array $alert_relabel_config = $::prometheus::params::alert_relabel_config, | |||
| Array $alertmanagers_config = $::prometheus::params::alertmanagers_config, | |||
| String $storage_retention = $::prometheus::params::storage_retention, | |||
|
Thanks for the PR @costela ! |
…igurable add explicit parameter for retention
…igurable add explicit parameter for retention
This way we don't need to use
$extra_options. Should make module slightly more newbie-friendly.Fixes #136