Parameter to not manage postgresql server

[jantman]

• add boolean parameter $manage_server to puppetdb::database::postgresql to conditionally not manage postgresql::server, allowing it to be managed separately, i.e. if we need to pass specific options to postgresql::server via an ENC, or if we want to manage the database but manage a (shared) postgresql server in one place
• remove the duplicate "manage_firewall" sections from README.md
• while we're here, fix the module description

[jantman]

Any chance of getting this looked at?

[golgy]

+1.

This is a current show-stopper, as specifying database parameters via puppetlabs-puppetdb prevents any other use of the puppetlabs-postgresql module.

[jantman]

I've opened https://tickets.puppetlabs.com/browse/MODULES-1137 - perhaps PRs aren't being tracked correctly for this module? Having not heard back in over 3 months, I'd make that assumption...

[blkperl]

Look good to me too.

@hunner @apenney @mhaskel

[blkperl]

@kbarber

[Spredzy]

+1.

This lets one having a PostgreSQL instance configured as desired via an external (to this module) call to the ::postgresql::server class.

[kbarber]

The PR requests have been stalled on this module as we're pretty busy and lack automated tests (we had them, we stopped them as we were promised new testing facilities, but that never surfaced). Not only that, I'm the only DSL expert on the PDB team atm.

We're looking to get this resolved at some point.

And yes, I know I can test this all manually but you've got to understand the amount of time to do this is prohibitive for our team atm. Especially since we need full distro coverage.

I know this is a pretty lame excuse, but alas - this is where we are.

[jantman]

Ken, that's pretty sad to hear. I'd thought this was a pretty straightforward PR.

Is there anything I can do to help speed this along? Write tests? Push on our sales people to get you guys what you need? We're up for renewal on our licenses in a month, I'd be happy to hold our licenses hostage for you getting what you need.

I'd really prefer not to just release my fork on the Forge, but if we and others need something usable....

[Spredzy]

@jantman As much as I would like to see this patch on the official module on the forge, I don't think there is a need to release a forked module on the forge. Can't you fork it, (either on Github or private git repo), apply your patch and create a Puppetfile that would point to it ?

That's what I am doing currently, until this gets merged.

[kbarber]

@jantman you're right, this PR is pretty basic and probably not a good example. I just wanted to express why we have reduced our merging generally - not specifically around this patch or anything. Basically our merge confidence is low because of the lack of testing generally, and as we know software changes that seem obvious can sometimes cause unexpected results, so these checks are very important.

It may not be obvious but I'm talking a lot to ppl internally to try and get this resolved somehow, which might mean I get some time carved out to work on this testing. Unfortunately this will cut into the time we spend on other important features, such as structured fact querying, which I've been waiting for YEARS to arrive :-( ...

[kbarber]

@jantman I'm going to merge this anyway for now, despite my concerns since it does seem basic as you say.

[jantman]

Ken, thanks so much for merging this. Understood about the testing stuff.

I'm sure you are, and yeah features are good. But if there's anything I can do as either a community member willing to cut some PRs as time allows, or as a customer, I'm happy to help.