Opened 7 years ago
Closed 7 years ago
#5395 closed defect (fixed)
Look into why session is not used on AlembicMigrationManager
|Reported by:||Jessica Tallon||Owned by:|
|Component:||programming||Keywords:||alembic, migrations, db|
|Cc:||Christopher Allan Webber, berkerpeksag||Parent Tickets:|
I noticed when debugging #5391 that migrations were forced to use SQLite which was defined in alembic.ini. I've pushed a fix for this which sets that option based on mediagoblin.ini however we shouldn't need to do this. We're providing a session on the manager which seems to be completely ignored, this leads to misleading code.
We need to decide between these two options:
- We remove the session from AlembicMigrationManager to make it obvious alembic will internally open a new one
- We make it so Alembic uses our database connection/session we've provided so it doesn't open another.
The way the code looks having a database connection we've setup but (seemingly) not using it is misleading and probably why it wasn't spotted earlier.
Change History (1)
comment:1 by , 7 years ago
|Status:||new → closed|
I think this is fixed as of 4c77f3d.
We ideally need to update alembic to at least 0.7.5 but probably just go to the latest if that is possible, we shouldn't be using really old versions. For now I've just used a bit of a hack, not great but it works, once we upgrade we can remove the hack and it'll just work.