Re: Migrations and swappable models/AUTH_USER_MODEL

[Andrew Godwin]

Good points. Regarding 1), I can't specially control reconstruction of
fields from migrations and raise errors only in that case - it's just
calling the normal __init__. Since I'm going to have to write out a call to
settings.AUTH_USER_MODEL anyway - the whole point of swappable=True is so
the migration serialiser knows to do that - having swappable be True or
False makes no difference (the only behaviour it affects is the migration
serialiser, for now, and that's never run on re-serialised models).
Nevertheless, I'll make sure it's forced to True just in case.

On 2), that's an issue I hadn't thought of. There's special syntax for
depending on the first migration of an app which I could inject, but it'd
have to be chosen at migration creation time, so not terribly useful. The
only concrete way to make sure this works is to have a dependency that's
defined based on the setting, e.g.

class Migration(migrations.Migration):
dependencies = [
("comments", "0001_initial"),
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
]

That function would just be lambda value: (value.split(".", 1)[0],
"__first__") - we already have the __first__ special migration name which
resolves to the start of any app (it's used for depending on apps with no
migrations at the moment, in case they gain them in future)

Andrew


On Sat, Jan 11, 2014 at 9:43 PM, Shai Berger  wrote:

> On Saturday 11 January 2014 11:45:55 Andrew Godwin wrote:
> > OK, I like that approach Russ - an inverted 1b. It'll be a bit harder to
> > explain in the docs, but it won't catch anyone out unawares, which is the
> > key thing.
> >
> I like it too.
>
> And foot-in-mouth re referencing swappables notwithstanding, there's two
> points I want to raise:
>
> 1) This approach is good for the models files; I think a more explicit
> approach
> should be taken for the migration files, where we don't currently have to
> worry
> about backwards compatibility. I'd like to see swappable=None in the model
> transformed to swappable=True in the migration, and swappable=None in the
> migration treated as an error.
>
> In particular, I don't like Marc's idea about migrations continuing to work
> when a target model, which used to be non-swappable, is being swapped out.
> The
> app's "main" code won't continue to work in this scenario, for reasons Russ
> has noted; I don't see why migrations should not be treated as part of the
> app's code.
>
> 2) Swappability of foreign-key targets in migrations may induce dynamic
> migration dependencies (if my migration creates a foreign key to the user
> model, and that model happens to be in a migrated app, my migration now
> depends on some -- not even necessarily the initial -- migration in that
> app;
> we'll only know at migrate time, not at makemigration time). I'd like to
> make
> this more explicit too -- e.g. add some placeholder, or a function call to
> calculate the dependency, in the generated list of dependencies. Although
> in
> principle it can be induced from the operations, I think it would make
> possible migration dependency issues much easier to troubleshoot.
>
> An alternative would to decree that swappable-replacement models cannot
> live
> in migrated apps; I think that is a very bad option.
>
> Shai.
>
> --
> 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/138953391.ZAfFGT0eZV%40deblack
> .
> For more options, visit https://groups.google.com/groups/opt_out.
>

-- 
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/CAFwN1upCnbp4%2B0fN0qsENAunKLWaUXZE-hSdW49icmfxBpeL%3DA%40mail.gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Migrations and swappable models/AUTH_USER_MODEL

[Shai Berger]

On Thursday 09 January 2014 01:10:09 Raphaël Barrois wrote:
> On Wed, 8 Jan 2014 20:27:54 +0200
> 
> Shai Berger  wrote:
> > as people already use auth.get_user_model() -- so, you can
> > monkeypatch *that* when generating migrations, and let them just keep
> > doing what they do today.
> 
> Actually, people don't use auth.get_user_model() in their models
> declaration, they use ``settings.AUTH_USER_MODEL``, as described in the
> documentation:
> https://docs.djangoproject.com/en/1.6/topics/auth/customizing/#referencing-> 
> the-user-model
> 

Yes, I stand corrected.
> 
> Actually, you have two problems with a third-party app:
> 
> - It needs to have a way to specify "I want a FK to whatever the user has
> chosen for a AUTH_USER_MODEL", no matter the *type* of its PK field. =>
> This mustn't depend on what the third-party-app developer's actual
> AUTH_USER_MODEL setting was while generating the migration 

As I've said in response to another mail in this thread, a variation on 
Russell's suggestion, where the migration adds a ForeignKey with 
swappable=True, should allow this to be done.

> if the user changes the model, this should be helped by migrations, for
> instance by put a warning e.g "Looks like you're changing a swappable
> model, where should I put automatic related migrations ? (these migrations
> shouldn't go into the app with the FKey to AUTH_USER_MODEL, which may be
> out of the user's control — e.g third party apps).
> 
I don't think related migrations should be generated automatically. There are 
too many issues -- besides third-party apps, there are non-migrated apps. 
Editing an app's models under its feet is very bad practice.

> Another problem is apps that provide a swappable model whose default is
> AUTH_USER_MODEL but that may be customized through their own setting, for
> instance ``return getattr(settings, 'MY_TARGET_MODEL',
> settings.AUTH_USER_MODEL)``.

No, such apps should be using the same getattr call  in their migrations -- 
that is, this is one of the cases where editing migration files manually should 
be required; and it doesn't matter if the default  target model happens to be 
swappable. It is enough that a referenced model is decided by settings.

On a related note, this strengthens the case for explicit references to 
dynamic dependencies; a migration for such an app should, IMO, look something 
like:

from django.db import migrations, models
from django.conf import settings

my_target_model = getattr(settings, 'MY_TARGET_MODEL',
  settings.AUTH_USER_MODEL)

class Migration(migrations.Migration):

dependencies = [("migrations", "0001_initial"),
migrations.model_created(my_target_model)]

operations = [
migrations.DeleteModel("Tribble"),
migrations.AddField("MyModel", "target",
models.ForeignKey(my_target_model)),
]

On swappable models, the dynamic dependency could be inferred automatically 
from the operations; in this case, where the target model is not (or at least 
not necessarily) swappable, the placeholder reference is required.

Shai.

-- 
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/3541296.khKWS2QS0i%40deblack.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Migrations and swappable models/AUTH_USER_MODEL

[Shai Berger]

On Saturday 11 January 2014 11:45:55 Andrew Godwin wrote:
> OK, I like that approach Russ - an inverted 1b. It'll be a bit harder to
> explain in the docs, but it won't catch anyone out unawares, which is the
> key thing.
> 
I like it too.

And foot-in-mouth re referencing swappables notwithstanding, there's two 
points I want to raise:

1) This approach is good for the models files; I think a more explicit approach 
should be taken for the migration files, where we don't currently have to worry 
about backwards compatibility. I'd like to see swappable=None in the model 
transformed to swappable=True in the migration, and swappable=None in the 
migration treated as an error.

In particular, I don't like Marc's idea about migrations continuing to work 
when a target model, which used to be non-swappable, is being swapped out. The 
app's "main" code won't continue to work in this scenario, for reasons Russ 
has noted; I don't see why migrations should not be treated as part of the 
app's code.

2) Swappability of foreign-key targets in migrations may induce dynamic 
migration dependencies (if my migration creates a foreign key to the user 
model, and that model happens to be in a migrated app, my migration now 
depends on some -- not even necessarily the initial -- migration in that app; 
we'll only know at migrate time, not at makemigration time). I'd like to make 
this more explicit too -- e.g. add some placeholder, or a function call to 
calculate the dependency, in the generated list of dependencies. Although in 
principle it can be induced from the operations, I think it would make 
possible migration dependency issues much easier to troubleshoot.

An alternative would to decree that swappable-replacement models cannot live 
in migrated apps; I think that is a very bad option.

Shai.

-- 
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/138953391.ZAfFGT0eZV%40deblack.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Migrations and swappable models/AUTH_USER_MODEL

[Andrew Godwin]

OK, I like that approach Russ - an inverted 1b. It'll be a bit harder to
explain in the docs, but it won't catch anyone out unawares, which is the
key thing.

Andrew


On Thu, Jan 9, 2014 at 1:28 AM, Russell Keith-Magee  wrote:

>
>
>
> On Thu, Jan 9, 2014 at 12:04 AM, Andrew Godwin 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.,
> 

Re: Migrations and swappable models/AUTH_USER_MODEL

[Russell Keith-Magee]

On Thu, Jan 9, 2014 at 12:04 AM, Andrew Godwin  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 

Re: Migrations and swappable models/AUTH_USER_MODEL

[Andrew Godwin]

> I don't see how "you need to remember that auth.User really means some
> model
> defined in settings" is more user friendly than "you need to remember that
> __swappable__.X means that X is swappable". More so, as people already use
> auth.get_user_model() -- so, you can monkeypatch *that* when generating
> migrations, and let them just keep doing what they do today.
>

That's not what people use, they use settings.AUTH_USER_MODEL. See
Raphaël's email about that.

No, not with any of the three suggestions. The migrations machinery doesn't
> introspect the database to find the FK targets -- it re-rolls the
> migrations in
> memory; and if any of the suggestions is taken, then when you run the
> migrations with the setting pointing at some model, it will seem like it
> has
> always been that way.
>

No, as the setting only exists in the models file - by the time it gets
into a migration (the things which are re-run) it's been baked into a real
model reference (of whatever the setting was at the time). Option 1b would
preserve this behaviour by default; the other two eradicate it and make
history changing of AUTH_USER_MODEL invisible in two different ways.


>
> > The problem is when the migrations are pre-made - i.e. from third-party
> > apps - and that's the issue I'm trying to solve.
> >
>
> The two problems are in conflict -- for evolving the user model, history
> must
> matter; for pre-made migrations, the choice of user model needs to look
> like
> it has always been the way it is "now" (at the time the migrations are
> run).


Yes, and I personally think the pre-made migrations problem is more
important. I don't think people should change user model mid-project on a
whim - there's a lot of manual work they'd always have to do to port all
their user data over losslessly ignoring just this foreign key problem -
and if they do, I think we recommend they start again with their migrations.

The only solution that I can see which somehow gives something to both
sides - 1b - makes it really, really easy for third-party apps to ship
broken migrations and not know about it, and I don't like that. I think we
have to sacrifice being able to tell that the user model was changed in
favour of having the app ecosystem actually work.

Andrew

-- 
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/CAFwN1urJpcV%2B%2BTU0tGHaRFiR0nfJWzngsChtpnuk%3Dj_YAmXd6A%40mail.gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Migrations and swappable models/AUTH_USER_MODEL

[Raphaël Barrois]

On Wed, 8 Jan 2014 20:27:54 +0200
Shai Berger  wrote:

> On Wednesday 08 January 2014 20:00:25 Andrew Godwin wrote:
> > > Instinctively, I'm almost -1 on 2); I'm not too happy about 1)
> > > either. If a model says it has a FK to auth.User, that shouldn't
> > > mean anything other than
> > > auth.User. I don't see that as cleanliness -- it's effectively
> > > monkeypatching.
> > > 
> > > 1b seems to me like the most reasonable option.
> > 
> > It's also the least user-friendly option. I don't see the same
> > problem with 2) - sure, the model says it has an FK to auth.user,
> > and your settings say that auth.user is swapped out in favour of
> > another model. I think it would actually be worse if we kept it
> > like it is - since auth.user is swapped out, it shouldn't have a
> > table made for it, so how are you even going to make an FK to it?
> > (this is how migrations currently fails - there's no auth.user left
> > for it to refer to).
> > 
> 
> I don't see how "you need to remember that auth.User really means
> some model defined in settings" is more user friendly than "you need
> to remember that __swappable__.X means that X is swappable". More so,
> as people already use auth.get_user_model() -- so, you can
> monkeypatch *that* when generating migrations, and let them just keep
> doing what they do today.

