RunPython/RunSQL operations in contrib migrations and multi-db routers.

[Loic Bistuer]

By default the RunPython and RunSQL don't give much information to the multi-db router, and before #22583 the router wasn't even consulted.

In Django 1.7 multi-db users could avoid issues by not using those operations and avoiding any 3rd party app that uses them. But in Django 1.8 the contenttypes application adds a RunPython operation that will fail in many multi-db setups (​see commit).

To fix the immediate issue we can leverage the feature added by #22583 and provide hints to the router:

RunPython(foo, bar, hints={'app_label': 'contenttypes'})

and/or

RunPython(foo, bar, hints={'affected_models': [ContentType]})

Another option would be to make allowed_to_migrate always provide allow_migrate with the app_label for the current migration.

Both options would be backwards incompatible in the same way (although the first one is constrained to people that use django.contrib.contenttypes) because they require routers to accept the app_label kwarg or the new **hints syntax (​see release notes).

Note that #22583 already introduced a backward incompatibility (​see release notes) so people already have to update their existing routers for 1.8.

The advantage of the second option is that it'll automatically work for RunPython and RunSQL operations from 3rd party apps.

[Loic Bistuer]

Tentative PR ​https://github.com/django/django/pull/4152.

[Markus Holtermann]

Looking at the PR, I'm not sure we really need the app label on every allow_migrate call. Do we? I'd rather go with hints and put app_label and model_name in there. Or at least make app_label an optional argument in which case we can use inspect.getcallargs() to check if the respective function allows for that argument.

[Loic Bistuer]

To summurize what I mentioned on IRC:

Every migration and operation have an app_label but not necessarily a model so I think it'd be unfortunate to make the former an optional keyword arg while the latter is a required positional arg.

app_label always make sense when routing migrations, even if only as a first pass, not having it readily available requires convoluted code, see ​without app_label vs ​with app_label.

Existing routers need to be updated for Django 1.8 to handle model == None, since any router that inspects model._meta will blow up, so now is a good opportunity to get the API right.

inspect.getcallargs won't work because we document calling allow_migrate directly.

[Loic Bistuer]

I've reworked the patch to allow a deprecation cycle for the old signature using inspect; things are unsurprisingly easier when I'm not half-asleep.

Of course it's still debatable whether or not allow_migrate(self, db, app_label, model, **hints) is the right API (I believe it is). I've started working on a ML post but it needs more work to properly lay out context and pros and cons of alternatives, I'm running out of time for today so it may have to wait for this weekend.

[Loic Bistuer]

ML thread: ​https://groups.google.com/d/topic/django-developers/9SedacGgyLY/discussion.

[Loic Bistuer <loic.bistuer@…>]

In bed504d70bede3431a213203c13a33905d6dbf77:

Fixed #24351, #24346 -- Changed the signature of allow_migrate().

The new signature enables better support for routing RunPython and
RunSQL operations, especially w.r.t. reusable and third-party apps.

This commit also takes advantage of the deprecation cycle for the old
signature to remove the backward incompatibility introduced in #22583;
RunPython and RunSQL won't call allow_migrate() when when the router
has the old signature.

Thanks Aymeric Augustin and Tim Graham for helping shape up the patch.

Refs 22583.

[Loic Bistuer <loic.bistuer@…>]

In 3a6c37fce452a3bbf185b5e58dd3e7c89b456b19:

[1.8.x] Fixed #24351, #24346 -- Changed the signature of allow_migrate().

The new signature enables better support for routing RunPython and
RunSQL operations, especially w.r.t. reusable and third-party apps.

This commit also takes advantage of the deprecation cycle for the old
signature to remove the backward incompatibility introduced in #22583;
RunPython and RunSQL won't call allow_migrate() when when the router
has the old signature.

Thanks Aymeric Augustin and Tim Graham for helping shape up the patch.

Refs 22583.

Conflicts:

django/db/utils.py

Backport of bed504d70bede3431a213203c13a33905d6dbf77 from master

[Loic Bistuer <loic.bistuer@…>]

In fa5f936b4803416d1b16e7171dce8df535d13766:

Fixed allow_migrate signature in one of the tests.

Refs #24351.

[Tim Graham <timograham@…>]

In 4fd264b6:

Refs #24351 -- Removed support for the old allow_migrate() signature per deprecation timeline.