Re: Draft branch: Swappable User models in contrib.auth

2012-08-23 Thread Russell Keith-Magee
On Thu, Aug 23, 2012 at 10:16 PM, sergzach  wrote:
> The question about databases.
>
> Do I understand correctly that if we create a MyUser class (as in your
> example) then extra fields (e.g. date_of_birth) will be stored in the same
> table of a database with inherited fields (from AbstractBaseUser)?

The example I provided will result in the creation of a single MyUser
table that contains all the fields that will be needed to operate as a
user in Django. The auth_user table (from auth.User) *won't* be
created; and the AbtractBaseUser doesn't cause the creation of a table
at all.

Yours,
Russ Magee %-)

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Draft branch: Swappable User models in contrib.auth

2012-08-23 Thread sergzach
The question about databases.

Do I understand correctly that if we create a MyUser class (as in your 
example) then extra fields (e.g. date_of_birth) will be stored in the same 
table of a database with inherited fields (from AbstractBaseUser)?


> === 
> from django.db import models 
>
> from django.contrib.auth.models import BaseUserManager, AbstractBaseUser 
>
> class MyUserManager(BaseUserManager): 
> def create_user(self, email, date_of_birth, password=None): 
> if not email: 
> raise ValueError('Users must have an email address') 
>
> user = self.model( 
> email=MyUserManager.normalize_email(email), 
> date_of_birth=date_of_birth, 
> ) 
>
> user.set_password(password) 
> user.save(using=self._db) 
> return user 
>
> def create_superuser(self, username, password, date_of_birth): 
> u = self.create_user(username, password=password, 
> date_of_birth=date_of_birth) 
> u.is_admin = True 
> u.save(using=self._db) 
> return u 
>
>
> class MyUser(AbstractBaseUser): 
> email = models.EmailField(verbose_name='email address', 
> max_length=255, unique=True) 
> date_of_birth = models.DateField() 
> is_admin = models.BooleanField(default=False) 
>
> objects = MyUserManager() 
>
> USERNAME_FIELD = 'email' 
> REQUIRED_FIELDS = ['date_of_birth'] 
>
> def get_full_name(self): 
> return self.email 
>
> def get_short_name(self): 
> return self.email 
>
> def __unicode__(self): 
> return self.email 
>
> # Admin required fields 
> def has_perm(self, perm, obj=None): 
> return True 
>
> def has_module_perms(self, app_label): 
> return True 
>
> @property 
> def is_staff(self): 
> return self.is_admin 
>
> @property 
> def is_active(self): 
> return True 
> === 
>
>  * Set AUTH_USER_MODEL = 'myauth.MyUser' 
>
>  * Run syncdb. 
>
> At this point, feedback and offers of assistance are most welcome. 
>
> [1] 
> https://code.djangoproject.com/wiki/ContribAuthImprovements#Recommendations 
> [2] https://github.com/freakboy3742/django/tree/t3011 
> [3] http://www.w3.org/International/questions/qa-personal-names 
>
> Yours, 
> Russ Magee %-) 
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To view this discussion on the web visit 
https://groups.google.com/d/msg/django-developers/-/e_eXtydc004J.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Draft branch: Swappable User models in contrib.auth

2012-08-20 Thread Russell Keith-Magee
On Mon, Aug 20, 2012 at 7:11 AM, Russell Keith-Magee
 wrote:
> On Sun, Aug 19, 2012 at 10:02 AM, Victor Hooi  wrote:
>> Hi,
>>
>> I'm just wondering, has there been any updates on the User model refactor?
>>
>> My understanding is that this is the official way of handling Users going
>> forward.
>>
>> Is there any roadmap on when it might hit trunk? I didn't see any reference
>> to User models in the 1.5 release notes.
>
> The precise answer? Soon. Ish. :-)
>
> There's a draft branch which works [1], but requires some more testing
> and documentation. It also hasn't been updated to reflect the recent
> py3k changes.
>
> I'm sprinting at PyCon AU this year, so I'm hoping to at least get the
> branch up to date. I might also be able to convince someone to work on
> the documentation issue.
>
> I still want to get this in for 1.5; it's just a matter of find the spare 
> time.
>
> [1] https://github.com/freakboy3742/django/tree/t3011

A follow up on post-PyCon AU sprint activity -- I've gotten my branch
up to date with Django trunk; there has been some work done on getting
some documentation ready; and a few more pairs of eyes have been over
the code looking for admin integration problems and other assumptions
that I've made that might not shake out. Thanks to everyone at PyCon
AU that pitched in (Thomas Ashelford, Greg Turner, Brenda Moon, Simon
Meers, and a few others).

There's one issue lurking with testing that has some potential to turn
into a yak shaving exercise, but if we shave that yak, we'll have
another big feature to put on the list for Django 1.5. I'll try to
post to django-developers about this in the near future (hopefully
before DjangoCon US).

Yours,
Russ Magee %-)

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Draft branch: Swappable User models in contrib.auth

2012-08-20 Thread Marc Tamlyn
Ah, thanks Russ, I'd missed the final resolution to that.

M

On Monday, 20 August 2012 00:12:18 UTC+1, Russell Keith-Magee wrote:
>
> On Sun, Aug 19, 2012 at 5:23 PM, Marc Tamlyn 
>  
> wrote: 
> > I believe changes to auth (and several other things) are waiting for 
> > contrib.migrations. It will be some time... 
>
> Incorrect. The strategy that was approved for trunk won't require 
> migrations unless you want to change an existing project, which means 
> it will be entirely opt-in. There was an extensive django-dev 
> discussion about 6 months ago about this; the redux is on the wiki: 
>
> https://code.djangoproject.com/wiki/ContribAuthImprovements 
>
> Yours, 
> Russ Magee %-) 
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To view this discussion on the web visit 
https://groups.google.com/d/msg/django-developers/-/YRjWXtBIuD0J.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Draft branch: Swappable User models in contrib.auth

2012-08-19 Thread Russell Keith-Magee
On Sun, Aug 19, 2012 at 5:23 PM, Marc Tamlyn  wrote:
> I believe changes to auth (and several other things) are waiting for
> contrib.migrations. It will be some time...

Incorrect. The strategy that was approved for trunk won't require
migrations unless you want to change an existing project, which means
it will be entirely opt-in. There was an extensive django-dev
discussion about 6 months ago about this; the redux is on the wiki:

https://code.djangoproject.com/wiki/ContribAuthImprovements

Yours,
Russ Magee %-)

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Draft branch: Swappable User models in contrib.auth

2012-08-19 Thread Russell Keith-Magee
On Sun, Aug 19, 2012 at 10:02 AM, Victor Hooi  wrote:
> Hi,
>
> I'm just wondering, has there been any updates on the User model refactor?
>
> My understanding is that this is the official way of handling Users going
> forward.
>
> Is there any roadmap on when it might hit trunk? I didn't see any reference
> to User models in the 1.5 release notes.

The precise answer? Soon. Ish. :-)

There's a draft branch which works [1], but requires some more testing
and documentation. It also hasn't been updated to reflect the recent
py3k changes.

I'm sprinting at PyCon AU this year, so I'm hoping to at least get the
branch up to date. I might also be able to convince someone to work on
the documentation issue.

I still want to get this in for 1.5; it's just a matter of find the spare time.

[1] https://github.com/freakboy3742/django/tree/t3011

Yours,
Russ Magee %-)

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Draft branch: Swappable User models in contrib.auth

2012-08-19 Thread Marc Tamlyn
I believe changes to auth (and several other things) are waiting for 
contrib.migrations. It will be some time...

On Sunday, 19 August 2012 03:02:51 UTC+1, Victor Hooi wrote:
>
> Hi,
>
> I'm just wondering, has there been any updates on the User model refactor?
>
> My understanding is that this is the official way of handling Users going 
> forward.
>
> Is there any roadmap on when it might hit trunk? I didn't see any 
> reference to User models in the 1.5 release notes.
> Cheers,
> Victor
>
> On Wednesday, 6 June 2012 21:34:42 UTC+10, Andrew Ingram wrote:
>>
>> On 4 June 2012 16:12, Russell Keith-Magee  
>> wrote: 
>> >  * The swapping mechanic is set up using a new Meta option on models 
>> > called 'swappable' that defines the setting where an alternate model 
>> > can be defined. Technically, the swappable option *could* be used for 
>> > any model; however, I'm not proposing that we actually document this. 
>> > I've implemented it as a generic approach purely to make the internals 
>> > cleaner -- mostly to avoid needing to make references to auth.User in 
>> > the model syncdb code, and to make testing possible (i.e., we can do 
>> > validation tests on a dummy 'swappable' model, rather than trying to 
>> > munge a validation test for auth.User specifically). 
>>
>> I like the idea of a 'swappable' option, but I can see some potential 
>> issues with the implementation. I'm one of the developers of Oscar 
>> (https://github.com/tangentlabs/django-oscar/) and providing a clean 
>> method to for overriding models has been a major pain point. One of 
>> our key requirements is that any model in Oscar may be overridden, to 
>> that end every model has both abstract and concrete versions (much 
>> like your implementation of AbstractBaseUser and User). 
>>
>> Our current way of handling overrides is for every reference to a 
>> model in Oscar to use get_model rather than the explicit classname. 
>> But this does end up causing some ugly things like this: 
>>
>> from oscar.apps.catalogue.abstract_models import AbstractProduct 
>>
>> class Product(AbstractProduct): 
>> # stuff 
>>
>> from oscar.apps.catalogue.models import * 
>>
>>
>> Issues: 
>> 1) The override model HAS to have the same name as the default model, 
>> otherwise get_model('catalogue', 'Product') won't work. 
>> 2) We have to import all the remaining default models after we declare 
>> our overrides. But this only works because Django's model metaclass 
>> prevents the default Product replacing the one we've just defined. I 
>> don't like this because it's a little bit magic. 
>> 3) You have to remove Oscar's version of the app from INSTALLED_APPS 
>> and replace it with your own. Actually, technically you don't. If you 
>> leave Oscar's app installed but put your own one ('foo.catalogue') in 
>> front of it, you can even get rid of the nasty import * thing - but 
>> again, more magic. (Aside: you can actually use this approach already 
>> to override Django's User model, because the metaclass means any 
>> references to Django's User will be replaced with references to your 
>> own. ) 
>>
>> I had investigated using a class decorator to handle overrides: 
>>
>> @replace_model('catalogue.Product') 
>> class MyProduct(AbstractProduct): 
>> # foo 
>>
>> But this got seriously complicated because the metaclass modifies the 
>> class before the decorator can be applied, so I was looking into ways 
>> to sever all the ties with the app cache before I went insane and gave 
>> up. 
>>
>> Back to my main points... Your swappable option would solve quite a 
>> lot, in the case of the User model it's ideal. But I'm concerned that 
>> if people start using it more widely we'd end up having dozens of new 
>> settings that would get quite silly in the case of a large project 
>> like Oscar. I think it's important that overrides can be defined in 
>> the settings file, but I'd also like to see it possible at model 
>> definition time, either using a decorator (like above) or a Meta 
>> option like 'replaces'. The risk, of course, is that it means any 
>> third-party app could override any other model without you necessarily 
>> being aware of it, not sure how this would be mitigated. 
>>
>> If I've not made much sense let me know, I've always found it hard to 
>> articulate on this particular topic. 
>>
>>
>> Regards, 
>> Andrew Ingram 
>>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To view this discussion on the web visit 
https://groups.google.com/d/msg/django-developers/-/VkctqktqAxMJ.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Draft branch: Swappable User models in contrib.auth

2012-08-18 Thread Victor Hooi
Hi,

I'm just wondering, has there been any updates on the User model refactor?

My understanding is that this is the official way of handling Users going 
forward.

Is there any roadmap on when it might hit trunk? I didn't see any reference 
to User models in the 1.5 release notes.
Cheers,
Victor

On Wednesday, 6 June 2012 21:34:42 UTC+10, Andrew Ingram wrote:
>
> On 4 June 2012 16:12, Russell Keith-Magee 
>  
> wrote: 
> >  * The swapping mechanic is set up using a new Meta option on models 
> > called 'swappable' that defines the setting where an alternate model 
> > can be defined. Technically, the swappable option *could* be used for 
> > any model; however, I'm not proposing that we actually document this. 
> > I've implemented it as a generic approach purely to make the internals 
> > cleaner -- mostly to avoid needing to make references to auth.User in 
> > the model syncdb code, and to make testing possible (i.e., we can do 
> > validation tests on a dummy 'swappable' model, rather than trying to 
> > munge a validation test for auth.User specifically). 
>
> I like the idea of a 'swappable' option, but I can see some potential 
> issues with the implementation. I'm one of the developers of Oscar 
> (https://github.com/tangentlabs/django-oscar/) and providing a clean 
> method to for overriding models has been a major pain point. One of 
> our key requirements is that any model in Oscar may be overridden, to 
> that end every model has both abstract and concrete versions (much 
> like your implementation of AbstractBaseUser and User). 
>
> Our current way of handling overrides is for every reference to a 
> model in Oscar to use get_model rather than the explicit classname. 
> But this does end up causing some ugly things like this: 
>
> from oscar.apps.catalogue.abstract_models import AbstractProduct 
>
> class Product(AbstractProduct): 
> # stuff 
>
> from oscar.apps.catalogue.models import * 
>
>
> Issues: 
> 1) The override model HAS to have the same name as the default model, 
> otherwise get_model('catalogue', 'Product') won't work. 
> 2) We have to import all the remaining default models after we declare 
> our overrides. But this only works because Django's model metaclass 
> prevents the default Product replacing the one we've just defined. I 
> don't like this because it's a little bit magic. 
> 3) You have to remove Oscar's version of the app from INSTALLED_APPS 
> and replace it with your own. Actually, technically you don't. If you 
> leave Oscar's app installed but put your own one ('foo.catalogue') in 
> front of it, you can even get rid of the nasty import * thing - but 
> again, more magic. (Aside: you can actually use this approach already 
> to override Django's User model, because the metaclass means any 
> references to Django's User will be replaced with references to your 
> own. ) 
>
> I had investigated using a class decorator to handle overrides: 
>
> @replace_model('catalogue.Product') 
> class MyProduct(AbstractProduct): 
> # foo 
>
> But this got seriously complicated because the metaclass modifies the 
> class before the decorator can be applied, so I was looking into ways 
> to sever all the ties with the app cache before I went insane and gave 
> up. 
>
> Back to my main points... Your swappable option would solve quite a 
> lot, in the case of the User model it's ideal. But I'm concerned that 
> if people start using it more widely we'd end up having dozens of new 
> settings that would get quite silly in the case of a large project 
> like Oscar. I think it's important that overrides can be defined in 
> the settings file, but I'd also like to see it possible at model 
> definition time, either using a decorator (like above) or a Meta 
> option like 'replaces'. The risk, of course, is that it means any 
> third-party app could override any other model without you necessarily 
> being aware of it, not sure how this would be mitigated. 
>
> If I've not made much sense let me know, I've always found it hard to 
> articulate on this particular topic. 
>
>
> Regards, 
> Andrew Ingram 
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To view this discussion on the web visit 
https://groups.google.com/d/msg/django-developers/-/AKe1T5bePTkJ.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Draft branch: Swappable User models in contrib.auth

