Skip to content
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

Bump postgresql client to version 16 #138

Merged
merged 14 commits into from
May 23, 2024
Merged

Conversation

lars-fillmore
Copy link
Contributor

Bumping postgresql client to a later version. Official documentation notes that psql is backwards compatible.

"If you want to use psql to connect to several servers of different major versions, it is recommended that you use the newest version of psql. Alternatively, you can keep around a copy of psql from each major version and be sure to use the version that matches the respective server. But in practice, this additional complication should not be necessary."

https://www.postgresql.org/docs/current/app-psql.html

@lars-fillmore lars-fillmore requested a review from omad May 3, 2024 01:27
@lars-fillmore lars-fillmore self-assigned this May 3, 2024
@pjonsson
Copy link
Collaborator

pjonsson commented May 3, 2024

I've seen a PR on GDAL that updates its base image to Ubuntu 24.04 LTS. I don't know if that will end up/has ended up in what will be GDAL 3.9.0, but that would give a more recent Postgres client by just changing the version number like you've done.

Edit: GDAL 3.9.0 will use Ubuntu 24.04 as base for the Docker images (https://github.com/OSGeo/gdal/tree/release/3.9/docker). Dependabot will open a PR here to update the Dockerfile to use GDAL 3.9.0 when that is released.

@alexgleith
Copy link
Contributor

I think you'll need to update the base image, from here: https://github.com/OSGeo/gdal/pkgs/container/gdal

That'll update Ubuntu underneath it.

@lars-fillmore
Copy link
Contributor Author

GDAL 3,9 is still at release candidate 2. I'll leave this as is rn and wait a bit for a full release.

# Cleanup
&& apt-get autoclean \
&& apt-get autoremove \
&& rm -rf /var/lib/{apt,dpkg,cache,log}

ENV VIRTUAL_ENV /virtualenv/python3.10
ENV PATH /virtualenv/python3.10/bin:$PATH

COPY requirements.txt constraints.txt version.txt /conf/

RUN cat /conf/version.txt \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you just add --break-system-packages after install in the next few lines and skip everything else?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer not to as this feature in Ubuntu 24 was introduced for a reason. Other than a few extra lines of code, is there any other drawbacks from using virtualenv?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there are any other drawbacks, but if there's only one-to-one mapping between containers and virtual environments there's no real advantage either - other than making setup the same in a containerised and non-containerised environments, which I guess is a minor win.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @SpacemanPaul Agree that the only real tangible benefit will be consistency on how the python runtime is used between both container and non-container environments, and hopefully make code maintenance easier in the future.

# Cleanup
&& apt-get autoclean \
&& apt-get autoremove \
&& rm -rf /var/lib/{apt,dpkg,cache,log}

ENV VIRTUAL_ENV /virtualenv/python3.10
ENV PATH /virtualenv/python3.10/bin:$PATH

COPY requirements.txt constraints.txt version.txt /conf/

RUN cat /conf/version.txt \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there are any other drawbacks, but if there's only one-to-one mapping between containers and virtual environments there's no real advantage either - other than making setup the same in a containerised and non-containerised environments, which I guess is a minor win.

# Cleanup
&& apt-get autoclean \
&& apt-get autoremove \
&& rm -rf /var/lib/{apt,dpkg,cache,log}

ENV VIRTUAL_ENV /virtualenv/python3.12
ENV PATH /virtualenv/python3.12/bin:$PATH

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to set these manually? Isn't sourcing the activate script sufficient? Or does the way Docker uses intermediate containers break that approach?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess if it's passing tests it's probably OK.

@lars-fillmore lars-fillmore merged commit 7f16745 into main May 23, 2024
3 checks passed
@lars-fillmore lars-fillmore deleted the bump-postgresql-client branch May 23, 2024 03:20
@pjonsson pjonsson mentioned this pull request Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants