voxpupuli / puppet-mongodb

mongodb installation
Apache License 2.0
93 stars 451 forks source link

Server config as hash #735

Closed h-haaks closed 7 months ago

h-haaks commented 7 months ago

Still working on this. Just wanted to publish it to get feedback.

h-haaks commented 7 months ago

I see your point :) If we sort out the issues in #737 I'm happy with that.

stevenpost commented 7 months ago

It seems both our PR were authored simultaneously, one I had created mine, I noticed yours :) The main difference is that I tried to preserve the interface we had. Not sure if that is actually a good thing. I do see a benefit in your approach.

stevenpost commented 7 months ago

@h-haaks I think we either need to go with your approach or with a combined one. I think the issue I'm currently facing is that the authorization is not properly configured because one of our custom config keys is overriding it, even though it shouldn't conflict.

stevenpost commented 7 months ago

Perhaps we can take this to another level and not have any parameters for the config apart from a single hash?

stevenpost commented 7 months ago

I decided your approach is superior, and have closed my PR.

h-haaks commented 7 months ago

Perhaps we can take this to another level and not have any parameters for the config apart from a single hash?

I actually started out changing the config param to a catch all config hash ( as changing the config file location will break service startup ..) but if we do that users have to copy all defaults set by hiera if they need to change/add settings.

I will try to work a bit on the code in your branch if that's ok to see if I can get the hash created in the template alternatively in a function.

stevenpost commented 7 months ago

How about having a single hash and doing an explicit lookup for the defaults, then merging the hashes?

h-haaks commented 7 months ago

740 turned out to be a better solution I think