On Thu, Jan 9, 2014 at 12:04 AM, Andrew Godwin <and...@aeracode.org> wrote:
> So, the last major bug left in migrations that I'm aware of (bar the
> completion of the GIS backends) is dealing with swappable models - in
> particular, of course, AUTH_USER_MODEL.
>
> As long as you're not using any third-party apps, then everything works
> fine, but as soon as third-party apps with migrations come into the picture
> there's an issue. These apps have to ship with migrations that reference
> any project's current user model - and the way swapping is currently done
> means that ForeignKeys end up with concrete references to the user model
> _at the time the migration was made_.
>
> There are a couple of potential approaches here:
>
> 1) Introduce a new value that can be used as a "to" parameter on
> ForeignKeys which resolves to a swapped model in its internal
> string-to-model converter. I'm thinking something like
> "__swapped__.AUTH_USER_MODEL" - I would use settings here, but I can
> imagine people having an app called settings.
>
> The problem here is I'd then have to make ForeignKey deconstruct to this
> special value if the model it was pointing to was the replacement model -
> whether or not it's got settings.AUTH_USER_MODEL in the models.py file
> (there's no way to tell if it has an actual string or a setting reference
> in its models.py declaration). This means that some FKs might be too
> eagerly converted into swapped references.
>
> 1b) Like 1), but rather than having the value automatically inserted,
> require apps to actually use this new string rather than a direct reference
> to settings.AUTH_USER_MODEL, meaning we can tell if it's swapped or not and
> deconstruct it appropriately.
>
> 2) Change ForeignKey's constructor to automatically point itself to the
> new swapped-in model if it's pointing to the model that was swapped out (so
> ForeignKey("auth.User") automatically becomes
> ForeignKey("myapp.CustomUser") internally.
>
>
> Now, I prefer the cleanliness of 2), but I suspect there was a reason this
> wasn't done when swappable models were first introduced; that said, I
> haven't seen any coding patterns in the wild this would disrupt.
>
The implementation of swappable models is just exploiting the pre-existing
late-binding feature of Django's model tools - a string reference to a
model class is always valid, and will be resolved at runtime;
settings.AUTH_USER_MODEL is just a string. Migrations notwithstanding,
We didn't automagically convert auth.User into myapp.CustomUser because
this acts as a validation aid. There's no guarantee that myapp.CustomUser
adheres to the same user contract as auth.User. If a third party app
references user.username, the app will break in production if you supply a
user model that doesn't have a username field. A hard reference to a
swapped model (either as auth.User *or* as "auth.User") can be identified
as a validation issue on startup, flagging that the third party app isn't
ready to support swappable user models. An app author can then signal that
they're swappable-user ready by making the code change replacing the hard
reference with a settings.AUTH_USER_MODEL reference.
So - there's a reason not to do (2) IMHO.
Functionally, the problem is that ForeignKey(myauth.CustomUser) and
ForeignKey(settings.AUTH_USER_MODEL) are both legal foreign key references,
and if AUTH_USER_MODEL=CustomUser, then they both point to the same model -
but only the second is a "swappable" reference.
In the internals, this resolves to the fact that while auth.User knows that
it's been swapped out (and can identify this fact), but FKs to
myauth.CustomUser don't know if this particular reference is a hard
reference to a particular model, or a swapped in reference.
As an approach, (1b) concerns me a little - we've talking about a feature
in the wild that's only been out there for 2 releases, and required third
party apps to adapt in response to that change - now we're asking them to
adapt again to flag that the reference is swappable. It's a very small
change, but it feels like code churn to me, with the potential to upset the
wider community.
My preference would be (1), but with a metadata clarification.
We are in a position to make an educated guess that a FK reference was to a
swappable model. If the FK definition uses a string, and that string
matches the definition of one of the SWAPPABLE meta declarations, then we
can say the FK is a reference to a swappable.
This will make an incorrect guess in one specific case -- String-based
explicit FK references to the replacement model. (i.e.,
ForeignKey("myauth.CustomUser") )
However, IMHO, this is an edge case. Meta.SWAPPABLE is an undocumented
feature; so in practice, we're only talking about User models here. I'm
having difficulty imaging why you'd have an explicit reference to a
replacement model *and* use it as a swappable.
Given that it's an edge case, lets handle it as one -- assume our guesses
will be right, and make this opt-out for the cases where it may be a
problem. If you've got an explicit FK reference to a replacement model, and
it *isn't* a swappable reference, require a ForeignKey("myauth.CustomUser",
swappable=False) definition. Allow swappable=True if someone wants to be
explicit; but default to swappable=None, which uses the educated guess
approach. This is essentially 1b, but turning the direction around -- given
the use case and the existence of historical code, lets assume we can pick
the right thing, but provide tools to be explicit.
Then, as a version upgrade check, flag any reference that hits the educated
guess marker. The validation framework (soon to hit trunk… promise…) will
let us flag this at a low warning level, and provide a way to silence the
warning once the update path has been validated.
The only implementation hiccup - there's not currently any way to identify
if a FK reference was provided as a string. However, the very first thing
that the FK constructor does is check to see if the reference is a model or
a string - we just don't persist this metadata. However, even if we ignored
this, and just used the resolved model reference, the cross section of
affected references would be "explicit references to the replacement
model", which is, IMHO, and equally narrow cross section.
Yours,
Russ Magee %-)
--
You received this message because you are subscribed to the Google Groups
"Django developers" 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/CAJxq848VcRg0ZHfptuXzg0n17qNbom79k_2LhhTAPPYggfWJLw%40mail.gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.
