-
Notifications
You must be signed in to change notification settings - Fork 823
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update DB dockerfile to use the postgis image #4294
Conversation
To review this I updated to the latest version of Docker Desktop 3.1.0 (on MacOS 10.14.6) I checked out this branch after using The
And also kosmtik gets this error:
When you tested this, did you remove all the existing containers and rebuild them as above? |
I completely removed all docker containers, images and volumes, then tried it again. I got this error:
PostgreSQL seems not to have run properly and the database did not connect. |
Strange, that is the same way I tried. I will try again tomorrow. |
I updated the PR. Apparently I did not clean up my Docker environment as much as I thought. There were indeed some missing PostGIS configuration changes. |
@hiddewie what is the reason for this PR at the current time? You said: "Using this base image will allow updating to more modern Postgres versions as well (see the full list in https://registry.hub.docker.com/r/postgis/postgis/tags). In the future, newer PostgreSQL and PostGIS versions can be used. The upgrade is out of scope of this PR.” But we are not upgrading the PostgreSQL or PostGIS version right now. So is it necessary to change the docker image for this release? Are there some benefits? |
@hiddewie I could review this PR again, but it takes about an hour to make the changes to my docker setup each time, so I would like to understand the benefit we will gain from this PR before working on it further. |
@jeisenbe Thanks for your comments. I would argue that upgrading a Docker image should be easy if there will ever be an infrastructure change in the deployed version of this style. The (Docker) development environment should be as close as possible to the production environment. And when the infrastructure versions are upgraded, you do not want that change blocked by having to wait for an up-to-date image. As mentionend in the PR description, the currently used PostGIS image https://hub.docker.com/r/mdillon/postgis has not had updates for two years. You can choose to accept this fact, and let the development environment 'fall behind' on updates. If you choose that, feel free to close this PR. After looking around in Github it seems the person responsible for the An additional benefit of the official PostGIS Docker image is that the Alpine version is much smaller to download. |
I agree with moving to the official postgis image, but don't use docker so can't review this PR |
I tested this, but am concerned that we don't have enough people using this code who are reviewing PRs. |
Thank you! Do not hesitate to contact/tag me in new issues that arise for the Docker images. I trill try to help if I am able to. |
For local development, the Dockerfiles are used to set up a development environment.
The database is created with the Dockerfile.db image, which is based on the image
mdillon/postgis
(https://hub.docker.com/r/mdillon/postgis/). This base image has not been updated for 2 years. Moreover, the Dockerhub link links to the official PostGIS imagepostgis/postgis
(https://github.com/postgis/docker-postgis).This PR updates the base image for the Dockerfile.db. The official image of PostgreSQL 10 with PostGIS 2.5 is used. The alpine version is used which results in a smaller image to download.
Using this base image will allow updating to more modern Postgres versions as well (see the full list in https://registry.hub.docker.com/r/postgis/postgis/tags). In the future, newer PostgreSQL and PostGIS versions can be used. The upgrade is out of scope of this PR.
Finally, the Docker
COPY
command is used instead of theADD
command. TheADD
command can also untar files, or fetch remote URLs which introduces complexity.Tested by running the database, importing a dataset and viewing it in the Kosmtik previewer.