#27200: router.allow_migrate without a model/hint, what is it supposed to do?
Does
it make sense to make that call?
-------------------------------------+-------------------------------------
Reporter: Joseph Kahn | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.10
Severity: Normal | Resolution:
Keywords: makemigrations, | Triage Stage:
router, allow_migrate | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Shai Berger):
Hi Joseph, sorry for taking so long to respond. I'll answer your questions
in reverse order, starting with the second:
Replying to [comment:8 Joseph Kahn]:
>
> However, there is nothing in the context of a migration to prevent or
guide me away from saying:
> Model A1 is in app A and model B is in app B. When I update A1, I need
to update the corresponding things in B1. When everything is on one
database this makes sense to do as a data migration, is the idea that if I
need to update both together they cannot be on one data migration IF they
are on different shards?
This has always been the case, and it's not because of a Django design
decision, but because if some changes need to be done together, then they
need to be done in a (database) transaction, and that is a little hard to
do when they are on separate databases.
> If they should be done on a data migration than in the context of
looping over one database in the migration, I'm accessing some n other
databases for all the values of B that need to update. I would assume that
you are not supposed to write code like that, where a migration on one
database has some effect on another one?
>
> I'm just struggling a little to understand the intention here in multi-
db scenarios where an update on one db requires another update on another
database. If the Django team has a clear intent then it just needs a more
definite explanation in the docs (in my opinion).
>
Again -- you describe a case where database integrity is broken by design.
I believe the developers who made the design decisions here did not give
such cases a high priority and probably didn't completely think them
through, and I think Django should not encourage such usage, so I would be
against making recommendations about it in the docs.
> edit: I'm in the middle of some stuff right now so sorry if this feels a
bit scattered. Since I'm allowed to access models in other apps inside a
data migration, what if those models don't have complete database overlap.
Or is the intention that if that's the case I'm supposed to only read the
other model but not write to it, since allow_migrate is this context is in
an `app_label` context and should only loop over some of those databases?
I'm not sure if this was super clear so let me know if I need to rephrase
that.
I'm not completely sure I got your meaning, but if I did: When migrations
are run against a given database, we don't expect changes to be made to
other databases. These other databases can still be read, as the
migrations can always use data sources other than the migrated database
(trivially project source files, but also data files, Web API calls,
whatever).
Now, back to the first question:
>
> My understanding is that `allow_migrate(db, app_label)` is equiavlent to
`any(allow_migrate(db, app_label, model_name) for model_name in
get_model_names(app_label))`, where `get_model_names` is a generator for
retrieving all the names of all the models inside an app.
>
> What I'm still a little confused about is:
> 1) Why not keep the strict requirement and write the code like the above
loop? Requiring each statement to be for one model?
> note: I can easily make that my implementation inside allow_migrate so
it's not that this isn't doable.
>
I think the current API is a better reflection of the decisions needed to
be made, and so gives better hints to the authors of routers.
As an example: Suppose I have an app which has several models, but only
one of them, say `ModelA`, is stored on database `dbA`. If the API
requires a model name as you suggest, the natural thing for me to do is
something like
{{{
def allow_migrate(self, db, app_label, model_name):
if app_label==my_app_label:
return ((db=='dbA') == (model_name=='modela'))
else:
return None
}}}
I might be surprised to find out that this allows all `RunPython` and
`RunSQL` operations in migrations in the app on `dbA`, which is a
consequence of your suggestion. On the other hand, the current API tells
me explicitly: There are cases when you need to decide without reference
to a specific model, where the closest general hint we can give the router
is the app label.
On second thought, though, it may be worth reconsidering if we want the
reply for the `makemigrations` case and the reply for the `RunSQL` case to
always be the same. In the specific example I just gave, we probably
don't, and it would be served better if `makemigrations` implemented the
loop instead of calling the no-model version, or at least, gave a hint
that this call is asking about the need to run a consistency check, and
not about the need to change the database.
I guess I'm replying to [comment:9 Tim]'s question about going forward
with "we should probably be adding a hint to the call in makemigrations".
I'm not sure if we should backport that fix to 1.10.x, but I think that is
an option too, since this would be a fix to a new feature.
--
Ticket URL: <https://code.djangoproject.com/ticket/27200#comment:10>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/064.e3459a6098ae4d872acda363421e3719%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