Actually, people don't use auth.get_user_model() in their models declaration, 
they use ``settings.AUTH_USER_MODEL``,
as described in the documentation: 
https://docs.djangoproject.com/en/1.6/topics/auth/customizing/#referencing-the-user-model

The actual user model may not be prepared at the time the ``models.py`` is 
loaded, thus failing to import.
>From a migrations point of view, this means that the ForeignKey is declared 
>exactly as if the user had actually, manually written 
>``ForeignKey('auth.User')`` instead of ``settings.AUTH_USER_MODEL``.

I think that's why Andrew suggests to either use a special placeholder 
(``__swapped__.X``) or to automatically detect swapped models from their actual 
``class Meta: swappable=True`` declaration.

> 
> > > However, I've had a different thought about this:
> > > > As long as you're not using any third-party apps, then
> > > > everything works fine
> > > 
> > > No, it doesn't really. You can't change the user model as part of
> > > the project
> > > evolution. Supporting this raises a whole set of additional
> > > problems -- even
> > > with 1b, not to mention the scenarios where we try to guess
> > > swappables from concrete models that happen to be their sources
> > > or targets.
> > 
> > Changing the user model during the project's lifespan is a
> > different task. Migrations will nominally support it - in that
> > it'll change all your FK constraints to immediately point to the
> > new table - but you still have to manage moving the data into that
> > table yourself, and I'm not sure we can do it any other way.
> > Migrations sees you changing the user model as an actionable
> > change, since as far as it's concerned you changed a load of "to"
> > arguments of foreignkeys when you changed the setting, and so it
> > can make migrations for that.
> > 
> 
> No, not with any of the three suggestions. The migrations machinery
> doesn't introspect the database to find the FK targets -- it re-rolls
> the migrations in memory; and if any of the suggestions is taken,
> then when you run the migrations with the setting pointing at some
> model, it will seem like it has always been that way.
> 
> > The problem is when the migrations are pre-made - i.e. from
> > third-party apps - and that's the issue I'm trying to solve.
> > 
> 
> The two problems are in conflict -- for evolving the user model,
> history must matter; for pre-made migrations, the choice of user
> model needs to look like it has always been the way it is "now" (at
> the time the migrations are run).