2012-06-10 Thread Anssi Kääriäinen
On 10 kesä, 10:42, Russell Keith-Magee 
wrote:
> I'm not sure I see why. This looks to me like *exactly* what apps are
> designed to achieve. By way of implementation, it might make sense to
> introduce this as a 'sub app' - i.e., contrib.auth.improveduser would
> be an installable app in itself; this would avoid the need to dirty up
> the contrib namespace. However, for me that's a relatively minor
> implementation detail.
>
> I don't understand the objection to using app installation as the
> basis for declaring what features you want a particular project to
> have. Can you elaborate on why you think this isn't a reasonable
> solution to the problem?

You have dual-control for the same thing: you need to change both a
setting and the installed apps to pick improved user as your user
model. So, you can accidentally end up with the improved model
installed, but still have the original model as the settings model.
There can't be any validation errors in this case. Nothing ties the
ImprovedUser to the User model on code level. Additionally, the
requirement of separate apps places restrictions on where you must
place your code.

But, I have bugged you enough already. I don't fully agree on all of
the details of the API. Luckily there is no need for full agreement. I
don't see others complaining about the API, so maybe the API is
actually good and it's just me... :)

> > I was driving for the use case where you define multiple different
> > models for the same "slot" using the swappable option. If there can be
> > only one model with swappable set for each slot, then there is no
> > problem here.
>
> To me, that seems like solving an entirely different problem. It
> *might* be a problem that could be solved with app-refactor -- the
> configuration of an app could potentially specify all sorts of
> configuration points, such as "Model X has a foreign key to
> placeholder A; what model will A be?". That's an orthogonal problem to
> replacing all references to User with a custom model across an entire
> project.

Misunderstanding here, I didn't explain myself well above. The idea
was that in a single models.py file you could have multiple choices
for the same slot:

class User(models.Model):
...
class Meta:
  swappable = 'AUTH_USER_MODEL'

class ImprovedUser(models.Model):
...
class Meta:
swappable = 'AUTH_USER_MODEL'

Thus, two models for the same swappable slot. You can pick project
wide which one to use by AUTH_USER_MODEL = 'auth.User' or
AUTH_USER_MODEL = 'auth.ImprovedUser'. You can pick only one in your
project. And the original problem was that if you don't control the
settings, both models think they are the chosen one if no setting is
defined for the AUTH_USER_MODEL. If you can have only one model with
swappable set, then there is no problem.

> I'm not sure I understand what you're saying here. We can specify that
> the model class itself needs to provide certain methods (e.g.,
> User.get_full_name()), and we can specify that the Manager must
> provide certain methods (e.g., User.objects.create_superuser()). If we
> really needed to, we could also specify that the queryset
> implementation for the model had to provide certain methods, or that
> the model class had to provide certain class methods.
>
> If you're saying that we have no way to require that certain queries
> work or are parameterized, then I agree -- but I also think that this
> is a good thing. If you're building an app that is going to utilise
> pluggable models, then it's up to you to abstract away the
> implementation behind an interface that provides whatever utility you
> require. If you're not in a position to define an interface like that,
> then I'd argue that you don't really have a model that is a candidate
> for being swapped out.
>
> For the purposes of this work, the only model we actually care about
> is User (since swappability as a general feature isn't on the table
> here), and we can definitely define an interface for User.

What I was driving for was a way to do
User.objects.order_by('shortname') and have the ORM basically rewrite
this to User.objects.order_by(User.short_name_order_field()). Two
reasons: the lookup interface is really nice, and it is easily usable
across relations. Thus, the interface is not on queryset method level
or model level - it is on lookup level.

This doesn't actually seem to be that big of a problem: if there are
queryset interfaces you could always do
Document.objects.filter(creator__in=User.objects.fullname_search('Anssi')).order_by('creator__'
+ User.short_name_order_field())

That should be good enough.

 - Anssi

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 

Re: Draft branch: Swappable User models in contrib.auth

2012-06-10 Thread Russell Keith-Magee
On Fri, Jun 8, 2012 at 7:50 PM, Anssi Kääriäinen
 wrote:
> On 8 kesä, 13:43, Russell Keith-Magee  wrote:
>> That's certainly an interesting use case. However, I can think of at
>> least 2 ways it could be mitigated.
>>
>> One way would be to treat this as part of the contract of a pluggble
>> app. We've always said that an app should do one thing, and one thing
>> only; providing a *single* swappable model candidate, and any
>> associated support (e.g., forms, admin declarations) would seem to fit
>> that definition fairly well. In this context, a "library" of reusable
>> User models is really just a collection of apps, one of which will be
>> installed in your project. The CMS with a range of User model options
>> would provide them as a series of apps, one (and only one) of which
>> needs to be installed.
>
> We might want to have two user models already in the auth.models. The
> current User, and then an "ImprovedUser", which is like the default
> user, but has the worst limitations lifted (email field can be 255
> chars long, username restrictions lifted, ...). It seems the current
> implementation would not allow for this, because only one model could
> define itself as swappable, and the other would get installed even if
> it wasn't needed. For our use case this would work as we control
> global_settings.py, and we can use the hidden swappable attribute for
> both models, but I guess the ImprovedModel/BackwardsCompatibilityModel
> pattern isn't such a rare case. To make the swappable option nicer to
> use for 3rd party apps a better solution than different app for each
> swappable model would be welcome.

I'm not sure I see why. This looks to me like *exactly* what apps are
designed to achieve. By way of implementation, it might make sense to
introduce this as a 'sub app' - i.e., contrib.auth.improveduser would
be an installable app in itself; this would avoid the need to dirty up
the contrib namespace. However, for me that's a relatively minor
implementation detail.

I don't understand the objection to using app installation as the
basis for declaring what features you want a particular project to
have. Can you elaborate on why you think this isn't a reasonable
solution to the problem?

>> > The model_label_for_default_model can't be defined anywhere in the
>> > current implementation. The Model._meta.swapped uses:
>> >    return self.swappable and getattr(settings, self.swappable, None)
>> > not in (None, model_label)
>>
>> Sure - but model_label_of_default_model doesn't *need* to be defined
>> anywhere - it's the model that has swappable=XXX defined on it.
>
> I was driving for the use case where you define multiple different
> models for the same "slot" using the swappable option. If there can be
> only one model with swappable set for each slot, then there is no
> problem here.

To me, that seems like solving an entirely different problem. It
*might* be a problem that could be solved with app-refactor -- the
configuration of an app could potentially specify all sorts of
configuration points, such as "Model X has a foreign key to
placeholder A; what model will A be?". That's an orthogonal problem to
replacing all references to User with a custom model across an entire
project.

