Opened 8 years ago

Closed 6 years ago

#459 closed enhancement (wontfix)

Improve migration framework

Reported by: Elrond Owned by:
Priority: minor Milestone:
Component: programming Keywords: patch
Cc: berkerpeksag Parent Tickets:

Description

We had a problem with two migrations not working nicely on sqlite, because the second one wanted to do an alter table and sqlite doesn't like to do that in a bigger transaction. Long story: Every migration should say "I'm done" and commit. This happened for the existing migrations.

So. In my opinion, the migration framework should run a rollback after each migration "just in case" to clean up any cruft left over from a (possibly broken) migration.

- migration_func(self.session)
+ try:
+   migration_func(self.session)
+ finally:
+   self.session.rollback()

I have NOT TESTED this patch. This is just to show the idea. I want some feedback on it, and possibly someone to test it.

Subtickets

Change History (15)

comment:1 Changed 8 years ago by Elrond

Owner: set to Christopher Allan Webber
Status: newassigned

comment:2 Changed 8 years ago by Elrond

Hi Chris,

can you please comment on this? You're the master of the migrations, after all!

(hinted by ShawnRisk)

Last edited 8 years ago by Elrond (previous) (diff)

comment:3 Changed 8 years ago by Christopher Allan Webber

Elrond:

I trust your judgement here better than I trust my own. It seems like a good idea.

(However, I think if somehow a previous migration broke things and migrations did not stop, that's a bug!)

comment:4 Changed 8 years ago by Christopher Allan Webber

Owner: changed from Christopher Allan Webber to Elrond

comment:5 Changed 8 years ago by Christopher Allan Webber

Elrond and I discussed on IRC. We agreed on a better way:

try:
  run_migration()
except:
  rollback()
  raise
else:
  commit()

comment:6 Changed 8 years ago by Elrond

Milestone: 0.3.4

Okay, setting this for the "after release" milestone.
I don't want to break the upcoming release.

comment:7 Changed 8 years ago by Elrond

Milestone: 0.4.00.4.1

Chris Webber and I agreed, that we both improve our migration system after the next release. See #685

Yes, postponing this one again. But really, it's not pressingly needed.

comment:8 Changed 7 years ago by Christopher Allan Webber

Status: assignedin_progress

Hi! I'm moving this ticket from "assigned" to in_progress per our new workflow. Please update the ticket and let us know if you're still working on this. If you are, super great! If not, we'll remove the claim and move it back to "accepted" in a couple of weeks.

Thanks!

comment:9 Changed 7 years ago by Christopher Allan Webber

Milestone: 0.5.00.6.0

comment:10 Changed 7 years ago by Christopher Allan Webber

Milestone: 0.6.00.7.0

comment:11 Changed 6 years ago by Christopher Allan Webber

Milestone: 0.7.00.8.0

I'm bumping this to 0.8.0 partly, but if it doesn't really make it in 0.8.0, the milestone should be removed so as to avoid milestone-bump spam. Additionally, the python 3 branch may deprecate our current sqlalchemy-migrate based migration setup, in which case we can just close this out.

comment:12 Changed 6 years ago by berkerpeksag

Cc: berkerpeksag added

comment:13 Changed 6 years ago by Christopher Allan Webber

Milestone: 0.8.0

comment:14 Changed 6 years ago by Christopher Allan Webber

I'm closing this. We're going to be moving to Alembic soon anyway.

But hey, for documentation, someone posted this useful advice [here https://code.google.com/p/sqlalchemy-migrate/issues/detail?id=143]:

   Hi,
   it always fails in the case of Boolean because by default boolean has the  
   Constraints.
   so first you have to remove the Constraints then you can remove the column..
   
   Internaly your table looks like:
   CREATE TABLE creature (
         CHECK (is_demon IN (0, 1)),
            ....
            ....
   
         )
   
   Regards
   Rakesh

So if we need to, we could make use of that advice.

comment:15 Changed 6 years ago by Christopher Allan Webber

Owner: Elrond deleted
Resolution: wontfix
Status: in_progressclosed
Note: See TracTickets for help on using tickets.