Hi Tim,
The API doesn't assume anything, it just gives to the router all the
information that's available, which is limited to connection_alias and
app_label in the case of RunPython/RunSQL.
Previously all RunPython/RunSQL operations would run on every db no matter
what, this is obviously broken for many multi-db setups.
With this branch at the very least you'd get the app_label which is sufficient
for routing in most cases.
If you need even more granularity then you need the operations to give you some
hints. For example a RunPython operation could provide the list of affected
models with `hints={'affected_models': [ModelA, ModelB]}`, but when you need
this level of control over routing, you hopefully also control the app so you
can easily provide the hints that you need.
--
Loïc
On Feb 18, 2015, at 00:16, Tim Graham <timogra...@gmail.com> wrote:
>
> I'm not a big user of multi-db so I could be wrong here, but it seems like
> this API seems to assume that all models in a given app are stored in the
> same database. Have you thought through what happens if this isn't true? This
> question seems to come into play when we allowed model=None in RunPython/SQL.
>
> On Tuesday, February 17, 2015 at 5:06:24 AM UTC-5, Loïc Bistuer wrote:
> Hi all,
>
> The proposed fix for the release blocker
> https://code.djangoproject.com/ticket/24351 suggests changing the signature
> of the `allow_migrate` database router.
>
> From:
> def allow_migrate(self, db, model):
>
> To:
> def allow_migrate(self, db, app_label, model, **hints):
>
> We need to make a design decision.
>
> Some background:
>
> 1. Django 1.7 doesn't call the `allow_migrate` router for RunPython and
> RunSQL operations. If you control all migrations you can somewhat work around
> the issue inside RunPython using
> https://github.com/django/django/blob/1.7.4/docs/topics/migrations.txt#L472-L500,
> but you still have to stay away from RunSQL, and you are out of luck if
> these operations live in django.contrib or in a third-party app.
>
> 2. Behavior from 1. changed in Django 1.8 (#22583) and the RunPython and
> RunSQL operations now call `allow_migrate` with `model=None`. These
> operations also accept a `hints` kwarg that is passed as `**hints` to
> `allow_migrate`. Unless hints are provided `allow_migrate` can only make a
> blanket yes/no decision because it doesn't have anything other than the
> `db_alias` to work with.
>
> 3. The change from 2. is backwards incompatible in a sense that routers will
> blow up if they don't expect None as a possible value for `model`. The
> `hints` part is backwards compatible as long as no migrations make use of
> them.
>
> 4. Django 1.8 introduced a migration for contrib.contenttypes that contains a
> RunPython operation
> (https://github.com/django/django/blob/b4ac23290772e0c11379eb2dfb81c750b7052b66/django/contrib/contenttypes/migrations/0002_remove_content_type_name.py#L33-L36).
> This is a problem because it won't work in many multi-db setups. The problem
> already existed for third-party apps but now it becomes a core issue.
>
> 5. We could take advantage of 2. to fix 4. (e.g. `RunPython(noop,
> add_legacy_name, hints={'app_label': 'contenttypes'})`) which would allow
> multi-db users to make a routing decision, but then we introduce a backwards
> incompatibility (see point 3.) that requires changing the signature of
> `allow_migrate` in user code (to expect either the `app_label` kwarg or
> `**hints`). While this would fix the problem for contrib.contenttypes, the
> problem would still exist in third-party apps unless they also cooperate by
> providing similar hints (note that those that still want to support 1.7
> projects won't be able to do so).
>
> 6. `app_label` applies to all operations including RunPython and RunSQL,
> `model` only applies to some model operations (CreateModel, AddField, etc.).
> Our docs and tests only use the `model` argument to dig out the `app_label`,
> and I suspect this is the most common usage (at least it certainly matches my
> usage).
>
> My opinion is that `app_label` is always useful in `allow_migrate`, even if
> only as a first pass before inspecting the model, so since we are introducing
> a change in the signature of `allow_migrate`, we might as well bite the
> bullet and make it a required argument. The latest version of my branch
> https://github.com/django/django/pull/4152 does this while still supporting
> the old signature during a deprecation period.
>
> An alternative that was proposed by Markus is to pass `app_label` as a hint
> instead of arg from within `allowed_to_migrate`. I don't think it's a good
> idea because it's just as invasive (the signature of existing routers still
> need to change) and we make it less obvious to people calling
> `router.allow_migrate()` directly that they should supply an `app_label`.
> It's IMO more error prone if we can't reliably expect that `app_label` will
> be provided because it requires writing additional defensive code.
>
> Thoughts?
>
> --
> Loïc
>
>
> --
> 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-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> Visit this group at http://groups.google.com/group/django-developers.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/fbaf40ce-f2de-41e5-9bf5-3c15a411f012%40googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
--
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-developers/EB4AD3D6-4C8C-4D28-A140-B2958798B682%40gmail.com.
For more options, visit https://groups.google.com/d/optout.