>> Again - for me this is one of those things where it's all about the 
>> interface.
>>
>> In this example, your end problem isn't "I need to issue a query with
>> Q(creator__last_name__ilike=q)|Q(creator__first_name__ilike=q)" --
>> it's "I need to search for document creator by name", and  *that* is
>> what your interface should be defining. If your interface spec said
>> you had to provide a "find_document_by_author()" method, and one
>> particular implementation happens to use that query, then you've just
>> solved the problem. The specific document attributes *should* be
>> irrelevant -- and if they aren't, you haven't defined your document
>> interface well enough.
>>
>> It's impossible to claim that every model will be usable in every
>> circumstance. At some point, you have to lock down an interface
>> specification, which will define what it is possible to do in a
>> 'generic' sense with your swappable model. This isn't some new concept
>> we're inventing here - interfaces have been a major part of more than
>> one language for decades. Luckily, with Python, we get the benefits of
>> interfaces without needing to be strict about it -- we can rely on
>> duck typing, rather than rigorous and inflexible compile-time checks.
>
> The problem is that we have no ability to define ORM interface for the
> model. The ORM doesn't allow for this. It would be nice, but you can't
> have everything. I am not claiming this is a blocker issue at all, I
> was just wondering if there were any plans to define an ORM interface
> for the model.

I'm not sure I understand what you're saying here. We can specify that
the model class itself needs to provide certain methods (e.g.,

Re: Draft branch: Swappable User models in contrib.auth

2012-06-08 Thread Russell Keith-Magee
On Fri, Jun 8, 2012 at 8:53 AM, Anssi Kääriäinen
 wrote:
> On 8 kesä, 02:43, Russell Keith-Magee  wrote:
>> >  - For documentation: It should be suggested that the example MyUser
>> > should define class Meta: swappable = 'AUTH_USER_MODEL'. Otherwise it
>> > will be synced to the database even if it is not the chosen user model
>> > for the project, which is likely not wanted. Maybe AbstractBaseUser
>> > should define swappable = 'AUTH_USER_MODEL' so that anybody who
>> > inherits from that gets the setting automatically?
>>
>> Ah - I think I see the problem now. The swappable Meta option isn't
>> visible to to end user -- ever -- unless they go spelunking in the
>> source code for auth.models.
>>
>> If I define MyUser, I *dont'* need to define it as swappable. I just
>> need to define AUTH_USER_MODEL in my settings file to point at my
>> replacement model definition. The only place where swappable is
>> defined is on the base model that is being swapped out -- in this
>> case, auth.User.
>>
>> I'm expecting that there will be *no* documentation of the swappable
>> option (unless, at some point in the future, we decide to make
>> swappable models an official feature). It exists purely to make the
>> internals clean, and avoid making auth.User a special case in code.
>
> The real problem comes when you have multiple swappable models defined
> for the same swappable "slot". This could happen in a CMS for example
> - they might define a couple of classes to use, you can choose which
> one to use by using a setting. Or, maybe you have some library
> installed which defines some reusable User models. Problem is, every
> one of these models will be installed unless they provided the
> swappable = 'AUTH_USER_MODEL' meta option. If you don't provide that
> option, nothing ties the models to the same slot.

That's certainly an interesting use case. However, I can think of at
least 2 ways it could be mitigated.

One way would be to treat this as part of the contract of a pluggble
app. We've always said that an app should do one thing, and one thing
only; providing a *single* swappable model candidate, and any
associated support (e.g., forms, admin declarations) would seem to fit
that definition fairly well. In this context, a "library" of reusable
User models is really just a collection of apps, one of which will be
installed in your project. The CMS with a range of User model options
would provide them as a series of apps, one (and only one) of which
needs to be installed.

Alternatively, I suppose we could introduce a separate Meta option
(lets call it "swapper" for the moment -- I don't particularly like
the name, but it will do for discussion purposes) which has the
opposite behaviour of swappable - i.e., a model with swappable=XXX is
only synchronised if settings.XXX is undefined, or XXX points to the
default model; a model with swapper=True is only synchronised if XXX
points to it. Otherwise, it's treated as an abstract model.

Personally, my preference would be the former approach.

>> >  - Assume a 3rd party developer was going to use the swappable Meta
>> > attribute. The developer provides a couple of different swappable
>> > implementations to use, each defining swappable = 'MY_SETTING'.
>> > However, by default settings.MY_SETTING is None, and thus each of
>> > these models will be synced into the DB. How does the 3rd party
>> > developer designate the default implementation? For auth.user this is
>> > easy, as global_settings can contain the default setting. A 3rd party
>> > developer doesn't have this option. (This one isn't important for the
>> > auth.User case, only for making this feature someday publicly
>> > available).
>>
>> If MY_SETTING isn't defined, all the code I've written should be
>> assuming that the model isn't swapped out (i.e., it's effectively
>> getattr(settings, 'MY_SETTING', model_label_of_default_model) ). As
>> proof of concept, it certainly wouldn't hurt to drop out the
>> AUTH_USER_MODEL setting from globals.
>
> The model_label_for_default_model can't be defined anywhere in the
> current implementation. The Model._meta.swapped uses:
>    return self.swappable and getattr(settings, self.swappable, None)
> not in (None, model_label)

Sure - but model_label_of_default_model doesn't *need* to be defined
anywhere - it's the model that has swappable=XXX defined on it.

>> >> > About the ORM capabilities of the interface: It seems there can not be
>> >> > any assumptions about the swapped in model here: for example
>> >> > SomeModel.filter(user__is_staff=True) or
>> >> > SomeModel.order_by('user__lastname', 'user__firstname') works
>> >> > currently, but there are no such lookups available for the interface.
>> >> > Maybe there should be some way to order by "full_name" and
>> >> > "short_name" at least. But, this gets us quickly to virtual fields or
>> >> > virtual lookups territory...
>>
>> >> Imo it's fine if we require some 

Re: Draft branch: Swappable User models in contrib.auth

2012-06-07 Thread Russell Keith-Magee
On Thu, Jun 7, 2012 at 7:48 PM, Anssi Kääriäinen
 wrote:
> On Jun 7, 11:57 am, Florian Apolloner  wrote:
>> Hi,
>>
>> On Wednesday, June 6, 2012 4:32:02 PM UTC+2, Anssi Kääriäinen wrote:
>>
>> > Still, yet another API idea: [snip]
>>
>> Then, Model.__new__ will replace the SwappableUser class with the
>>
>> > swapped in class. The 'swappable' in model.Meta tells which concrete
>> > model implementation to use.
>>
>> I'd rather not dynamically replace models. The system proposed by Russell
>> is clean and requires no extra magic. The only thing which might look a bit
>> odd is inheriting from a swapped model, eg like:
>>
>> class MyUserProxy(get_user_model()):
>>   class Meta:
>>     proxy = True
>>
>> etc… (Which obviously can get a bit nicer by using User = get_user_model()
>> and then inheriting from that).
>
> I just don't see it as a good idea to hard-code meta.swappable to be a
> settings variable in user-visible way, without any option to use
> anything else. I see I am in minority here, so I will let this issue
> be.
>
> Review issues spotted:
>  - Queries using a swapped-out model should be prevented (probably at
> "execute sql" time).

