Postres secrets #310
Postres secrets #310
Conversation
There was a problem hiding this comment.
By removing the .env.docker you remove the default .env used in the docker-compose.yml and you force all the developers to make an extra step of configuration by creation the .env from the .env.example.
- I'm not sure that it's interesting since every additional step in a quick-start is always a source of potential problem for newcomers
- If we do this change, we should at least explain it in the documentation since it's not just in case we want to
modify itanymore
Other than that, putting docker config from ENV is a nice improvement I think.
| @@ -31,4 +31,4 @@ services: | |||
| volumes: | |||
| - ./:/code | |||
| ports: | |||
| - "8001:8001" | |||
| - "8010:8010" | |||
There was a problem hiding this comment.
Why do you change the port ? I don't understand this change.
And if it's correct to change it, we should also update the documentation
API-Volontaria/docs/getting_started/docker.md
Lines 49 to 58 in 9bd7c63
| def pg_isready(host, user, password, dbname): | ||
| while time() - start_time < check_timeout: | ||
| try: | ||
| conn = psycopg2.connect(**vars()) | ||
| conn = psycopg2.connect(**config) | ||
| logger.info("Postgres is ready! ✨ 💅") | ||
| conn.close() | ||
| return True | ||
| except psycopg2.OperationalError: | ||
| logger.info( | ||
| f"Postgres isn't ready. Waiting for {check_interval} " | ||
| f"{interval_unit}..." | ||
| f"{interval_unit}... using {config}" | ||
| ) | ||
| sleep(check_interval) | ||
|
|
There was a problem hiding this comment.
Why not using the vars() ? It was a good way to make it configurable.
If we don't use the vars anymore, we should remove it from the function parameters and change the call on line 42
What have you changed ?
...
improve the way postgres secrets are managed
How did you change it ?
all postgres secrets like db name host, user and pass will use .env variables everywhere preventing hard code this values.
...
Is there a special consideration?
...
Verification :
CONTRIBUTING.mdCONTRIBUTING.md