Actually, you have two problems with a third-party app:

- It needs to have a way to specify "I want a FK to whatever the user has 
chosen for a AUTH_USER_MODEL", no matter the *type* of its PK field.
=> This mustn't depend on what the third-party-app developer's actual 
AUTH_USER_MODEL setting was while generating the migration
- Ideally, if the user changes the model, this should be helped by migrations, 
for instance by put a warning e.g "Looks like you're changing a swappable 
model, where should I put automatic related migrations ? (these migrations 
shouldn't go into the app with the FKey to AUTH_USER_MODEL, which may be out of 
the user's control — e.g third party apps).

Another problem is apps that provide a swappable model whose default is 
AUTH_USER_MODEL but that may be customized through their own setting,
for instance ``return getattr(settings, 'MY_TARGET_MODEL', 
settings.AUTH_USER_MODEL)``.


-- 
Raphaël

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop 

Re: Migrations and swappable models/AUTH_USER_MODEL

[Shai Berger]

On Wednesday 08 January 2014 20:00:25 Andrew Godwin wrote:
> > Instinctively, I'm almost -1 on 2); I'm not too happy about 1) either. If
> > a model says it has a FK to auth.User, that shouldn't mean anything
> > other than
> > auth.User. I don't see that as cleanliness -- it's effectively
> > monkeypatching.
> > 
> > 1b seems to me like the most reasonable option.
> 
> It's also the least user-friendly option. I don't see the same problem with
> 2) - sure, the model says it has an FK to auth.user, and your settings say
> that auth.user is swapped out in favour of another model. I think it would
> actually be worse if we kept it like it is - since auth.user is swapped
> out, it shouldn't have a table made for it, so how are you even going to
> make an FK to it? (this is how migrations currently fails - there's no
> auth.user left for it to refer to).
> 