Good idea. I haven't tried to see what it actually does; I'm guessing
you're going to get Table Does Not Exist errors. A nicer error
wouldn't hurt.

>  - For documentation: It should be suggested that the example MyUser
> should define class Meta: swappable = 'AUTH_USER_MODEL'. Otherwise it
> will be synced to the database even if it is not the chosen user model
> for the project, which is likely not wanted. Maybe AbstractBaseUser
> should define swappable = 'AUTH_USER_MODEL' so that anybody who
> inherits from that gets the setting automatically?

Ah - I think I see the problem now. The swappable Meta option isn't
visible to to end user -- ever -- unless they go spelunking in the
source code for auth.models.

If I define MyUser, I *dont'* need to define it as swappable. I just
need to define AUTH_USER_MODEL in my settings file to point at my
replacement model definition. The only place where swappable is
defined is on the base model that is being swapped out -- in this
case, auth.User.

I'm expecting that there will be *no* documentation of the swappable
option (unless, at some point in the future, we decide to make
swappable models an official feature). It exists purely to make the
internals clean, and avoid making auth.User a special case in code.

>  - Creating a "class UserDerived(User)" will not succeed if the User
> is swapped out. So, you can't make a swappable derived class.
> Documenting this limitation is IMO enough.

Documentation would be one option; another would be to treat a
reference to a swapped out base class as an abstract class. The
handling is identical in every other sense (i.e., an unsynchronized
model). Better error messages would be another option; I'm having
difficulty thinking of a case where you'd want to have a subclass of a
model that isn't being used in the rest of your system.

>  - Create model UserProxy(auth.User), swap out auth.User, and make a
> foreign key to UserProxy - you get an error that "DatabaseError:
> relation "auth_user" does not exist" instead of a mention that
> UserProxy is swapped out. The error should probably say something like
> "UserProxy derives from swapped out model auth.User - can't reference
> this model.", as telling to point to settings.AUTH_USER_MODEL in this
> case is incorrect.

Agreed -- error messages would be better.

>  - The abstract user class defines two fields: password, and
> last_login. Is the last_login really a core requirement?

last_login is at least arguable as an inclusion in the abstract base
model. We certainly *could* drop it from the list of requirements. The
reason to include it is so that all the 'user token' code works -- so
all the password reset views et al work as expected.

An alternative option would be to require that the user model provide
an implementation of a "login_timestamp" attribute (just like the
get_full_name and get_short_name requirements).

The last option -- make last_login completely optional, but document
the fact that the password reset views require the existence of the
last_login attribute.

>  - Assume a 3rd party developer was going to use the swappable Meta
> attribute. The developer provides a couple of different swappable
> implementations to use, each defining swappable = 'MY_SETTING'.
> However, by default settings.MY_SETTING is None, and thus each of
> these models will be synced into the DB. How does the 3rd party
> developer designate the default implementation? For auth.user this is
> easy, as global_settings can contain the default setting. A 3rd party
> developer doesn't have this option. (This one isn't important for the
> auth.User case, only for making this feature someday publicly
> available).

If MY_SETTING isn't defined, all the code I've written should be
assuming that the model isn't 

Re: Draft branch: Swappable User models in contrib.auth

2012-06-07 Thread Anssi Kääriäinen
On Jun 7, 11:57 am, Florian Apolloner  wrote:
> Hi,
>
> On Wednesday, June 6, 2012 4:32:02 PM UTC+2, Anssi Kääriäinen wrote:
>
> > Still, yet another API idea: [snip]
>
> Then, Model.__new__ will replace the SwappableUser class with the
>
> > swapped in class. The 'swappable' in model.Meta tells which concrete
> > model implementation to use.
>
> I'd rather not dynamically replace models. The system proposed by Russell
> is clean and requires no extra magic. The only thing which might look a bit
> odd is inheriting from a swapped model, eg like:
>
> class MyUserProxy(get_user_model()):
>   class Meta:
>     proxy = True
>
> etc… (Which obviously can get a bit nicer by using User = get_user_model()
> and then inheriting from that).

I just don't see it as a good idea to hard-code meta.swappable to be a
settings variable in user-visible way, without any option to use
anything else. I see I am in minority here, so I will let this issue
be.

Review issues spotted:
  - Queries using a swapped-out model should be prevented (probably at
"execute sql" time).

  - For documentation: It should be suggested that the example MyUser
should define class Meta: swappable = 'AUTH_USER_MODEL'. Otherwise it
will be synced to the database even if it is not the chosen user model
for the project, which is likely not wanted. Maybe AbstractBaseUser
should define swappable = 'AUTH_USER_MODEL' so that anybody who
inherits from that gets the setting automatically?

  - Creating a "class UserDerived(User)" will not succeed if the User
is swapped out. So, you can't make a swappable derived class.
Documenting this limitation is IMO enough.

  - Create model UserProxy(auth.User), swap out auth.User, and make a
foreign key to UserProxy - you get an error that "DatabaseError:
relation "auth_user" does not exist" instead of a mention that
UserProxy is swapped out. The error should probably say something like
"UserProxy derives from swapped out model auth.User - can't reference
this model.", as telling to point to settings.AUTH_USER_MODEL in this
case is incorrect.

  - The abstract user class defines two fields: password, and
last_login. Is the last_login really a core requirement?

  - Assume a 3rd party developer was going to use the swappable Meta
attribute. The developer provides a couple of different swappable
implementations to use, each defining swappable = 'MY_SETTING'.
However, by default settings.MY_SETTING is None, and thus each of
these models will be synced into the DB. How does the 3rd party
developer designate the default implementation? For auth.user this is
easy, as global_settings can contain the default setting. A 3rd party
developer doesn't have this option. (This one isn't important for the
auth.User case, only for making this feature someday publicly
available).

> > About the ORM capabilities of the interface: It seems there can not be
> > any assumptions about the swapped in model here: for example
> > SomeModel.filter(user__is_staff=True) or
> > SomeModel.order_by('user__lastname', 'user__firstname') works
> > currently, but there are no such lookups available for the interface.
> > Maybe there should be some way to order by "full_name" and
> > "short_name" at least. But, this gets us quickly to virtual fields or
> > virtual lookups territory...
>
> Imo it's fine if we require some fields on a Usermodel to be compatible
> with admin and friends. (eg a first/last_name field which is nullable [or
> not required in the form validation sense] shouldn't hurt anyone, the admin
> could check if the field is filled and if not display something else like
> the username).

Admin is actually somewhat easy to make work re the ordering, because
in there you can use get_full_name.admin_order_field =
'somemodelfield'. This doesn't work outside Admin. This isn't a major
issue, it prevents things like ordering comments on the commenter's
"get_short_name" output. Even then, this is only an issue for reusable
code - in your project you know the concrete user model you have in
use, and can thus use the specific lookups your model has.

 - Anssi

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Draft branch: Swappable User models in contrib.auth

2012-06-07 Thread Florian Apolloner
Hi,

On Wednesday, June 6, 2012 4:32:02 PM UTC+2, Anssi Kääriäinen wrote:
>
> Still, yet another API idea: [snip]
>   
>
Then, Model.__new__ will replace the SwappableUser class with the 
> swapped in class. The 'swappable' in model.Meta tells which concrete 
> model implementation to use.
>

I'd rather not dynamically replace models. The system proposed by Russell 
is clean and requires no extra magic. The only thing which might look a bit 
odd is inheriting from a swapped model, eg like:

