WIP: Untested POC for #851 #852
Conversation
templates/sensu.erb
Outdated
| <% end -%> | ||
| <% if @heap_size -%> | ||
| HEAP_SIZE="<%= @heap_size %>" | ||
| # File managed by Puppet |
There was a problem hiding this comment.
We may also mantain all the if conditions in the template, it would just require more maintenance, as contante would need to be changed whenever a new parameter is added
types/env_vars.pp
Outdated
| Optional[PATH] => String, | ||
| Optional[CONFD_DIR] => String, | ||
|
|
||
| }] |
There was a problem hiding this comment.
Each time a new environment variable has to be supported this would be the only place where to add something.
types/env_vars.pp
Outdated
| @@ -0,0 +1,14 @@ | |||
| type Sensu::Env_Vars = Struct[{ | |||
|
|
|||
| Optional[EMBEDDED_RUBY] => Boolean, | |||
There was a problem hiding this comment.
Wow, love the type validation here of the struct!
There was a problem hiding this comment.
yeah, struct is the missing link to allow usage of single hashes to manage AND validate multiple configuration settings without multiplying classes' parameters.
Only limit is that currently, afaik, it' not possible to allow any possible key value, the ones accepted, even if optional, must be defined in the struct.
manifests/package.pp
Outdated
| 'SERVICE_MAX_WAIT' => $init_stop_max_wait, | ||
| 'PATH' => $path, | ||
| 'CONFD_DIR' => "${conf_dir},${confd_dir}", | ||
| } |
There was a problem hiding this comment.
shouldn't heap size be here?
if we remove it, we would need need to bump the major version, which I'm not excited about doing since we'll be doing that soon enough for the next version of Sensu.
There was a problem hiding this comment.
heap_size afaik is used only on sensu enterprise, so it would be used in sensu::enterprise
|
Closing this PR, too many fixes to do on tests which are not worth, now, the advantages of this refactoring. @ghoneycutt Use #861 instead. |
|
@alvagante I like this approach to managing config files. Doing a similar thing with the facter module - ghoneycutt/puppet-module-facter#59 |
@cwjohnston @ghoneycutt this is what I have in mind, I think that seeing the sample (not tested) code give you the idea of what I mean.
A similar thing could be done for the sensu::enterprise class and /etc/default/sensu-enterprise.
Pull Request Checklist
Description
Related Issue
Fixes # .
Motivation and Context
How Has This Been Tested?
General
Update
README.mdwith any necessary configuration snippetsNew parameters are documented
New parameters have tests
Tests pass -
bundle exec rake validate lint spec