#3011 - Custom User Models -- Call for final review
Hi all, The implementation of custom User models is now ready for final review, prior to commit to trunk. The code is available in my Github repo, in the t3011 branch. https://github.com/freakboy3742/django/tree/t3011 The diff is available here: https://github.com/freakboy3742/django/compare/master...t3011 Documentation is also available in the auth topic guide: https://github.com/freakboy3742/django/blob/t3011/docs/topics/auth.txt#L1761 Barring the discovery of major problems, I'm aiming to commit this towards the end of next week. Any and all feedback is welcome. 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: #3011 - Custom User Models -- Call for final review
Hey Russ -- I took the liberty of opening up a pull request (https://github.com/django/django/pull/370) so that I can leave comments inline. Hope you don't mind. Jacob On Sat, Sep 15, 2012 at 7:34 AM, Russell Keith-Magee wrote: > Hi all, > > The implementation of custom User models is now ready for final > review, prior to commit to trunk. > > The code is available in my Github repo, in the t3011 branch. > > https://github.com/freakboy3742/django/tree/t3011 > > The diff is available here: > > https://github.com/freakboy3742/django/compare/master...t3011 > > Documentation is also available in the auth topic guide: > > https://github.com/freakboy3742/django/blob/t3011/docs/topics/auth.txt#L1761 > > Barring the discovery of major problems, I'm aiming to commit this > towards the end of next week. Any and all feedback is welcome. > > 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. > -- 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: #3011 - Custom User Models -- Call for final review
On 15 syys, 15:34, Russell Keith-Magee
wrote:
> Hi all,
>
> The implementation of custom User models is now ready for final
> review, prior to commit to trunk.
>
> The code is available in my Github repo, in the t3011 branch.
>
> https://github.com/freakboy3742/django/tree/t3011
>
> The diff is available here:
>
> https://github.com/freakboy3742/django/compare/master...t3011
>
> Documentation is also available in the auth topic guide:
>
> https://github.com/freakboy3742/django/blob/t3011/docs/topics/auth.tx...
>
> Barring the discovery of major problems, I'm aiming to commit this
> towards the end of next week. Any and all feedback is welcome.
The above branch does not work on Python3, which seems like a blocker
issue.
Some questions:
- If I proxy the auth.User model when it is swapped out I get an
error from model validation: "AttributeError: type object 'UserProxy'
has no attribute '_base_manager'". The error could be cleaner.
- The last_login field is in the AbstractBaseUser, but it isn't
documented as a required field. Is this field required for something?
Is it needed as part of AbstractBaseUser?
- There is no way to do single-table extension of the default User
model. Should there be a way? To me it seems this would be beneficial
when dealing with "project profiles". There is little reason I would
like to store a required employee number in another table. Maybe it
would work if current User was moved to AbstractUser, and User would
be class User(AbstractUser): class Meta: swappable =
'AUTH_USER_MODEL'? Quick testing indicates this could work... See
commit:
https://github.com/akaariai/django/commit/08bcb4aec1ed154cefc631b8510ee13e9af0c19d
- I did a little change to REQUIRED_FIELDS handling in conjunction
with create_(super)user(). See commit:
https://github.com/akaariai/django/commit/d9f5e5addbad5e1a01f67e7358e4f5091c3cad81
With the above commits I can do this:
class MyUser(AbstractUser):
employee_no = models.CharField(max_length=5)
some_other_field = models.TextField(null=True, blank=True)
REQUIRED_FIELDS = AbstractUser.REQUIRED_FIELDS + ['employee_no']
syncb & create_superuser commands work. Admin doesn't work directly,
but it seems you can easily subclass the default user admin & change
form to make it work.
Single-table inheritance seems useful to me, and the added complexity
to code isn't much.
The above questions aren't meant to be commit blockers. In my
(somewhat limited) testing the only real issue was the py3
incompatibility, and that seems to be an easy fix. Overall, this looks
good to me.
- Anssi
BTW: I did also a little test to allow select_related() for user
profiles:
https://github.com/akaariai/django/commit/81f91db3234110b90572b25ca69b9012a475c460
- the idea is that the profile models could just to
get_user_model().SELECT_RELATED_MODELS.add('user_reverse_name'). I
wonder if we want something like this now/later...
--
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: #3011 - Custom User Models -- Call for final review
On 15 syys, 18:59, Anssi Kääriäinen wrote: > With the above commits I can do this: > > class MyUser(AbstractUser): > employee_no = models.CharField(max_length=5) > some_other_field = models.TextField(null=True, blank=True) > REQUIRED_FIELDS = AbstractUser.REQUIRED_FIELDS + ['employee_no'] > > syncb & create_superuser commands work. Admin doesn't work directly, > but it seems you can easily subclass the default user admin & change > form to make it work. > > Single-table inheritance seems useful to me, and the added complexity > to code isn't much. Actually, there is more common use case which doesn't seem to be solved by the current branch: getting proxy instances into request.user. If you use just ProxyUser(auth.User) the proxy model will not be loaded into request.user, and if you set the ProxyUser as AUTH_USER_MODEL, then it will not work as you can't subclass the swapped out auth.User. The added commits solves this by allowing you to do: class MyUser(AbstractUser): # define additional methods etc. and: settings.AUTH_USER_MODEL = 'myuser' This could also be solved by somehow allowing fetching proxy instance to request.user. In any case, IMO this needs some solution before release. This feature is already looking really good. I know that every single one of my projects will have uses for this feature, so I am really looking forward to getting this into 1.5. - 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: Testing django 1.5 with firebirdsql
On 15 syys, 01:05, maxi wrote: > BTW, I've another issue related to one specific test. > In test/regressiontests/model_fields/models.py there is a model > defined as: > > class BigD(models.Model): > d = models.DecimalField(max_digits=38, decimal_places=30) > > Firebird doesn't accept that definitions for max_digits and > decimal_places (the max is 18) > It will be possible to add a DatabaseFeature attribute for avoid or > skip related tests? I have a feeling I have seen the max_digits issue before. Quick trac- search didn't reveal anything, though. But yes, a database feature is the way to go. - 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: #3011 - Custom User Models -- Call for final review
On Sat, Sep 15, 2012 at 11:59 PM, Anssi Kääriäinen
wrote:
> On 15 syys, 15:34, Russell Keith-Magee
> wrote:
>> Hi all,
>>
>> The implementation of custom User models is now ready for final
>> review, prior to commit to trunk.
>>
>> The code is available in my Github repo, in the t3011 branch.
>>
>> https://github.com/freakboy3742/django/tree/t3011
>>
>> The diff is available here:
>>
>> https://github.com/freakboy3742/django/compare/master...t3011
>>
>> Documentation is also available in the auth topic guide:
>>
>> https://github.com/freakboy3742/django/blob/t3011/docs/topics/auth.tx...
>>
>> Barring the discovery of major problems, I'm aiming to commit this
>> towards the end of next week. Any and all feedback is welcome.
>
> The above branch does not work on Python3, which seems like a blocker
> issue.
Dang - I knew I forgot something. I'll put this on my list to look at.
> Some questions:
> - If I proxy the auth.User model when it is swapped out I get an
> error from model validation: "AttributeError: type object 'UserProxy'
> has no attribute '_base_manager'". The error could be cleaner.
> - The last_login field is in the AbstractBaseUser, but it isn't
> documented as a required field. Is this field required for something?
> Is it needed as part of AbstractBaseUser?
Yes, last_login is required - it's needed in order to generate
password reset tokens etc.
It isn't documented because we *really* want people to be subclassing
AbstractBaseUser, not building their own User from scratch.
> - There is no way to do single-table extension of the default User
> model. Should there be a way? To me it seems this would be beneficial
> when dealing with "project profiles". There is little reason I would
> like to store a required employee number in another table. Maybe it
> would work if current User was moved to AbstractUser, and User would
> be class User(AbstractUser): class Meta: swappable =
> 'AUTH_USER_MODEL'? Quick testing indicates this could work... See
> commit:
> https://github.com/akaariai/django/commit/08bcb4aec1ed154cefc631b8510ee13e9af0c19d
> - I did a little change to REQUIRED_FIELDS handling in conjunction
> with create_(super)user(). See commit:
> https://github.com/akaariai/django/commit/d9f5e5addbad5e1a01f67e7358e4f5091c3cad81
>
> With the above commits I can do this:
>
> class MyUser(AbstractUser):
> employee_no = models.CharField(max_length=5)
> some_other_field = models.TextField(null=True, blank=True)
> REQUIRED_FIELDS = AbstractUser.REQUIRED_FIELDS + ['employee_no']
>
> syncb & create_superuser commands work. Admin doesn't work directly,
> but it seems you can easily subclass the default user admin & change
> form to make it work.
>
> Single-table inheritance seems useful to me, and the added complexity
> to code isn't much.
To be clear -- is this just targeting the "I'm completely happy with
auth.User; I just want to put XXX on the model" use case? If so, I can
certainly agree with this; I'll merge these changes in.
> The above questions aren't meant to be commit blockers. In my
> (somewhat limited) testing the only real issue was the py3
> incompatibility, and that seems to be an easy fix. Overall, this looks
> good to me.
>
> - Anssi
>
> BTW: I did also a little test to allow select_related() for user
> profiles:
> https://github.com/akaariai/django/commit/81f91db3234110b90572b25ca69b9012a475c460
> - the idea is that the profile models could just to
> get_user_model().SELECT_RELATED_MODELS.add('user_reverse_name'). I
> wonder if we want something like this now/later...
Possibly; but if we do, I'd rather attack it at the model Meta level
-- i.e., an analog to Meta: ordering that applies a select_related
automatically to every queryset.
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.
end of "coming soon" in the tutorial? - tutorial 5 feedback needed!
Katie Miller and Ben Sturmfels have written a fifth tutorial covering making "polls" a reusable app. I've given it an initial review, but I'm hoping we can get a few more people to give it a look and/or try it out, particularly if you are an expert in packaging (which I'm not). Thanks guys! p.s. I've built the docs with the latest patch here: http://techytim.com/django/tutorial05/intro/tutorial05.html -- 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/-/4EH8SfW_RjEJ. 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: #3011 - Custom User Models -- Call for final review
On Sun, Sep 16, 2012 at 1:07 AM, Anssi Kääriäinen wrote: > On 15 syys, 18:59, Anssi Kääriäinen wrote: >> With the above commits I can do this: >> >> class MyUser(AbstractUser): >> employee_no = models.CharField(max_length=5) >> some_other_field = models.TextField(null=True, blank=True) >> REQUIRED_FIELDS = AbstractUser.REQUIRED_FIELDS + ['employee_no'] >> >> syncb & create_superuser commands work. Admin doesn't work directly, >> but it seems you can easily subclass the default user admin & change >> form to make it work. >> >> Single-table inheritance seems useful to me, and the added complexity >> to code isn't much. > > Actually, there is more common use case which doesn't seem to be > solved by the current branch: getting proxy instances into > request.user. If you use just ProxyUser(auth.User) the proxy model > will not be loaded into request.user, and if you set the ProxyUser as > AUTH_USER_MODEL, then it will not work as you can't subclass the > swapped out auth.User. > > The added commits solves this by allowing you to do: > class MyUser(AbstractUser): > # define additional methods etc. > and: > settings.AUTH_USER_MODEL = 'myuser' > > This could also be solved by somehow allowing fetching proxy instance > to request.user. In any case, IMO this needs some solution before > release. I'm not sure there can be *any* clean answer for this. A base class can't be swapped out at time of class construction, so I'm not sure how we would handle proxied User models in any meaningful way. My inclination here is to raise an error if, during class construction, the base class of a proxy class is found to be swapped, and document this as a known limitation. Russ %-) -- 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.