class MyUserProxy(get_user_model()):
  class Meta:
proxy = True

etc… (Which obviously can get a bit nicer by using User = get_user_model() 
and then inheriting from that).
 
>
> About the ORM capabilities of the interface: It seems there can not be 
> any assumptions about the swapped in model here: for example 
> SomeModel.filter(user__is_staff=True) or 
> SomeModel.order_by('user__lastname', 'user__firstname') works 
> currently, but there are no such lookups available for the interface. 
> Maybe there should be some way to order by "full_name" and 
> "short_name" at least. But, this gets us quickly to virtual fields or 
> virtual lookups territory... 
>

Imo it's fine if we require some fields on a Usermodel to be compatible 
with admin and friends. (eg a first/last_name field which is nullable [or 
not required in the form validation sense] shouldn't hurt anyone, the admin 
could check if the field is filled and if not display something else like 
the username).

Cheers,
Florian

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To view this discussion on the web visit 
https://groups.google.com/d/msg/django-developers/-/3yOTO--OzlkJ.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Draft branch: Swappable User models in contrib.auth

2012-06-06 Thread Andrew Ingram
On 4 June 2012 16:12, Russell Keith-Magee  wrote:
>  * The swapping mechanic is set up using a new Meta option on models
> called 'swappable' that defines the setting where an alternate model
> can be defined. Technically, the swappable option *could* be used for
> any model; however, I'm not proposing that we actually document this.
> I've implemented it as a generic approach purely to make the internals
> cleaner -- mostly to avoid needing to make references to auth.User in
> the model syncdb code, and to make testing possible (i.e., we can do
> validation tests on a dummy 'swappable' model, rather than trying to
> munge a validation test for auth.User specifically).

I like the idea of a 'swappable' option, but I can see some potential
issues with the implementation. I'm one of the developers of Oscar
(https://github.com/tangentlabs/django-oscar/) and providing a clean
method to for overriding models has been a major pain point. One of
our key requirements is that any model in Oscar may be overridden, to
that end every model has both abstract and concrete versions (much
like your implementation of AbstractBaseUser and User).

Our current way of handling overrides is for every reference to a
model in Oscar to use get_model rather than the explicit classname.
But this does end up causing some ugly things like this:

from oscar.apps.catalogue.abstract_models import AbstractProduct

class Product(AbstractProduct):
# stuff

from oscar.apps.catalogue.models import *


Issues:
1) The override model HAS to have the same name as the default model,
otherwise get_model('catalogue', 'Product') won't work.
2) We have to import all the remaining default models after we declare
our overrides. But this only works because Django's model metaclass
prevents the default Product replacing the one we've just defined. I
don't like this because it's a little bit magic.
3) You have to remove Oscar's version of the app from INSTALLED_APPS
and replace it with your own. Actually, technically you don't. If you
leave Oscar's app installed but put your own one ('foo.catalogue') in
front of it, you can even get rid of the nasty import * thing - but
again, more magic. (Aside: you can actually use this approach already
to override Django's User model, because the metaclass means any
references to Django's User will be replaced with references to your
own. )

I had investigated using a class decorator to handle overrides:

@replace_model('catalogue.Product')
class MyProduct(AbstractProduct):
# foo

But this got seriously complicated because the metaclass modifies the
class before the decorator can be applied, so I was looking into ways
to sever all the ties with the app cache before I went insane and gave
up.

Back to my main points... Your swappable option would solve quite a
lot, in the case of the User model it's ideal. But I'm concerned that
if people start using it more widely we'd end up having dozens of new
settings that would get quite silly in the case of a large project
like Oscar. I think it's important that overrides can be defined in
the settings file, but I'd also like to see it possible at model
definition time, either using a decorator (like above) or a Meta
option like 'replaces'. The risk, of course, is that it means any
third-party app could override any other model without you necessarily
being aware of it, not sure how this would be mitigated.

If I've not made much sense let me know, I've always found it hard to
articulate on this particular topic.


Regards,
Andrew Ingram

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Draft branch: Swappable User models in contrib.auth

2012-06-06 Thread Russell Keith-Magee
On Tue, Jun 5, 2012 at 9:00 PM, Anssi Kääriäinen
 wrote:
> Understood & agreed (the "this model is dynamic made explicit" part
> seems really important specifically).
>
> I am afraid of the hard-coding of meta.swappable must be 'SOME_VAR'
> which then references settings.SOME_VAR, and that this is made part of
> the public API by the error messages. I would imagine that after app-
> loading refactor one would make the swappable an app-level constant,
> but the use of settings.AUTH_USER_MODEL directly in the code makes
> this change backwards incompatible. Any chance of making what the
> "swappable" contains more abstract? Something like suggesting making
> foreign keys point to auth_user_ref() instead of
> settings.AUTH_USER_MODEL.
>
> Another idea would be to change the "swappable" to contain a method
> reference, and then hint in the foreign key error message that "point
> to User._meta.swappable() instead". (Or, make that Model property, so
> that you could say to point to User.swapped_by instead, and
> User.swapped_by would contain the real reference by checking where
> User._meta.swappable points to).

I can see the problem you're driving at, but I'm not sure it will
actually be a problem in practice.

Of course, it's impossible to know for sure without knowing the final
App-refactor API, but based on the most recent discussions, the idea
is to define a class that defines the App. I imagine that swappable
models would be a part of this definition -- something like:

auth_app = App(
   …
   swappable_models = {
  'User': 'myauth.MyUser'
   })

The exact syntax may be different, but the point should be clear --
the App provides the point where the substituted User model can be
configured, replacing the need for an AUTH_USER_MODEL setting.

If this is the case, the requirement for "Meta: swappable" reduces
from needing to name a specific setting, to just providing a boolean
that defines the fact that the App is allowed to accept overrides.
Conveniently, 'AUTH_USER_MODEL' (or any other string value) evaluates
as True, so AUTH_USER_MODEL will be a slightly non-sensical value, but
a perfectly legal on. It also means that the app can exist in both
worlds (a settings-overridden model, and an App-defintion overridden
model. There's some prioritisation logic required there, but thats a
relatively minor detail). Long term, the syntax can reduce to 'class
Meta: swappable=True'.

> As for the User model as an ORM object - what lookups can be assumed
> to exist? It seems the only requirement is that it has a numeric
> primary key called 'id', otherwise there isn't anything that can be
> assumed of it?

The short answer is that there aren't any *hard* requirements. The
only time requirements come into the picture is if you want to
minimise the amount of additional code you want to write.

If you want to integrate with a minimum of effort, the only
requirement that exists is that your model must provide a unique
string-based identifier field (e.g. username), a password field, and a
last login timestamp. The latter two you get for free from the
AbstractBaseUser class; the unique identifier field must be provided
as part of your model definition. It doesn't matter what the unique
identifier field is called -- it just needs to exist, and if it's not
called username, you need to define USERNAME_FIELD as a class
attribute. Strictly, the login timestamp isn't required, but it's used
for password reset tokens, so I figured it makes sense to include it
by default.

I've done some further checking, and the primary key restriction I
wrote about previously doesn't actually exist. As long as your primary
key can be stored as a string in the session, the default auth backend
will work.

