voxpupuli / puppet-php

Generic Puppet module to manage PHP on many platforms
http://forge.puppet.com/puppet/php
MIT License
87 stars 269 forks source link

Replace hiera with lookup #449

Open SimonHoenscheid opened 6 years ago

SimonHoenscheid commented 6 years ago

Time goes on, hiera lookups are going to be deprecated, we should replace them with the appropriate lookup functions

Refers to:

c33s commented 6 years ago

mainly we should not use lookup functions alt all, i personally prefer "automatic class parameter lookup". mixing both leads to unwanted behavior. see https://github.com/voxpupuli/puppet-php/pull/435

SimonHoenscheid commented 6 years ago

I have in Mind, that you have to provide a lookup behavior. Explicit vs. Implicit lookup is something that needs to be discussed. Is there a policy from Voxpopuli?

bastelfreak commented 6 years ago

We currently don't have a written policy on this. Normally we don't define a policy in the hiera.yaml, unless it's desired.

c33s commented 6 years ago

@SimonHoenscheid the problem is the lookup behavior is hardcoded. of course we can add a parameter to allow to configure this lookup behavior but what is the benefit over simply using "automatic class parameter lookup"? with automatic class parameter lookup we can use hiera in the module, we can let the user override it via hiera and also allow the user to let the user set the parameters in the class call.

look at #435 what happend there was that i used the automatic class parameter lookup which uses its default setting to "not to merge with other hierachies" (i think unique or first). the module then has done the hiera lookup again with an hard coded "merge all you find" behavior which led to a wrong behavior as suddenly php settings from other hierachies where in the final settings hash which could not be configured or changed by the user/developer. this is a must not for me.

so i vote for removing all lookup functions and use "automatic class parameter lookup" everywhere (is there one location where it is not possible to do that?)