Add db-migrate to container path to meet infra template requirements #176
Conversation
| # Load env vars before anything further | ||
| from src.util.local import load_local_env_vars # noqa: E402 isort:skip | ||
|
|
||
| load_local_env_vars() |
There was a problem hiding this comment.
How do the env vars get loaded locally? I thought that was done when running Flask commands, but this doesn't look to run via Flask.
There was a problem hiding this comment.
good question. i have to confess that was prioritizing getting database integration working, so i intentionally let running on host machine break. i was gonna work on that soon with #136
i'm currently thinking of having developers install direnv, and having an app/.envrc file that looks like
source local.env
and then whenever you cd into app direnv will automatically get all the env vars from local.env into your shell, and when you cd out of the directory direnv will automatically unload those env vars.
the benefit of this is that local developers can also override vars in local.env without risking checking those in by adding the overrides to .envrc e.g.
source local.env
export DB_HOST=localhost
There was a problem hiding this comment.
(I actually probably already broke running host machine with PR #172 , so it was probably already broken before this PR)
Ticket
Resolves #175
Changes
Context for reviewers
The infra template PR https://github.com/navapbc/template-infra/pull/316/files adds application-requirements that require application container to have a "db-migrate" command available in the PATH. This PR updates template-application-flask to do that.
The previous migrations code connects to the database using a different method than what the application uses, which means that the migrations code does not have the functionality of connecting to the DB using IAM auth, which is a requirement for the infra template. In this change, we make the migrations code in env.py use the existing PostgresDBClient class and remove the extraneous make_connection_uri class.
As part of the change, we also remove support for offline migrations, which is not needed in general. This simplifies the code paths in env.py and reduces maintenance burden.
Testing
Locally ran
make release-buildthen randocker -it <image hash> db-migrateThe migration obviously didn't succeed since I didn't pass any necessary env vars, but the db-migrate command was found.