Re: AbstractUser to get more abstract

[Marc Tamlyn]

Thanks for compiling a more complete solution.

The biggest disadvantage as I see it is that is adds even more complexity
to the implementation of those forms. In particular the `setattr` on the
form class of a `clean_` method is a really nasty code smell. The code in
the `create_user`/`create_superuser` commands is also a prime example of
additional unnecessary complexity in a function which would be trivially
avoided by separation.

The authentication form renders a field called "username" which is a
CharField, not an EmailField. This seems pretty bad to me.

Other things which appear to be bugs to me:
- You automatically register an admin class for any custom user model,
which is likely to break validation.
- If I set first_name and last_name as REQUIRED_FIELDS then your admin
class will break validation or render a weird form as the field names
appear twice.
- Docstring for AbstractCommonUser is wrong.
- There are a number of style errors in the code - backslashes for line
continuation, unnecessarily wrapped lines, spaces in the wrong places etc.

The Admin related issues here are the biggest problem. If admin validation
fails for *any custom user model*, then the server will not start if
contrib.auth is installed.

Your main aim here seems to be to reduce code duplication by writing
clever, quasi-magical code instead. We're not just trying to make this work
for a particular new User model - we need to make it not break for *every*
user model. Writing custom user models is an inherently duplicatory process
in some places - I've done it in several projects and it always involves
lifting some components from Django and tweaking them appropriately. The
more we complexify the forms, the harder this is to do. Russell's original
point about providing a complete implementation of "a custom user model" as
an example is still very valid. Introducing an easier way to do say, email
based auth, should not make writing a custom social based auth solution
harder.

Now I admit there's not as much magic so far as I expected (though I expect
it may get worse in the test suite). However I think the points regarding
complexity still stand. From a maintenance perspective, changing this code
in two places is bad, but always having to consider at least two (in your
case actually many) distinct cases whenever changing the code is worse.


On 25 September 2013 01:04, Luke Sneeringer  wrote:

> Good evening, Russell, et. al.,
> I had some time this afternoon. :-) Since there are already a couple of
> reference implementations for how to do this with an e-mail app, I decided
> to take a crack at an implementation that would include an EmailUser within
> the base auth app. I understand we've far from decided on a method, and I
> still stand by my promise to do whatever Russell decides, but I figured
> code to look at would be a good thing, and that it would give me a better
> idea of the task.
>
> Here's the branch:
> https://github.com/lukesneeringer/django/tree/ticket-20824-noemailapp
>
> I have a few observations and self-criticisms:
>
>
>- The big advantage, as I see it, to this approach over a separate-app
>approach is that it *drastically* reduces code duplication. Forms, the
>admin, etc., all use the same classes they did before, just those classes
>are now a bit more aware of what the USERNAME_FIELD and REQUIRED_FIELDS
>are. This is really the reason I believe this to be the correct approach. A
>separate app would have to be maintained, well, separately. :-)
>- I have a few criticisms of my approach, now that I've had a chance
>to see it in action:
>   - The create_user and create_superuser methods on UserManager were
>   a bit of a land mine. The current (1.6) implementation takes username,
>   email, and password as positional arguments, and making this into an
>   all-kwarg approach would break back-compat. I landed on a solution, but 
> I'm
>   not sure it's the right one.
>   - In order to maintain the form error message but not have the form
>   itself be opinionated about the presence of "username", I removed the
>   explicit validator on the form, simply letting ModelForm outsource it to
>   the Model. This provides the same error message for both invalid and
>   duplicate usernames / e-mail addresses (as appropriate). I think this
>   actually might be a *good* thing (again, code duplication is reduced), 
> but
>   I don't really know if there's some reason why it was constructed the 
> way
>   it was before.
>   - There's a special message for duplicate usernames ("A user with
>   that username already exists."), and it's not a trivial thing to just 
> make
>   an e-mail address correlate because of internationalization. I reverted 
> to
>   the previous message, which was translated ("Please correct the 
> duplicate
>   data for email, which must be unique."), because I really don't know 
> what
>   the 

Re: AbstractUser to get more abstract

[Luke Sneeringer]

On Sep 25, 2013, at 2:58 AM, Marc Tamlyn  wrote:

> Thanks for compiling a more complete solution.
> 
> The biggest disadvantage as I see it is that is adds even more complexity to 
> the implementation of those forms. In particular the `setattr` on the form 
> class of a `clean_` method is a really nasty code smell. The code in the 
> `create_user`/`create_superuser` commands is also a prime example of 
> additional unnecessary complexity in a function which would be trivially 
> avoided by separation.
> 
> The authentication form renders a field called "username" which is a 
> CharField, not an EmailField. This seems pretty bad to me.

It does this in 1.5 and 1.6. AuthenticationForm is already used for custom user 
models; this doesn't represent an alteration on my part.

> Other things which appear to be bugs to me:
> - You automatically register an admin class for any custom user model, which 
> is likely to break validation.

Rats. I thought about that particular issue, but forgot to address it.

> - If I set first_name and last_name as REQUIRED_FIELDS then your admin class 
> will break validation or render a weird form as the field names appear twice.

This is a good point, although it could be avoided using a set instead of a 
list.

> The more we complexify the forms, the harder this is to do. Russell's 
> original point about providing a complete implementation of "a custom user 
> model" as an example is still very valid. Introducing an easier way to do 
> say, email based auth, should not make writing a custom social based auth 
> solution harder.

I'm not clear on how this would make that harder. It would either make it 
easier, or not introduce a change at all.

> Now I admit there's not as much magic so far as I expected (though I expect 
> it may get worse in the test suite). However I think the points regarding 
> complexity still stand. From a maintenance perspective, changing this code in 
> two places is bad, but always having to consider at least two (in your case 
> actually many) distinct cases whenever changing the code is worse.