As your model gets less like the out-of-the-box auth.User model, your
workload increases. Depending on your exact User model, you may need
to define forms to create/update a user, and define a ModelAdmin for
your custom User model. I still need to look into how much of these
forms can be made even easier to reuse.

If your User model requires something more than 1 identifier field
(e.g., username and domain), you're still ok -- you just need to
define your own Authentication forms and auth backend.

So - in summary, it really should be a case of "Any User model will
do"; it's just a matter of how much extra work needs to be done.

However, if your requirement is to work with Admin, there are a couple
more requirements. I *think* the minimum requirement is a definition
for the following:

 * get_full_name
 * get_short_name
 * __unicode__
 * has_perm
 * has_module_perms
 * is_staff
 * is_active

However, I need to do a full audit for this list to make sure I
haven't missed anything. The permissions in particular are
interesting, because they don't need to be "auth.Permission" objects
-- you just need to be able to answer True/False for a string name.
This means you can handle permissions using Django's approach if you

Re: Draft branch: Swappable User models in contrib.auth

2012-06-05 Thread Anssi Kääriäinen
On Jun 5, 2:44 pm, Russell Keith-Magee 
wrote:
> Two significant reasons:
>
> Firstly, if we use the Meta attribute approach, we can raise a
> specific validation error when the user has an app with a model that
> references auth.User directly, and User has been swapped out. This is
> mostly useful as a migration trigger -- as soon as you change
> AUTH_USER_MODEL, you'll get a bunch of errors about all the apps that
> haven't been updated and will break as a result. This catches as a
> development-time validation error the most-likely set of problems that
> will occur when swappable User models start getting into the wild.
> Using the code you've provided, changing AUTH_USER_MODEL would
> silently fail at some point in production, because auth.User still
> "exists", but is now a very different beast.
>
> Secondly, there's some avoidance of nightmares from Django's past.
> Using the approach you describe, you're making auth.User a dynamic
> model reference -- the model that is referred to by auth.User is
> changed at runtime, based on the value of a setting. This means that
> just because you have a auth.User object, doesn't mean you can know
> for sure anything about it.
>
> This is somewhat comparable to the bad old "magic removal" days of
> Django. A lot of the problems we experienced back then were caused by
> the fact that you couldn't be certain that a model existed, or if it
> did exist, that it was the model that you thought it was, because
> other mechanisms in the load sequence would swap out models/modules,
> and/or move them somewhere else. This is a set of headaches that I'd
> rather not revisit.
>
> There's a scaled back alternative of what you describe -- essentially,
> where you only have the "else" clause from your sample code. This is
> slightly better, but still leads to the situation where the failure
> mode is an import failing, rather than a helpful message that points
> at the model that you shouldn't be referencing.
>
> By using the swappable approach, a model reference is always exactly
> the model that you've imported, unless you've got the reference
> through a get_user_model() method call, and other validation
> mechanisms work out whether that model reference is legal. At the end
> of the day, these validation mechanisms aren't really that complex,
> either. If you look at the additions to the validation logic, they're
> essentially just "if f.rel.to.swapped, make noise".
>
> The only other complex part is the synchronisation code, but even that
> is just layering on top of code that's already there. Abstract, proxy
> and non-managed models all fall in the group of "importable, but not
> synchronisable" models; swappable just adds one more to that list.
>
> There are two other lesser reasons that I've taken this approach.
>
> Firstly, I'm not wild about module level if statements. For me, a
> module should be a container of logic, not an executable unit in
> itself. I appreciate that there are sometimes valid reasons to make
> module level code executable, and at some level a def statement is
> just the execution of a declaration statement, but to my mind it's
> worth keeping a clear distinction. A good practical example of why
> this matters is testing -- module level if statements can only be
> (easily) tested at time of import, so you really only get one chance
> to test that code.
>
> Secondly, I don't like special cases, and a swappable Meta attribute
> -- even if it's a stealth option -- means that the underlying
> mechanism is generic, and doesn't require any User-specific references
> anywhere. Again, testing is a good reason for why this matters. We can
> test that the mechanics of swappable models work as expected with a
> suite of non-User swapped/unswapped models. If the mechanism was User
> specific, we'd only be able to test using User, but the swappability
> would be determined as an import-time definition, so we'd only be able
> to test one configuration per test run (unless we got into some messy
> reload/app cache flushing).
>
> As a side effect, it also means that if we were to ever 'bless'
> swappable models as an official feature, we just need to document what
> we've already got. In the interim, we have a hidden feature that
> adventurous members of the community can start playing with. As an
> unofficial feature, we're not obligated to support or maintain it, but
> it gives us a way to test out the waters to see whether the idea
> should be encouraged as broader practice.
>
> This approach to API development has precedent in Django. For example,
> shadow reverse attributes (i.e, related_name="+foo") existed as an
> undocumented feature for some time before we decided that they weren't
> actually harmful and added them as official API.

Understood & agreed (the "this model is dynamic made explicit" part
seems really important specifically).

I am afraid of the hard-coding of meta.swappable must be 'SOME_VAR'
which then references 

Re: Draft branch: Swappable User models in contrib.auth

2012-06-05 Thread Russell Keith-Magee
On Tue, Jun 5, 2012 at 3:56 AM, Anssi Kääriäinen
 wrote:
> On Jun 4, 6:12 pm, Russell Keith-Magee 
> wrote:
>>  * The swapping mechanic is set up using a new Meta option on models
>> called 'swappable' that defines the setting where an alternate model
>> can be defined. Technically, the swappable option *could* be used for
>> any model; however, I'm not proposing that we actually document this.
>> I've implemented it as a generic approach purely to make the internals
>> cleaner -- mostly to avoid needing to make references to auth.User in
>> the model syncdb code, and to make testing possible (i.e., we can do
>> validation tests on a dummy 'swappable' model, rather than trying to
>> munge a validation test for auth.User specifically).
>
> The "swappable" meta attribute part of the patch raises a question:
> would the following pattern work instead of the meta attribute:
> # in auth/models.py
>
> if settings.AUTH_USER_MODEL:
>    User = get_model(AUTH_USER_MODEL)
> else:
>    class User(models.Model):
>        # the traditional auth-user model definition here
>
> If I understand the patch correctly the "swappable" Meta option
> basically just removes the model from the app-cache. This means the
> model will not be synced or validated. In addition you can't make
> foreign keys to the swapped out model due to other changes in the
> patch. Still, the auth.User model exists, and is importable.
>
> The idea above gets rid of the User model 100% if
> settings.AUTH_USER_MODEL is set. In runtime it never exists at any
> level. It is very simple in this regard, and of course the
> implementation is simple, too.
>
> Is there some design consideration which prevents the use of the above
> mentioned idea?

Two significant reasons:

Firstly, if we use the Meta attribute approach, we can raise a
specific validation error when the user has an app with a model that
references auth.User directly, and User has been swapped out. This is
mostly useful as a migration trigger -- as soon as you change
AUTH_USER_MODEL, you'll get a bunch of errors about all the apps that
haven't been updated and will break as a result. This catches as a
development-time validation error the most-likely set of problems that
will occur when swappable User models start getting into the wild.
Using the code you've provided, changing AUTH_USER_MODEL would
silently fail at some point in production, because auth.User still
"exists", but is now a very different beast.

Secondly, there's some avoidance of nightmares from Django's past.
Using the approach you describe, you're making auth.User a dynamic
model reference -- the model that is referred to by auth.User is
changed at runtime, based on the value of a setting. This means that
just because you have a auth.User object, doesn't mean you can know
for sure anything about it.