I don't see how "you need to remember that auth.User really means some model 
defined in settings" is more user friendly than "you need to remember that 
__swappable__.X means that X is swappable". More so, as people already use 
auth.get_user_model() -- so, you can monkeypatch *that* when generating 
migrations, and let them just keep doing what they do today.

> > However, I've had a different thought about this:
> > > As long as you're not using any third-party apps, then everything works
> > > fine
> > 
> > No, it doesn't really. You can't change the user model as part of the
> > project
> > evolution. Supporting this raises a whole set of additional problems --
> > even
> > with 1b, not to mention the scenarios where we try to guess swappables
> > from concrete models that happen to be their sources or targets.
> 
> Changing the user model during the project's lifespan is a different task.
> Migrations will nominally support it - in that it'll change all your FK
> constraints to immediately point to the new table - but you still have to
> manage moving the data into that table yourself, and I'm not sure we can do
> it any other way. Migrations sees you changing the user model as an
> actionable change, since as far as it's concerned you changed a load of
> "to" arguments of foreignkeys when you changed the setting, and so it can
> make migrations for that.
> 

No, not with any of the three suggestions. The migrations machinery doesn't 
introspect the database to find the FK targets -- it re-rolls the migrations in 
memory; and if any of the suggestions is taken, then when you run the 
migrations with the setting pointing at some model, it will seem like it has 
always been that way.