This is an important point. Backward compatibility changes become more serious 
when there are more instances of custom user models in the wild that are able 
to use the basic UserCreationForm (and similar) classes. That actually might be 
the ultimate reason why this solution won't be tenable (regardless of 
preferability), because any maintenance on the forms would have back-compat 
implications for unknown custom User models.

L

> On 25 September 2013 01:04, Luke Sneeringer  wrote:
> Good evening, Russell, et. al.,
> I had some time this afternoon. :-) Since there are already a couple of 
> reference implementations for how to do this with an e-mail app, I decided to 
> take a crack at an implementation that would include an EmailUser within the 
> base auth app. I understand we've far from decided on a method, and I still 
> stand by my promise to do whatever Russell decides, but I figured code to 
> look at would be a good thing, and that it would give me a better idea of the 
> task.
> 
> Here's the branch: 
> https://github.com/lukesneeringer/django/tree/ticket-20824-noemailapp
> 
> I have a few observations and self-criticisms:
> 
> The big advantage, as I see it, to this approach over a separate-app approach 
> is that it drastically reduces code duplication. Forms, the admin, etc., all 
> use the same classes they did before, just those classes are now a bit more 
> aware of what the USERNAME_FIELD and REQUIRED_FIELDS are. This is really the 
> reason I believe this to be the correct approach. A separate app would have 
> to be maintained, well, separately. :-)
> I have a few criticisms of my approach, now that I've had a chance to see it 
> in action:
> The create_user and create_superuser methods on UserManager were a bit of a 
> land mine. The current (1.6) implementation takes username, email, and 
> password as positional arguments, and making this into an all-kwarg approach 
> would break back-compat. I landed on a solution, but I'm not sure it's the 
> right one.
> In order to maintain the form error message but not have the form itself be 
> opinionated about the presence of "username", I removed the explicit 
> validator on the form, simply letting ModelForm outsource it to the Model. 
> This provides the same error message for both invalid and duplicate usernames 
> / e-mail addresses (as appropriate). I think this actually might be a *good* 
> thing (again, code duplication is reduced), but I don't really know if 
> there's some reason why it was constructed the way it was before.
> There's a special message for duplicate usernames ("A user with that username 
> already exists."), and it's not a trivial thing to just make an e-mail 
> address correlate because of internationalization. I reverted to the previous 
> message, which was translated ("Please correct the duplicate data for email, 
> which must be unique."), because I really

Re: AbstractUser to get more abstract

[Luke Sneeringer]

On Sep 25, 2013, at 2:58 AM, Marc Tamlyn  wrote:

> Other things which appear to be bugs to me:
> - You automatically register an admin class for any custom user model, which 
> is likely to break validation.
> - If I set first_name and last_name as REQUIRED_FIELDS then your admin class 
> will break validation or render a weird form as the field names appear twice.
> 
> The Admin related issues here are the biggest problem. If admin validation 
> fails for *any custom user model*, then the server will not start if 
> contrib.auth is installed.

I fixed these. :-)

I am by no means claiming that these are the only drawbacks of my approach, but 
this is a simple fix: only register User or EmailUser.

Thanks!

L

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

Re: Hyperlink CSS on djangoproject.com

[Daniele Procida]

On Tue, Sep 24, 2013, Marc Tamlyn  wrote:

>As someone who has color blindness (off the top of my head I can't remember
>the exact condition, but it's not simple red-green that 1 in 8 men have) it
>looks fine to me. Personally I like the distinction between the external
>and internal links.

It's not so much between internal and external links, but between links to 
documentation topics (orange, not underlined) and links to Django objects 
(green, underlined).

>My guidance for people is generally that if you aren't using the actual
>colour to denote meaning (eg Red is bad), and the colours still appear
>different in monochrome, then chances are everyone can see it OK. Maybe not
>as clear as you, but OK enough.

Yes, the contrast between links and documentation topics links (i.e. not 
underlined) seems OK, even though they are less obviously links.

Daniele

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

Re: Hyperlink CSS on djangoproject.com

[Tim Graham]

See also ticket #14834 "Colour issues in CSS - particularly documentation."

However, it may not be worth working on this right now as I believe there's 
an initiative to redesign djangoproject.com (although it's been nearly a 
year since the email thread on the topic has been updated).

see 
https://groups.google.com/d/topic/django-developers/2Xy7SZAOc7E/discussion

On Wednesday, September 25, 2013 12:42:58 PM UTC-4, Daniele Procida wrote:
>
> On Tue, Sep 24, 2013, Marc Tamlyn > 
> wrote: 
>
> >As someone who has color blindness (off the top of my head I can't 
> remember 
> >the exact condition, but it's not simple red-green that 1 in 8 men have) 
> it 
> >looks fine to me. Personally I like the distinction between the external 
> >and internal links. 
>
> It's not so much between internal and external links, but between links to 
> documentation topics (orange, not underlined) and links to Django objects 
> (green, underlined). 
>
> >My guidance for people is generally that if you aren't using the actual 
> >colour to denote meaning (eg Red is bad), and the colours still appear 
> >different in monochrome, then chances are everyone can see it OK. Maybe 
> not 
> >as clear as you, but OK enough. 
>
> Yes, the contrast between links and documentation topics links (i.e. not 
> underlined) seems OK, even though they are less obviously links. 
>
> Daniele 
>
>

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

Add strutctured settings module to django 1.7?

[VernonCole]

I find myself using up lots of time and keystrokes explaining about the 
benefits and methods of a structured settings module in django.  
For example: 
Using-a-Structured-Settings-environment

It occurs to me that life would be easier if django shipped already set up 
for structured settings, rather than having to retro-fit it.  The pull 
request I sent to formhub could be a starting point for the conversion. (
formhub/pull/1240)   The 
changes to core are simple enough that I could do it.

Your  opinions please, Would such a change be good for django 1.7?
--
Vernon Cole
eHealth Africa

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

Re: Add strutctured settings module to django 1.7?

[Russell Keith-Magee]

On Thu, Sep 26, 2013 at 4:21 AM, VernonCole  wrote:

> I find myself using up lots of time and keystrokes explaining about the
> benefits and methods of a structured settings module in django.
> For example: 
> Using-a-Structured-Settings-environment
>
> It occurs to me that life would be easier if django shipped already set up
> for structured settings, rather than having to retro-fit it.  The pull
> request I sent to formhub could be a starting point for the conversion. (
> formhub/pull/1240)   The
> changes to core are simple enough that I could do it.
>
> Your  opinions please, Would such a change be good for django 1.7?
>

Based on conversations that I had at the sprints at DjangoCon US, I'd say
probably not -- because the winds are blowing in another direction.

The primary reason to need structured settings like this is because your
development environment is different to your deployment environment (e.g.,
different passwords, paths and so on).

However, the emerging best practice for this sort of thing is best
described by the "12 factor" approach:

http://12factor.net

Factors 10 and 3 are the most relevant to this discussion -- 10 says that
development and production should be the same; 3 says that anything that
needs to vary should be set as an environment variable, and consumed from
there.

So - I'd expect to see Django moving towards better support for a 12 factor
environment, rather than embedding separate settings files as a deployment
practice.

Yours,
Russ Magee %-)

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Add strutctured settings module to django 1.7?

[Michael Mior]

Given that, what about incorporating something like DJ-Database-URL[1] into 
Django? It would be great if this could be the default if DATABASES was 
left unspecified.

[1] https://github.com/kennethreitz/dj-database-url

Le mercredi 25 septembre 2013 19:45:20 UTC-4, Russell Keith-Magee a écrit :
>
>
> On Thu, Sep 26, 2013 at 4:21 AM, VernonCole 
> > wrote:
>
>> I find myself using up lots of time and keystrokes explaining about the 
>> benefits and methods of a structured settings module in django.  
>> For example: 
>> Using-a-Structured-Settings-environment
>>
>> It occurs to me that life would be easier if django shipped already set 
>> up for structured settings, rather than having to retro-fit it.  The pull 
>> request I sent to formhub could be a starting point for the conversion. (
>> formhub/pull/1240)   The 
>> changes to core are simple enough that I could do it.
>>
>> Your  opinions please, Would such a change be good for django 1.7?
>>
>
> Based on conversations that I had at the sprints at DjangoCon US, I'd say 
> probably not -- because the winds are blowing in another direction.
>
> The primary reason to need structured settings like this is because your 
> development environment is different to your deployment environment (e.g., 
> different passwords, paths and so on). 
>
> However, the emerging best practice for this sort of thing is best 
> described by the "12 factor" approach:
>
> http://12factor.net
>
> Factors 10 and 3 are the most relevant to this discussion -- 10 says that 
> development and production should be the same; 3 says that anything that 
> needs to vary should be set as an environment variable, and consumed from 
> there.
>
> So - I'd expect to see Django moving towards better support for a 12 
> factor environment, rather than embedding separate settings files as a 
> deployment practice.
>
> Yours,
> Russ Magee %-)
>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
For more options, visit https://groups.google.com/groups/opt_out.