Move api configuration to a config file #579
Conversation
|
This looks good to me, but I would like to see another review. @ekohl or @dhollinger maybe? |
|
@baurmatt can you please rebase? |
baurmatt
force-pushed
the
feature/api_configfile
branch
from
760e045
to
371284e
Compare
|
Done :) |
|
I think that the types should autorequire the config file. Not sure if that means the collectors could be replaced just yet. A host might require a template, but perhaps that can also be solved via autorequires. |
baurmatt
force-pushed
the
feature/api_configfile
branch
from
371284e
to
d1391f3
Compare
This is the first change for enhancing the custom types with deeper Puppet integration. This is needed to ensure the implementation of self.instances/self.prefetch works correctly.
baurmatt
force-pushed
the
feature/api_configfile
branch
from
d1391f3
to
e255928
Compare
|
Added the auto require for I'm happy to look into the requires between types, but I think that should be handled in a separate PR as this one is already big enough. |
|
@ekohl I am fine with this. What do you think? |
|
Hey, anything left we need to talk about? :) Would love to see this merged! |
| zabbix_pass => $zabbix_pass, | ||
| apache_use_ssl => $apache_use_ssl, | ||
| } | ||
| -> Zabbix_template <<| |>> |
There was a problem hiding this comment.
Are these needed anymore? Perhaps they are redundant (given the types all autorequire the API config file)?
It depends on whether they were just being used to set the API credentials or whether actually eg. all Zabbix_userparameters have to come after all Zabbix_hosts etc?
When given a block of attributes and values, a collector will set and override those attributes for every resource (virtual or not) that matches its search expression.
https://puppet.com/docs/puppet/5.3/lang_resources_advanced.html#amending-attributes-with-a-collector
There was a problem hiding this comment.
From my understanding they ensure that e.g. the host exists in Zabbix before the template gets added to it.
There was a problem hiding this comment.
Fair enough. Just checking that the relationships before this change were actually all required (and not just a slightly lazy syntax short-cut). Now they definitely look like they're there for a reason, so it's good to check.
|
Still looking to get this merged ;) Is there anything you guys don't feel comfortable with? |
|
@baurmatt Thanks! |
Pull Request (PR) description
This is the first change for enhancing the custom types with deeper Puppet
integration. This is needed to ensure the implementation of
self.instances/self.prefetch works correctly.
This is the basis for all changes needed for #570.
This Pull Request (PR) fixes the following issues