This doesn't match my usage pattern. Usually my migrations that just
go up are about fixing data or something else, like populating a
counter cache. I still want to be able to go 3 versions back, but if
there's an irreversible mixed in there it'll halt.
So then what is the point of the IrreversibleMigration exception? If
you are going to force people to create a rollback then that exception
should not exist.
It seems safer to assume that a missing down is equivalent to "either
there's no way to reverse this, or I didn't think about how to reverse
this", which this patch would do.
An empty down would signify "reversing this is a no-op", and it seems like
that would still work with this patch...
Yeah I actually have your workflow too. I'm not talking about changing behaviour and removing the self.down from the default template, just improving the use case of creating an IrreversibleMigration.
I guess really there's 3 categories of migrations:
(1) Up + down, reversible
(2) Up only, reversible
(3) Up only, irreversible
At the moment things are set up so (1) and (2) are easy peasy (from the generator template), but (3) is harder than it should be. Why not just remove self.down?
At the moment (3) requires looking up the (currently incorrect) API instructions and writing a message why you're raising.
As an aside: it seems odd/useless to me that the IrreversibleMigration class is defined and then only referenced in the documentation.... it's not actually handled anywhere. You could raise any old error.
I like the idea of a migration without a down method being
irreversible, and sympathise with some of the feelings of inelegance
when raising an exception.
> This doesn't match my usage pattern. Usually my migrations that just
> go up are about fixing data or something else, like populating a
> counter cache. I still want to be able to go 3 versions back, but if
> there's an irreversible mixed in there it'll halt.
Since the migration generator creates blank down methods, most people
won't be affected at all. Migrations that do raise exceptions will
still halt; migrations that are lacking down methods but which ARE
reversible can simply have them added and any stalled migration can
continue thereafter.
As an aside: it seems odd/useless to me that the
IrreversibleMigration class is defined and then only referenced in
the documentation.... it's not actually handled anywhere. You could
raise any old error.
For me, this is as good a reason as any to reconsider how migrations
handle reversibility. Who else has scratched their head trying to
remember what exception we ought to throw to prevent migrations down
from the current point? And it doesn't even matter! We could just as
easily call exit(-1) (and output something to stderr if we're feeling
generous).
For me, this is as good a reason as any to reconsider how migrations
handle reversibility. Who else has scratched their head trying to
remember what exception we ought to throw to prevent migrations down
from the current point? And it doesn't even matter! We could just as
easily call exit(-1) (and output something to stderr if we're feeling
generous).
I'll confess to having more than one migration with nil.asdf in the
down method simply because I don't remember how to correctly mark them
as irreversible... However I think I would prefer to encourage users
to explain WHY something isn't reversible, if just for the other
users.
It would beinteresting if the irreversible migration accepted a reason attribute and printed that reason when the exception is raised in console. Perhaps this would be a way to make self down useful in situations where all you are doing is raising the exception.
Perhaps we could get most of that by just having a specific rescue
block for IrreversibleMigration which printed out a nice error
message? Anyone care to take a stab at it?