1225274 - [PulseGuardian] SQL migration strategy

Status: RESOLVED FIXED

(Webtools :: Pulse, defect, P1)

Description

[Mark Côté [:mcote]]

There are a few bugs that will need changes to the database schema.  Right now we have no convenient way to do this; we just make the modifications to the db before deploying.  This is error prone and doesn't track history.  We should have a proper set of migration scripts.

Alembic is an option, especially since PulseGuardian's production instance runs on Heroku; see http://nyghtowl.io/2013/09/15/how-flask-heroku-alembic-play-together/.

Comment 1

[Mark Côté [:mcote]]

Note that, to really complete this, we'll need an actual migration to do, so we can bundle this bug with another, say, bug 1091485.

Comment 2

[Mike Yao]

This is commit link:

https://github.com/yaoweizhen/pulseguardian/commit/df36aff1bb5f5f6a1c2c28fc1964839d8f4204fc

Please run on PostgreSQL because sqlite isn't fully support SQL like "alter table".
See this: https://alembic.readthedocs.org/en/latest/batch.html

Comment 3

[Mark Côté [:mcote]]

So one issue here is that we can't use a customized .ini file in Heroku.  We can have one with the defaults as you've provided, but we can't make local changes; all configuration variables have to be in the environment.  This actually shouldn't be that bad, as the only variable that has to change depending on the installation is the SQLAlchemy URL, which you can load it from the environment in env.py instead of from the config file.  The environment variable is called DATABASE_URL.  Then you'll need to rename your ini file to just alembic.ini and update your instructions accordingly.

See http://nyghtowl.io/2013/09/15/how-flask-heroku-alembic-play-together/ for more info.  I realize this is probably hard for you to test in Heroku itself, although you can run Heroku apps locally.  If this is a bit confusing (and I realize it probably is, please send me questions. :)

Also, the whole offline-versus-online mode is confusing for someone who doesn't know Alembic

Comment 4

[Mike Yao]

Updated, please see https://github.com/yaoweizhen/pulseguardian/commit/5b9d56fe264af828ed255d8a37a87ea0b3fa706d

Comment 5

[Mark Côté [:mcote]]

This is great!  Just a few last steps:

As I put in a comment in one of the commits, please add a comment to the ini file above the sqlalchemy.url variable explaining that it's overridden.

Then, can you squash the commits down to just one clean commit and force push (or push as a new branch)?  Also please rebase on the latest commit in master.

After that, we're good to go! :)

Comment 6

[Mark Côté [:mcote]]

Thanks for the contribution!

https://github.com/mozilla/pulseguardian/commit/bbccea4efc5f1742c1c6134366970963936f87d4

Comment 7

[Mark Côté [:mcote]]

I'll deploy this next week when I'm back from vacation.  NIing me as a reminder.

Comment 8

[Mark Côté [:mcote]]

Deployed this morning.