odoo / docker

Other
931 stars 1.51k forks source link

Improve PSQL wait script #496

Open thatnerdjosh opened 3 months ago

thatnerdjosh commented 3 months ago

Follow up to #292

thatnerdjosh commented 3 months ago

@lathama - apologies for the back and forth - was struggling with the symlinks a bit.

This is ready for review :)

thatnerdjosh commented 3 months ago

@lathama - I don't think it's really ideal to maintain copy and paste of all those scripts, but I struggled to find a solution for that. I've ported all of the changes from the previous PR to all the new images

lathama commented 3 months ago

The installed package postgresql-client includes a tool called pg_isready which could be used to remove a ton of complexity. I had the cycles to take a look into this. The pg_isready could be added to the entrypoint.sh and the extra Python file could be removed.

$ pg_isready --help
pg_isready issues a connection check to a PostgreSQL database.

Usage:
  pg_isready [OPTION]...

Options:
  -d, --dbname=DBNAME      database name
  -q, --quiet              run quietly
  -V, --version            output version information, then exit
  -?, --help               show this help, then exit

Connection options:
  -h, --host=HOSTNAME      database server host or socket directory
  -p, --port=PORT          database server port
  -t, --timeout=SECS       seconds to wait when attempting connection, 0 disables (default: 3)
  -U, --username=USERNAME  user name to connect as

Report bugs to <pgsql-bugs@lists.postgresql.org>.
PostgreSQL home page: <https://www.postgresql.org/>
thatnerdjosh commented 3 months ago

Amazing - thank you @lathama - I'll migrate to this shortly

thatnerdjosh commented 3 months ago

@lathama - can you confirm if the below items are ok:

  1. This returns 0 even in the case of failed authentication
  2. There is no option to pass a DB password

This is fine if all we need to do is verify the DB is ready to accept connections, but seems less so if we want to validate a connection is actually established.

Please advise - thanks!

thatnerdjosh commented 3 months ago

Pending your feedback, I've completed the migration.

lathama commented 3 months ago

Looking now

thatnerdjosh commented 2 months ago

@amh-mw - I've removed the DB_NAME override from this PR since the bulk of the changes was adding the pg_isready support. Once that is reviewed, I'll start working on the DB_NAME override :)