> The problem is when the migrations are pre-made - i.e. from third-party
> apps - and that's the issue I'm trying to solve.
> 

The two problems are in conflict -- for evolving the user model, history must 
matter; for pre-made migrations, the choice of user model needs to look like 
it has always been the way it is "now" (at the time the migrations are run).

Shai.

-- 
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/201401082027.55045.shai%40platonix.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Migrations and swappable models/AUTH_USER_MODEL

[Marc Tamlyn]

Personally, I like approach 2. It means that if we (or another 3rd party
app) were to make a model swappable, then migrations which don't know about
this change would continue to work.


On 8 January 2014 18:00, Andrew Godwin  wrote:

>
>> Instinctively, I'm almost -1 on 2); I'm not too happy about 1) either. If
>> a
>> model says it has a FK to auth.User, that shouldn't mean anything other
>> than
>> auth.User. I don't see that as cleanliness -- it's effectively
>> monkeypatching.
>>
>> 1b seems to me like the most reasonable option.
>>
>
> It's also the least user-friendly option. I don't see the same problem
> with 2) - sure, the model says it has an FK to auth.user, and your settings
> say that auth.user is swapped out in favour of another model. I think it
> would actually be worse if we kept it like it is - since auth.user is
> swapped out, it shouldn't have a table made for it, so how are you even
> going to make an FK to it? (this is how migrations currently fails -
> there's no auth.user left for it to refer to).
>
>
>>
>> However, I've had a different thought about this:
>>
>> > As long as you're not using any third-party apps, then everything works
>> > fine
>>
>> No, it doesn't really. You can't change the user model as part of the
>> project
>> evolution. Supporting this raises a whole set of additional problems --
>> even
>> with 1b, not to mention the scenarios where we try to guess swappables
>> from
>> concrete models that happen to be their sources or targets.
>>
>> This is a problem that bugs me personally -- I'm involved in a big project
>> that evolved since Django 1.2. As 1.4 (no swappable user model) is LTS,
>> I'm
>> certain it's not just me.
>>
>
> Changing the user model during the project's lifespan is a different task.
> Migrations will nominally support it - in that it'll change all your FK
> constraints to immediately point to the new table - but you still have to
> manage moving the data into that table yourself, and I'm not sure we can do
> it any other way. Migrations sees you changing the user model as an
> actionable change, since as far as it's concerned you changed a load of
> "to" arguments of foreignkeys when you changed the setting, and so it can
> make migrations for that.
>
> The problem is when the migrations are pre-made - i.e. from third-party
> apps - and that's the issue I'm trying to solve.
>
> Andrew
>
> --
> 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/CAFwN1uoR0h%3D%2BjOH-12%2BCrMnUH9kqYLpYyt-80SSBEA1T0aB5Nw%40mail.gmail.com
> .
>
> For more options, visit https://groups.google.com/groups/opt_out.
>

-- 
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/CAMwjO1H4%3DUOBjwNtmmvBOLR%2BWhWwJkisrMnkSDVJsvYdqOAEng%40mail.gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Migrations and swappable models/AUTH_USER_MODEL

[Andrew Godwin]

>
>
> Instinctively, I'm almost -1 on 2); I'm not too happy about 1) either. If a
> model says it has a FK to auth.User, that shouldn't mean anything other
> than
> auth.User. I don't see that as cleanliness -- it's effectively
> monkeypatching.
>
> 1b seems to me like the most reasonable option.
>