This is somewhat comparable to the bad old "magic removal" days of
Django. A lot of the problems we experienced back then were caused by
the fact that you couldn't be certain that a model existed, or if it
did exist, that it was the model that you thought it was, because
other mechanisms in the load sequence would swap out models/modules,
and/or move them somewhere else. This is a set of headaches that I'd
rather not revisit.

There's a scaled back alternative of what you describe -- essentially,
where you only have the "else" clause from your sample code. This is
slightly better, but still leads to the situation where the failure
mode is an import failing, rather than a helpful message that points
at the model that you shouldn't be referencing.

By using the swappable approach, a model reference is always exactly
the model that you've imported, unless you've got the reference
through a get_user_model() method call, and other validation
mechanisms work out whether that model reference is legal. At the end
of the day, these validation mechanisms aren't really that complex,
either. If you look at the additions to the validation logic, they're
essentially just "if f.rel.to.swapped, make noise".

The only other complex part is the synchronisation code, but even that
is just layering on top of code that's already there. Abstract, proxy
and non-managed models all fall in the group of "importable, but not
synchronisable" models; swappable just adds one more to that list.

There are two other lesser reasons that I've taken this approach.

Firstly, I'm not wild about module level if statements. For me, a
module should be a container of logic, not an executable unit in
itself. I appreciate that there are sometimes valid reasons to make
module level code executable, and at some level a def statement is
just the execution of a declaration statement, but to my mind it's
worth keeping a clear distinction. A good practical example of why
this matters is testing -- module level if statements can only be
(easily) tested at time of import, so you really only get one chance
to test that code.


Re: Draft branch: Swappable User models in contrib.auth

2012-06-04 Thread Anssi Kääriäinen
On Jun 4, 6:12 pm, Russell Keith-Magee 
wrote:
>  * The swapping mechanic is set up using a new Meta option on models
> called 'swappable' that defines the setting where an alternate model
> can be defined. Technically, the swappable option *could* be used for
> any model; however, I'm not proposing that we actually document this.
> I've implemented it as a generic approach purely to make the internals
> cleaner -- mostly to avoid needing to make references to auth.User in
> the model syncdb code, and to make testing possible (i.e., we can do
> validation tests on a dummy 'swappable' model, rather than trying to
> munge a validation test for auth.User specifically).

The "swappable" meta attribute part of the patch raises a question:
would the following pattern work instead of the meta attribute:
# in auth/models.py

if settings.AUTH_USER_MODEL:
User = get_model(AUTH_USER_MODEL)
else:
class User(models.Model):
# the traditional auth-user model definition here

If I understand the patch correctly the "swappable" Meta option
basically just removes the model from the app-cache. This means the
model will not be synced or validated. In addition you can't make
foreign keys to the swapped out model due to other changes in the
patch. Still, the auth.User model exists, and is importable.

The idea above gets rid of the User model 100% if
settings.AUTH_USER_MODEL is set. In runtime it never exists at any
level. It is very simple in this regard, and of course the
implementation is simple, too.

Is there some design consideration which prevents the use of the above
mentioned idea?

 - Anssi

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Draft branch: Swappable User models in contrib.auth

2012-06-04 Thread Russell Keith-Magee
Hi all,

Following the BDFL pronouncement of a preferred option for
customisable User models in contrib.auth [1], I've just pushed a
branch to Github that contains a draft implementation [2].

It's not ready for trunk quite yet, but you can use this code to set
up a custom User model, and then log into admin with that User model,
and the effort required to do this is fairly minimal.

There isn't a whole lot by way of documentation -- all I've done is
add the documentation for the new AUTH_USER_MODEL setting, and a stub
for where the documentation about swappable User models will finally
go.

Testing is also an interesting area. I've got some tests in place for
the validation code, and for parts of the management. Widespread
testing that apps (e.g., admin) will work with a different User model
will be a bit difficult because the app cache is populated at the
start of the test run, which prohibits switching the User model later
in the suite. Any suggestions on areas where more testing is necessary
and possible are welcome.

So - how does it work? Here's a minimal example in which MyUser has a
unique, required EmailField acting as the identifier for users.
There's also a required "date_of_birth" field for every user. To
switch to using this User model:

 * Define a new "myauth" app, and put the following into your models.py file

===
from django.db import models

from django.contrib.auth.models import BaseUserManager, AbstractBaseUser

class MyUserManager(BaseUserManager):
def create_user(self, email, date_of_birth, password=None):
if not email:
raise ValueError('Users must have an email address')

user = self.model(
email=MyUserManager.normalize_email(email),
date_of_birth=date_of_birth,
)

user.set_password(password)
user.save(using=self._db)
return user

def create_superuser(self, username, password, date_of_birth):
u = self.create_user(username, password=password,
date_of_birth=date_of_birth)
u.is_admin = True
u.save(using=self._db)
return u


class MyUser(AbstractBaseUser):
email = models.EmailField(verbose_name='email address',
max_length=255, unique=True)
date_of_birth = models.DateField()
is_admin = models.BooleanField(default=False)

objects = MyUserManager()

USERNAME_FIELD = 'email'
REQUIRED_FIELDS = ['date_of_birth']

def get_full_name(self):
return self.email

def get_short_name(self):
return self.email

def __unicode__(self):
return self.email

# Admin required fields
def has_perm(self, perm, obj=None):
return True

def has_module_perms(self, app_label):
return True

@property
def is_staff(self):
return self.is_admin

@property
def is_active(self):
return True
===

 * Set AUTH_USER_MODEL = 'myauth.MyUser'

 * Run syncdb.

It's important that you do all this on a clean project; once you've
run syncdb, the foreign keys are already set up, and changing
AUTH_USER_MODEL will lead to hilarity.

Some implementation notes:

 * As the branch currently stands, you can log into admin with a
custom User model by doing nothing more than defining an
'admin-compatible' User model and setting AUTH_USER_MODEL.

 * The fields on MyUser are the minimum subset that I've been able to
establish in order to be "admin-compatible". The implementation of
MyUser is intentionally simple -- every user is effectively a
superuser. If you want fine-grained permissions, you'll need to
implement them.

 * The createsuperuser and changepassword management commands both
work with the designated User model.

 * The custom manager is required to make sure that the
createsuperuser management command works. Strictly, only
create_superuser is required, but create_user is an obvious utility to
have, so I've included it here.

 * I've made get_full_name() and get_short_name() part of the required
API; this follows the most practical and applicable of the suggestions
from the W3C guidelines on personal names [3].

 * USERNAME_FIELD and REQUIRED_FIELDS are constants used for
metaprogramming. For example, createsuperuser needs to ask for data
for all the required fields; these constants let you define what
createsuperuser should ask for. They're also used whenever the auth
backend needs to retrieve the User based on credentials (the backend
can't say User.objects.get(username=…), because the field may not be
called username). I'm not completely happy about the way these
constants are defined, but I also can't think of a better option,
other than top-level settings (e.g., AUTH_USER_USERNAME_FIELD).
Suggestions and opinions welcome.

 * The swapping mechanic is set up using a new Meta option on models
called 'swappable' that defines the setting where an alternate model
can be defined. Technically, the swappable option *could* be used for
any model; however, I'm not proposing that we actually document