Unapplying migrations in a branch

[Roman Odaisky]

Hi All,

I would like to solicit some further comments regarding #32267. The main question is:

Should Django support unapplying migrations in a branch?

Typical use case:

$ git switch otherdeveloper/feature

$ manage.py migrate

Applying app.0051_migration_from_feature... OK

Applying app.0052_migration_from_feature... OK

Applying app.0053_merge_master... OK

Applying app.0054_migration_from_feature... OK

$ do some work on the feature branch

$ manage.py migrate ???

$ git switch master

Despite there being a very clear path from the feature branch to the master branch, namely, to reverse the migrations that have just been applied, there’s no way to tell Django to do that. manage.py migrate app 0051_migration_from_feature will unapply too little (it will keep 0051 itself applied) and manage.py migrate app 0050_migration_from_master will unapply too much (specifically, all the subsequent migrations from master).There’s only this extremely hacky workaround:

migrate <app> <first migration of feature branch>

migrate --fake <app> <last common migration>

migrate --fake <app> <first migration of feature branch>

migrate <app> <last common migration>

migrate --fake <app> <last master migration>

Reversing the migrations does not present a potential for DB state corruption because already existing Django mechanisms prevent unapplying a migration if its children are still applied. Indeed, Django will happily reverse the feature branch migration chain... all except the oldest migration. 

What are the possible pitfalls with allowing Django to reverse a migration chain, starting with most recent applied migration and ending with a certain ancestor of that migration?

The ticket suggests a problem in case someone reverses a migration chain, deletes migration files and proceeds with committing new work to that branch. This is indeed an error-prone approach that should not be supported, for the primary reason that someone who had checked out that branch earlier and applied its migrations would be stuck. However, what’s wrong with the possibility of simply switching from that branch without ever deleting migration files?

What concerns are there pertaining to a possibility of migrating back to a node that has multiple children (currently unsupported) that do not apply to the possibility of migrating back to a node that has only one child (currently supported)?

Ticket: https://code.djangoproject.com/ticket/32267

PR: https://github.com/django/django/pull/13781

-- 

WBR

Roman.

[Adam Johnson]

manage.py migrate app 0050_migration_from_master will unapply too much (specifically, all the subsequent migrations from master)

I don't follow you. This is exactly the thing I do, migrating back to the last common migration.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/85a20b24-3f91-412d-9bce-664ba835d576n%40googlegroups.com.

--

Adam

[Roman Odaisky]

On Wednesday, 16 December 2020 23:38:55 EET Adam Johnson wrote:
> > manage.py migrate app 0050_migration_from_master will unapply too much
> > (specifically, all the subsequent migrations from master)
>
> I don't follow you. This is exactly the thing I do, migrating back to the
> last common migration.

But the problem with this is, this will unnecessarily unapply and reapply all
the master migrations, and it might not even be possible if some of those are
non-reversible, or only reversible with data loss.

[James Bennett]

On Wed, Dec 16, 2020 at 1:39 PM Adam Johnson <m...@adamj.eu> wrote:
>> manage.py migrate app 0050_migration_from_master will unapply too much (specifically, all the subsequent migrations from master)
>
>
> I don't follow you. This is exactly the thing I do, migrating back to the last common migration.

I think the issue being described is if other apps have migrations
that *should* be applied in the main branch, but would be unapplied by
migrating back to app/0050. So migrating to app/0050 does not actually
restore the state the main branch expected.

I'm also not sure this is a thing that Django can automate in any way
that's likely to be reliable; too many migrations in large projects
have complex interdependencies, a lot of migrations aren't reversible,
etc.

[Roman Odaisky]

On Thursday, 17 December 2020 02:28:02 EET James Bennett wrote:
> On Wed, Dec 16, 2020 at 1:39 PM Adam Johnson <m...@adamj.eu> wrote:
> >> manage.py migrate app 0050_migration_from_master will unapply too much
> >> (specifically, all the subsequent migrations from master)>
> > I don't follow you. This is exactly the thing I do, migrating back to the
> > last common migration.
> I think the issue being described is if other apps have migrations
> that *should* be applied in the main branch, but would be unapplied by
> migrating back to app/0050. So migrating to app/0050 does not actually
> restore the state the main branch expected.

Suppose all these are currently applied:

master ...---0050---0051a---0052a
\ \
feature 0051b---0052b---0053m---0054b

The issue is about unapplying 0054b, 0053m, 0052b and 0051b while not touching
the master migrations. Let’s ignore possible dependencies from other apps for
now.

> I'm also not sure this is a thing that Django can automate in any way
> that's likely to be reliable; too many migrations in large projects
> have complex interdependencies, a lot of migrations aren't reversible,
> etc.

Django will happily reverse all the way to 0051b without giving any thought to
the above concern. The only thing it won’t do is unapply that one last
migration. Why can’t we add that one step?

[Adam Johnson]

Ah right, thanks for elaborating. I normally stick with a single app and a linear migration history.  Avoiding merge migrations is good for a number of reasons and avoids the situation your diagram covers - see my recent package django-linear-migrations which help with that: 

https://adamj.eu/tech/2020/12/10/introducing-django-linear-migrations/ .

In general though app interdependencies and irreversible migrations complicate this, as James says.

There’s a tool for using DB “snapshots” locally to save/restore DB state when switching branches - it’s called stellar: 

https://github.com/fastmonkeys/stellar . I think this is the only way to be reliable. If we were going to make something like this that is Django specific, we’d normally want to have a working, battle-tested third party implementation before considering merging it to core.

--
You received this message because you are subscribed to the Google Groups "Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/2075285.l7tNb9eccj%40xps.

--

Adam

[Roman Odaisky]

> If we were going to make something like this that is Django
> specific, we’d normally want to have a working, battle-tested third party
> implementation before considering merging it to core.

Let me try to reformulate it in different terms.

It’s not about some new big workflow.

It’s not about some new big feature.

It’s not about a major change to an existing feature.

It’s about fixing an off-by-one error in the `migrate` management command.

The underlying machinery is 100% comfortable with doing the thing being
discussed, there’s just no way to instruct it to do so using manage.py migrate.
If there are 10 normal migrations and 10 merge migrations, the `migrate`
command will unapply 19 of those without incident, but it can’t unapply the
last one, simply because the command line interface is insufficient to tell it
what exactly needs to be done. That’s because when reversing, the command line
argument is not the migration to reverse but rather the first migration not to
reverse, and that’s ambiguous.

That’s all there is to it. The patch basically adds one line in an `if`:
https://github.com/django/django/pull/13781/files#diff-973e0df4f8467492bce02726075ea76ca3f574945cc3b9f81247758dce7e6b0fR43
and the rest is boilerplate.

It’s as though the interface to a chess program would accept all moves except
the lexicographically first one, which happens to be Ba1, and we would be
arguing that a1 is a poor square for a bishop and that players should not play
such a move.

The suggested change does not introduce any new data integrity concerns, it
merely allows the player to play any legal move.

Does this clarify the intent of the patch?