Don't pollute output when testing Postgres connection#33
Conversation
|
I just noticed that this has already been fixed upstream, see https://github.com/api-platform/api-platform/blob/master/api/docker/php/docker-entrypoint.sh#L17 What we have to do is just to sync this file with the one in api-platform/api-platform. |
4852c1a to
fa5e752
Compare
Replaced in api-platform#31
fa5e752 to
9fffd40
Compare
|
I restore the command I have the postgres image |
| composer install --prefer-dist --no-progress --no-suggest --no-interaction | ||
| >&2 echo "Waiting for Postgres to be ready..." | ||
| until bin/console doctrine:query:sql "SELECT 1" --quiet; do | ||
| until pg_isready --timeout=0 --dbname="${DATABASE_URL}"; do |
There was a problem hiding this comment.
Even if we prefer using a Postgres database for performances reasons, this command definitively connects this entrypoint to the database system & driver. I don't find it very useful, especially if the final user wants to use a different system.
WDYT @dunglas?
There was a problem hiding this comment.
There is a better way. Thanks to the Sylius peeps who used it: https://github.com/Sylius/Sylius-Standard/blob/943125142f0267ac49903efeaab85e24de08424d/docker/php/docker-entrypoint.sh#L20-L23
Let's adopt the same method as well.
There was a problem hiding this comment.
It seems the same, but what is the purpose of using >/dev/null 2>&1 instead of --quiet?
There was a problem hiding this comment.
We used this Doctrine command long ago, but I can't remember why we switched. pg_isready looks more reliable anyway.
Even if we prefer using a Postgres database for performances reasons, this command definitively connects this entrypoint to the database system & driver. I don't find it very useful, especially if the final user wants to use a different system.
It's the same for the PDO driver and so on. The entrypoint is part of the infrastructure, to me it's ok and intended to couple it with the services we use.
There was a problem hiding this comment.
Anyway, please update api-platform/api-platform first, and the demo after by copying it.
10cf5c2 to
8882512
Compare
|
We should be consistent with https://github.com/api-platform/api-platform/blob/master/api/docker/php/docker-entrypoint.sh#L25 closing this |
5 participants
Following issue #31 and the comment #31 (comment)