Skip to content

Move postgres host and port to a single location. #1205

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

iwanders
Copy link

@iwanders iwanders commented May 2, 2022

These are some minor changes I did when I started development work on #1204. I'm not on NixOS and did most development in a NixOS docker container. Following the steps to bring up the development environment in a Docker container didn't work at first, Postgres doesn't allow running as root, and the NixOS docker image user is root. So in the end I ended up using a second container to run the Postgres server and then pointed the hydra instance running in the NixOS docker container at the external database. This did allow me to easily load a copy of the production database, so in the end it was actually quite convenient for development.

The postgres command line utilities respect the PGPORT and PGHOST environment variables, so we don't need to specify them manually in various files. Putting them in the environment allows overriding them manually when using a different postgres server.

The LOGNAME variable is new, but it appeared to be necessary for the queue runner to come up.

This also switches communication to the database during initialisation of the development environment to use tcp instead of a unix domain socket.

I have not tested these changes on a 'real' instance NixOS, only in a NixOS docker container (where Postgres doesn't start), the environment variables looked good inside the nix shell.

fyi @mikepurvis.

This makes it easier to use a different postgres instance when developing on hydra.
The LOGNAME variable is new, but it is necessary for the queue runner to come up.
@Ericson2314
Copy link
Member

This looks good to me. Sockets for PGHOST should also work.

@iwanders
Copy link
Author

iwanders commented May 3, 2022

Sockets for PGHOST should also work.

I switched it to use tcp to allow easy use of the postgres docker container. Exposing a unix domain socket should work as well yes, but I think exposing tcp ports in Docker is more standard.

@Ericson2314
Copy link
Member

Ericson2314 commented May 3, 2022

You did what? This diff makes it seem like it was using TCP before and after the change. I am fine with that, but I would be wary of switching away from UDP if that were the prior default.

@iwanders
Copy link
Author

iwanders commented May 3, 2022

You did what? This diff makes it seem like it was using TCP before and after the change. I am fine with that, but I would be wary of switching away from UDP if that were the prior default.

My knowledge of postgres is pretty limited, but the original command was createdb -h $(pwd)/.hydra-data/postgres -p 64444 hydra. For the -h flag the docs state the following:

Specifies the host name of the machine on which the server is running. If the value begins with a slash, it is used as the directory for the Unix domain socket.

Since $(pwd) starts with a slash, the -p 64444 is then interpreted as the local Unix domain socket extension.

I think that means that previously communication happened through a unix domain socket? After this change we would be communicating over a tcp socket.

Edit: I realize this was not mentioned in the description, I've added:

This also switches communication to the database during initialisation of the development environment to use tcp instead of a unix domain socket.

to it to make this change clear, apologies for missing it, I looked at the diff when writing the description and this change is all but clear from that.

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.

2 participants