CircleCI-Public / cimg-postgres

MIT License
8 stars 25 forks source link

Request to add pgTAP variant #65

Closed pinealan closed 1 year ago

pinealan commented 1 year ago

pgTAP is a unit testing toolkit for postgres and CI is a natural place where one would need the test databse to have it available.

Demand for this can be evident from a number of derivative Dockerfiles in the wild that adds pgTap on top of the CircleCI image. These are all unfortunately no longer maintained and based on the legacy images.

It would be great if CCI can support this as an official image.

pinealan commented 1 year ago

I've made a branch adding pgTAP to postgres 13.9. Already tried it on CircleCI and it worked fine.

The extension adds about 40MB to the image size. If a PR is welcomed and I can do the same for the other postgres versions.

BytesGuy commented 1 year ago

@pinealan PRs are always welcome! If this is a small change to the image, and does not cause any changes to the behaviour of the image unless specifically invoked, then it might be best to include this in the main image rather than a variant. Postgis variant for example is close to 200MB larger, which is part of the reason it is a separate variant. Please reference this issue when opening the PR and one of the team will review it in due course. Thank you :)

villelahdenvuo commented 1 year ago

This would be a very welcome addition, we are currently using our custom image due to the need for pgTAP.

JalexChen commented 1 year ago

Hi @villelahdenvuo and @pinealan - since you two are requesting this change and also using this image the most, i wanted get your thoughts on how we should be supporting extensions. While extensions like pg_cron and pgTap are relatively small, I think they can potentially clutter the Dockerfile.template and also make it harder to debug when there are issues. Since these extensions don't seem to be updated frequently, I think there is little overhead and additional maintenance on our end.

With your workflows, do you think it is better to have a separate variant image, like postgres-postgis so the extensions are isolated?

I've already reviewed #81 and went with that because it implements cpanm and downloads the extension from the source repo. Let me know if there are any objections.

pinealan commented 1 year ago

@JalexChen thanks for checking in. My workflow is agnostic to whether I have to use a variant image or not, and in fact that was how my branch was originally implemented. That said I can imagine use cases where someone is using the postgis variant and would like to have access to pgTap too. With these cases they'd either lose out on pgTap because it's not in the base image, or they'd have to fork thier own image. The third option is to have postgres-postgis-pgtap variants, but I think that would be even messier from the maintainer's perspective.

villelahdenvuo commented 1 year ago

I agree that the downside of variants is less cacheability and the difficulty of combining variants. Therefore for smaller extensions I suggest including them in the main image.

JalexChen commented 1 year ago

pgTap has been merged into the main postgres image. Thank you both again for the contributions and the input.