It's also the least user-friendly option. I don't see the same problem with
2) - sure, the model says it has an FK to auth.user, and your settings say
that auth.user is swapped out in favour of another model. I think it would
actually be worse if we kept it like it is - since auth.user is swapped
out, it shouldn't have a table made for it, so how are you even going to
make an FK to it? (this is how migrations currently fails - there's no
auth.user left for it to refer to).


>
> However, I've had a different thought about this:
>
> > As long as you're not using any third-party apps, then everything works
> > fine
>
> No, it doesn't really. You can't change the user model as part of the
> project
> evolution. Supporting this raises a whole set of additional problems --
> even
> with 1b, not to mention the scenarios where we try to guess swappables from
> concrete models that happen to be their sources or targets.
>
> This is a problem that bugs me personally -- I'm involved in a big project
> that evolved since Django 1.2. As 1.4 (no swappable user model) is LTS, I'm
> certain it's not just me.
>

Changing the user model during the project's lifespan is a different task.
Migrations will nominally support it - in that it'll change all your FK
constraints to immediately point to the new table - but you still have to
manage moving the data into that table yourself, and I'm not sure we can do
it any other way. Migrations sees you changing the user model as an
actionable change, since as far as it's concerned you changed a load of
"to" arguments of foreignkeys when you changed the setting, and so it can
make migrations for that.

The problem is when the migrations are pre-made - i.e. from third-party
apps - and that's the issue I'm trying to solve.

Andrew

-- 
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/CAFwN1uoR0h%3D%2BjOH-12%2BCrMnUH9kqYLpYyt-80SSBEA1T0aB5Nw%40mail.gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Migrations and swappable models/AUTH_USER_MODEL

[Shai Berger]

On Wednesday 08 January 2014 18:04:44 Andrew Godwin wrote:
> 
> 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.
> 
> Opinions?
> 
Instinctively, I'm almost -1 on 2); I'm not too happy about 1) either. If a 
model says it has a FK to auth.User, that shouldn't mean anything other than 
auth.User. I don't see that as cleanliness -- it's effectively monkeypatching.

1b seems to me like the most reasonable option.

However, I've had a different thought about this: 

> As long as you're not using any third-party apps, then everything works
> fine

No, it doesn't really. You can't change the user model as part of the project 
evolution. Supporting this raises a whole set of additional problems -- even 
with 1b, not to mention the scenarios where we try to guess swappables from 
concrete models that happen to be their sources or targets.

This is a problem that bugs me personally -- I'm involved in a big project 
that evolved since Django 1.2. As 1.4 (no swappable user model) is LTS, I'm 
certain it's not just me.

Shai.

-- 
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/201401081950.11502.shai%40platonix.com.
For more options, visit https://groups.google.com/groups/opt_out.

Migrations and swappable models/AUTH_USER_MODEL

[Andrew Godwin]

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.

Opinions?

Andrew

-- 
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/CAFwN1uow9u0w_GOS8qRhBmNz%3D6r0vMHXx4guFf967wZeUnDVuQ%40mail.gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.