Safely Downgrade Bug Fix

[P0NDER0SA]

Summary | Résumé

Fixing a bug that fails on downgrade. This will allow this migration to safely upgrade and downgrade without problems

Test instructions | Instructions pour tester la modification

run Flask db downgrades and upgrades that would affect this migration. ensure all is well in the world.

[patheard]

As discussed, makes sense not to perform the REVOKE in the downgrade because of the chaos this would cause as the API relies on the app_db_user to run its migrations.

[P0NDER0SA]

@patheard @jimleroyer @sastels -- Might be worth having a conversation about this. I think the idea here is that if we downgrade and then upgrade this would be done as a more manual process for now, and only as necessary. I think @jimleroyer and @sastels would prefer to try and focus on doing a more representative 'downgrade' that we test thoroughly as a downgrade and upgrade before we merge these. There are historically many that will break if you do that process.

made some changes and looking for a bit of conversation with new reviewers

[sastels]

As discussed, makes sense not to perform the REVOKE in the downgrade because of the chaos this would cause as the API relies on the app_db_user to run its migrations.

Are we sure about this? we just run

flask db upgrade

here. I was assuming it was the postgres user or some other flask magic? app_db_user was just added when we started using blazer afaik.

[jimleroyer]

It's true we are using that user to connect with the app though, not just for blazer, as I think you standardized it @sastels (or Pat?).

[sastels]

It's true we are using that user to connect with the app though, not just for blazer, as I think you standardized it @sastels (or Pat?).

oh, 💯 the app would horribly break, but I didn't think flask would. unless flask is magically using POSTGRES_SQL or SQLALCHEMY_DATABASE_READER_URI - which it could be.

Update: ok, here's the relevant code. We set the flask variable SQLALCHEMY_DATABASE_URI from POSTGRES_SQL, which we set in the .env file and use app_db_user.

[P0NDER0SA]

Just another bit of reassurance on this -- this was tested 100% on dev and the upgrade and downgrade paths were working well. Very very low risk to merge this. We could always try a scenario on staging to see how it reacts in the future.

[jimleroyer]

@sastels Yes, the app, which is a Flask app, uses the values in the .env files. I am not sure which other DB connection config the app would leverage otherwise.

[sastels]

Just another bit of reassurance on this -- this was tested 100% on dev and the upgrade and downgrade paths were working well. Very very low risk to merge this. We could always try a scenario on staging to see how it reacts in the future.

dev is just using the postgres user for everything though (well, for the flask stuff?) because we were to lazy to figure out a programatic way to create the app_user 😞

[jimleroyer]

We talked offline and decided that this user is good for connecting to our app but we will have the admin one to bootstrap the schemas and take in charge the alembic upgrades. This user would be created by Terraform when booting up a new environment. A related task card was created and will be refined for this purpose.