search

mwhahaha / puppet-pgpool

Apache License 2.0
3 stars 22 forks source link

Shouldn't use Git submodule for Augeas lens #11

Open antaflos opened 9 years ago

antaflos commented 9 years ago

We ran into this today. It seems R10k does not support deploying Git submodules (https://tickets.puppetlabs.com/browse/RK-30) so the pgpool.aug Augeas is missing after the module is installed.

This also seems to be a problem for the release on the Forge. The latest release, when downloaded as a tarball, doesn't contain the pgpool.aug Augeas lens but only an empty directory named augeas-pgpool:

 $ tree mwhahaha-pgpool-0.1.14/
mwhahaha-pgpool-0.1.14/
├── checksums.json
├── files
│   └── augeas-pgpool
├── LICENSE
├── manifests
│   ├── config
│   │   ├── backend.pp
...

Obviously this is suboptimal :)

I suggest either maintaining the Augeas lens directly within this module, or releasing a proper module containing only the lens and listing that module as a dependency, like you do with camptocamp/augeas, herculesteam/augeasproviders_core, herculesteam/augeasproviders_postgresql and so on.

The latter approach is described in https://docs.puppetlabs.com/guides/plugins_in_modules.html, but I would favour the former, letting the Augeas lens live within the pgpool module, and making use of Puppet's plugin sync. The path would look like this:

{modulepath}/pgpool/lib/augeas/lenses/pgpool.aug

Thoughts?

mwhahaha commented 9 years ago

That was actually my fault, forgot to run the command to populate the submodule before I built the pgpool package for the forge. I'll correct that.

mwhahaha commented 9 years ago

As for the maintaining the lens with the module, I have it separate so that it would be more useful to someone who want to use it outside of puppet. Since I'm not implementing providers, it doesn't really make sense to roll it into a new module. Ideally it could be something to just provide with augeas so that it would just be available and we wouldn't have to be providing it as part of this module. It's one of those nice to have things if I had time to submit it.

antaflos commented 9 years ago

I'd argue that the pgpool.aug lens is probably used much more within the context of Puppet and specifically this module than anywhere else outside of Puppet, so I'd expect the lens to live here, treating Puppet as the first-class citizen, as it were. But I understand your reasoning and ultimately that's your decision.

We can work around the R10k problem by using a corrected Forge release (thanks for that, by the way!), but that doesn't really solve it. So I suggest adding a warning regarding R10k and the submodule contained to the README, as a courtesy to those who want (or have) to deploy this module from Github directly instead of the Forge.

As for pushing the lens upstream to the Augeas project: I think this is a worthwhile idea but even if you did that today it would probably take months until it is included in an upcoming Augeas release, and it would never be available for today's supported distros (precise, trusty, wheezy, jessie, Centos 6, 7, ...), anyways. At least not without backported Augeas packages.