Re: get_cache and multiple caches

2013-09-19 Thread Curtis Maloney
I guess the remaining question to address is :  close()

It looks like it was added to appease an issue with memcached, which may or
may not still be an issue [comments in tickets suggest it was a design
decision by the memcached authors].

Thinking as I type... it wouldn't hurt, also, to allow a cache backend to
provide an interface to a connection pool, so the manager can play friendly
with it.  If it doesn't have one, fall back to an instance-per-thread...
this would require still hooking request complete, but not so much for
"close" as "release".

--
Curtis



On 19 September 2013 01:33, Florian Apolloner  wrote:

> Hi,
>
>
> On Wednesday, September 18, 2013 1:29:25 PM UTC+2, Curtis Maloney wrote:
>>
>> 1) Can we share "ad-hoc" caches -- that is, ones created by passing more
>> than just the CACHES alias.
>>
> Imo no, you probably have a good reason if you create ad-hoc ones
>
>> 2) What to do about django.core.cache.cache ?
>>
> Has to stay for now, same as django.db.connection
>
>
>> A separate approach is to introduce a new API to provide access to the
>> shared, pre-configured caches, and retain get_cache for the old, ad-hoc,
>> non-shared caches.
>>
> I think it would be sensible if that API would mimic django.db.connections
>
> Florian.
>
> --
> 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.
>

-- 
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: FormSetView and ModelFormSetView

2013-09-19 Thread Russell Keith-Magee
On Fri, Sep 20, 2013 at 2:41 PM, Marc Tamlyn  wrote:

> This is partly because there's no obvious correct implementation of them ;)
>
> Yes, I think these views should exist. But they go with the same body of
> work as handling multiple forms, inline formsets etc. At present I have no
> yet found the time to think about this problem, as a whole and come up with
> a single consistent solution.
>
I'm not sure FormSetView *should* exist. I'd be a lot more inclined to look
at something like FormContainer -- a form-like container that can contain
multiple forms and formsets. There's a sample implementation on #18830 in
the context of extending FormWizard to support combinations of form and
formset.

https://code.djangoproject.com/ticket/18830

If you had a form container, you wouldn't need a new generic view -- you
can just use FormView, and use the FormContainer as the form.

(I'd also like to rethink generics in general, but that's a whole other
conversation… and one that I need to internally progress from vauge
rambling to slightly coherent proposal… and then find time to work on :-)

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: Is "transaction.atomic" in 1.6 supposed to work this way?

2013-09-19 Thread Aymeric Augustin
Yes, I agree, that why I suggested filing a documentation ticket in my first 
answer.

-- 
Aymeric.

> Le 20 sept. 2013 à 01:32, Richard Ward  a écrit :
> 
> I think the docs could be improved

-- 
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: FormSetView and ModelFormSetView

2013-09-19 Thread Marc Tamlyn
This is partly because there's no obvious correct implementation of them ;)

Yes, I think these views should exist. But they go with the same body of
work as handling multiple forms, inline formsets etc. At present I have no
yet found the time to think about this problem, as a whole and come up with
a single consistent solution.

If you feel like writing a concrete proposal, go for it. My concerns are
that any proposal should cover:
Formsets
Model formsets, bound to existing models
Inline model formsets with create and update of parent model
More than one form/formset in a view - both will all needing to be valid or
any.

Oh, and try to avoid more spaghetti code in the GCBV API as we'll ;)

Marc
On 20 Sep 2013 00:20, "Gert Steyn"  wrote:

> Hi All
>
> What are the chances of adding FormSetView and ModelFormSetView classes to
> the standard set of generic views?
>
> I know its not to much effort to do it yourself, but it will help a lot
> with consistency across different Django applications. Currently there are
> multiple 3rd party versions of this available, all of them are slightly
> different and none of them seem to stand out as an obvious choice.
>
> Most Django developers don't have the same skills as the core developers,
> please provide your guidance with this.
>
> Regards
> Gert Steyn
>
>  --
> 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.
>

-- 
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: Is "transaction.atomic" in 1.6 supposed to work this way?

2013-09-19 Thread Aymeric Augustin

On 19 sept. 2013, at 23:38, Anssi Kääriäinen  wrote:

> Here, another_obj.save() will succeed. But the transaction will be *always* 
> rolled back, silently. I don't see any good reason not to error out when 
> using a connection which is going to be rolled back in always.

As far as I can tell that's the actual proposal in your email, and I'm OK with 
trying to to raise exceptions when making a query and get_rollback returns 
True. I'm not sure it's doable; patch welcome ;-)

-- 
Aymeric.



-- 
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: Is "transaction.atomic" in 1.6 supposed to work this way?

2013-09-19 Thread Aymeric Augustin
On 19 sept. 2013, at 22:47, Hannu Krosing  wrote:

> Actually you can use subtransactions in postgresql if you want to manage
> failed queries yourself

That's exactly what transaction.atomic does, on all supported databases.

I'm sorry if I sound like I need a transactions 101 course; I actually spent a 
few dozen hours working on this before rewriting Django's transaction 
management ;-)

-- 
Aymeric.

-- 
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: AbstractUser to get more abstract

2013-09-19 Thread Russell Keith-Magee
On Fri, Sep 20, 2013 at 12:11 PM, Luke Sneeringer wrote:

>
>
> Sent from my iPad
>
> On Sep 19, 2013, at 9:24 PM, Russell Keith-Magee 
> wrote:
>
>
> On Fri, Sep 20, 2013 at 10:53 AM,  wrote:
>
>> > Note that EmailUser *doesn't* have a Meta: swappable definition. There
>> is nothing on this model that says "I am swappable", or "I am an
>> appropriate substitute for User".
>>
>> Ah, this is were the misunderstanding was. authtools does actually set
>> swappable on its replacement user, too[1]. The two definitions look like
>> this:
>>
>> class User(Model):
>> username = models.CharField()
>> USERNAME_FIELD = 'username'
>> class Meta:
>> swappable = 'AUTH_USER_MODEL'
>>
>> class EmailUser(Model):
>> email = models.CharField()
>> USERNAME_FIELD = 'email'
>> class Meta:
>>swappable = 'AUTH_USER_MODEL'
>>
>> > How does Django identify that EmailUser *shouldn't* be synchronised to
>> the database?
>>
>> The existing swappable mechanism takes care of it. Only the model
>> referenced by settings.AUTH_USER_MODEL will be installed, not both.
>>
>
> Ah. Thats… interesting. I'm moderately surprised it works at all. I'll
> need to dig in to the internals to work out *why* it works… swappable
> certainly wasn't intended to work like that. There might be some
> interesting land mines lying in wait.
>
>
> Aha, I think I found at least some of my disconnect. But I tested this
> yesterday, and it definitely does do that.
>

Agreed - this explains the source of the disconnect.


> I know I just asked this in a previous email, but I will repeat for ease
> of others possibly following this thread: can you provide a brief overview
> of what swappable *does* do? It seems to be on observation it does exactly
> this. You say it's intended to do something else. What is that something
> else?
>

The intention was to mark a particular model as a something that can be
replaced. It's about marking the endpoint that can be swapped, not
predeclaring all possible participants in a swap.

In the case of contrib.auth: Auth requires the concept of a model that
represents a User. At the end of the day, it shouldn't matter which model
that user actually is -- it just needs to be a model, and we can determine
at runtime what that model should be. You don't have to predeclare that
you're going to be used as a User - you just point to the model using
AUTH_USER_MODEL.

For ease of use (and backwards compatibility) we need to ship a concrete
User model. But then we need to be able to:

 * Not install the model if it's been replaced, and

 * Easily answer the runtime question "what have you been swapped out for?"

This is done by checking Meta.swappable, reading the value of the setting,
and checking if Meta.model_label matches the value in the setting. This is
cached as Meta.swapped. The internals of Django (like syncdb) then check
Meta.swapped as part of the checks performed to see if a model should be
included in any given activity.

I've had a bit of a closer look at the implementation with your use case in
mind, and I'm less concerned about land mines caused your proposed usage of
swappable. It works by happy accident :-)

However, I'd still make the argument that while it's *possible* to use
swappable in this way, it's still not desirable.

Firstly, it leads to inconsistent usage of API. Under the current scheme,
the default User model defines swappable. No other User model needs to. In
your proposed usage, you've just made a special case of "Swappable models
in the same app". Now, granted - this discrepancy isn't something that
needs to be documented (because swappable isn't documented), but if someone
goes looking, or if we ever want to formalise swappable (which is certainly
my long term hope), we need to explain why some models need swappable
defined and others don't.

Alternatively, if we change the game to say that swappable *is* required on
all models, then we have a backwards compatibility problem (because no
existing custom user model needs swappable to be defined). *Any* model can
be swapped in as a substitute user.

If we were to go down this path, the logical extension (to my mind) would
be to validate that you're not swapping in a model that hasn't declared
itself as swappable (otherwise, why would you go to the trouble of
predeclaring?). This *isn't* currently being done, and I'm not convinced it
has value. This is an instance where duck typing works really well. There's
also the example I raised about comments on User - you aren't necessarily
in control of the model that you want to use as a swappable alternative.

Secondly, marking a model as swappable doesn't make it zero cost. It's
still parsed, loaded and added to the app cache, even if it isn't used.
Django's internals then ignore any model that is marked as swapped. If we
put EmailUser in contrib.auth, it's *always* going to be part of every
project that uses auth, regardless of whether you're

Re: AbstractUser to get more abstract

2013-09-19 Thread Luke Sneeringer


Sent from my iPad

> On Sep 19, 2013, at 9:24 PM, Russell Keith-Magee  
> wrote:
> 
> 
>> On Fri, Sep 20, 2013 at 10:53 AM,  wrote:
>> > Note that EmailUser *doesn't* have a Meta: swappable definition. There is 
>> > nothing on this model that says "I am swappable", or "I am an appropriate 
>> > substitute for User".
>> 
>> Ah, this is were the misunderstanding was. authtools does actually set 
>> swappable on its replacement user, too[1]. The two definitions look like 
>> this:
>> 
>> class User(Model):
>> username = models.CharField()
>> USERNAME_FIELD = 'username'
>> class Meta:
>> swappable = 'AUTH_USER_MODEL'
>> 
>> class EmailUser(Model):
>> email = models.CharField()
>> USERNAME_FIELD = 'email'
>> class Meta:
>>swappable = 'AUTH_USER_MODEL'
>> 
>> > How does Django identify that EmailUser *shouldn't* be synchronised to the 
>> > database? 
>> 
>> The existing swappable mechanism takes care of it. Only the model referenced 
>> by settings.AUTH_USER_MODEL will be installed, not both. 
> 
> Ah. Thats… interesting. I'm moderately surprised it works at all. I'll need 
> to dig in to the internals to work out *why* it works… swappable certainly 
> wasn't intended to work like that. There might be some interesting land mines 
> lying in wait.

Aha, I think I found at least some of my disconnect. But I tested this 
yesterday, and it definitely does do that.

I know I just asked this in a previous email, but I will repeat for ease of 
others possibly following this thread: can you provide a brief overview of what 
swappable *does* do? It seems to be on observation it does exactly this. You 
say it's intended to do something else. What is that something else?

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: AbstractUser to get more abstract

2013-09-19 Thread Luke Sneeringer


> On Sep 19, 2013, at 8:06 PM, Russell Keith-Magee  
> wrote:
> 
> Note that EmailUser *doesn't* have a Meta: swappable definition. There is 
> nothing on this model that says "I am swappable", or "I am an appropriate 
> substitute for User".
> 

But isn't that assuming your conclusion? The point we are making is that 
EmailUser *should* have the swappable Meta option.

> Question: How does Django identify that EmailUser *shouldn't* be synchronised 
> to the database? EmailUser isn't mentioned in any setting (because 
> AUTH_USER_MODEL == 'auth.User'). There's nothing on EmailUser that flags it 
> as being a "User" model. It's in a models file with other models (Group, 
> Permission) which *will* be synchronized. Absent of any additional 
> information, EmailUser will be synchronised, because it looks like a normal 
> model.
> 
> Answer: We have 2 options. We need to modify contrib/auth/models.py to read:
> 
> class User(Model):
>…
> 
> if settings.AUTH_USER_MODEL == 'auth.EmailUser':
> class EmailUser(Model):
> ….

No, you just add the swappable Meta option, and then it "just works" as far as 
the model is concerned, right? (I could test this...)

>  1) We need to a bunch of internal plumbing in the app loader, Meta classes 
> and so on to determine whether or not a model should be synchronised, and 
> "hide" it in some way if it shouldn't be there. If you think this logic is 
> simple, I've got a bridge to sell you :-)

This is where I think I must be missing something. Doesn't swappable already do 
this? If not, what *does* swappable do?

>  2) We have to introduce a backwards incompatible change to pluggable models. 
> At present you don't have to pre-declare "my model is a User model". All 
> existing swappable User models in the wild need to be updated to be "1.7 
> compliant".

swappable is opt-in, not opt-out. Since existing models people are writing are 
not swappable, it would just work. The opt-in is to the ability to be subsumed 
if the setting does not match, not the ability to be used if it does.

> Does that clarify the problem for you?

Not really, to be honest, because your statements about how the existing system 
works are divergent from my observations. I am therefore very confused...even 
though I have mostly lurked here, I respect you highly and am inclined to trust 
your judgment. On the other hand, I can't figure out why your interpretation of 
the exists system is so different from what I seem to observe.

Maybe you could help me with something: Could you explain to me what swappable 
*does* do? Here's what I think it does: I think it provides the mechanism to 
say, "do not use this model unless the setting says to use me". I infer from 
both you and Marc that's not what it does (Marc said I was proposing a "clever 
use"). That's fine. What does it do? Help me out. I feel certain I am just one 
step away from the lightbulb moment, but I really want to understand.

I have the repo checked out, tests running, and want to work on this. But I 
want to make sure I do the best work possible, so I want to make sure I 
understand the problem space as much I can. Aim first, then fire, and all that. 
:-)

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: AbstractUser to get more abstract

2013-09-19 Thread Russell Keith-Magee
On Fri, Sep 20, 2013 at 10:53 AM,  wrote:

> > Note that EmailUser *doesn't* have a Meta: swappable definition. There
> is nothing on this model that says "I am swappable", or "I am an
> appropriate substitute for User".
>
> Ah, this is were the misunderstanding was. authtools does actually set
> swappable on its replacement user, too[1]. The two definitions look like
> this:
>
> class User(Model):
> username = models.CharField()
> USERNAME_FIELD = 'username'
> class Meta:
> swappable = 'AUTH_USER_MODEL'
>
> class EmailUser(Model):
> email = models.CharField()
> USERNAME_FIELD = 'email'
> class Meta:
>swappable = 'AUTH_USER_MODEL'
>
> > How does Django identify that EmailUser *shouldn't* be synchronised to
> the database?
>
> The existing swappable mechanism takes care of it. Only the model
> referenced by settings.AUTH_USER_MODEL will be installed, not both.
>

Ah. Thats… interesting. I'm moderately surprised it works at all. I'll need
to dig in to the internals to work out *why* it works… swappable certainly
wasn't intended to work like that. There might be some interesting land
mines lying in wait.


> > we still have a problem -- We can't just say
> "contrib.auth.forms.AuthenticationForm", because this form needs to change
> depending on the model that is in use. We have similar problems with admin,
> and potentially with views and URLs.
>
> It's straightforward to make the forms, views, and URLs work without
> checking what user model is installed, and this is the approach authtools
> takes. We can agree that code like `if settings.AUTH_USER_MODEL ==
> auth.EmailUser"` is absolutely awful, so it's nice that it's not required
> to implement forms that work with custom user models. The forms from
> authtools will work with any user model, not just authtools.User and
> auth.User.
>

Well… no, they won't. They'll work with a *very* wide subset of User
models, but not *every* user model. Most notably, you're relying on the
ModelForms providing the right widgets for all fields, which will be a
valid assumption for *most* models, but not *all* models. Given that Django
aims for a 90% solution, I'm not sure that this is a major problem, however.

What *is* a major problem is the issues you'll have with tests. Your code
binds User -- and thus the forms -- at import time. You're going to have
fun using those forms in a test environment that is swapping User models.

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: AbstractUser to get more abstract

2013-09-19 Thread gavinwahl
> Note that EmailUser *doesn't* have a Meta: swappable definition. There is 
nothing on this model that says "I am swappable", or "I am an appropriate 
substitute for User".

Ah, this is were the misunderstanding was. authtools does actually set 
swappable on its replacement user, too[1]. The two definitions look like 
this:

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

class EmailUser(Model):
email = models.CharField()
USERNAME_FIELD = 'email'
class Meta:
   swappable = 'AUTH_USER_MODEL'

> How does Django identify that EmailUser *shouldn't* be synchronised to 
the database? 

The existing swappable mechanism takes care of it. Only the model 
referenced by settings.AUTH_USER_MODEL will be installed, not both. 

> we still have a problem -- We can't just say 
"contrib.auth.forms.AuthenticationForm", because this form needs to change 
depending on the model that is in use. We have similar problems with admin, 
and potentially with views and URLs.

It's straightforward to make the forms, views, and URLs work without 
checking what user model is installed, and this is the approach authtools 
takes. We can agree that code like `if settings.AUTH_USER_MODEL == 
auth.EmailUser"` is absolutely awful, so it's nice that it's not required 
to implement forms that work with custom user models. The forms from 
authtools will work with any user model, not just authtools.User and 
auth.User. It doesn't use any ugly switches on the type of the installed 
user model to do it either. (Note: the views and URLs don't actually have 
to change to accommodate EmailUser. authtools ships with the CBV auth views 
simply because #17209[2] has stalled.)

[1]: 
https://github.com/fusionbox/django-authtools/blob/master/authtools/models.py#L86
[2]: https://code.djangoproject.com/ticket/17209

-- 
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: AbstractUser to get more abstract

2013-09-19 Thread Russell Keith-Magee
On Fri, Sep 20, 2013 at 2:21 AM, Luke Sneeringer wrote:

> But Django already has the mechanism to know not to install the model.
> It's the "swappable" Meta option. That is what causes User not to be
> installed if it is not used (contra my initial statement yesterday; I was
> flat wrong).
>
> Right now, User has a meta option:
>
> swappable = 'AUTH_USER_MODEL'
>
> ...that does exactly what we are describing. It seems EmailUser (or
> whatever we call it) should just do this also.
>
>
No, it doesn't -- at least, not completely and in both directions. See my
full answer to Aaron.

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: AbstractUser to get more abstract

2013-09-19 Thread Russell Keith-Magee
Hi Jorge,

On Thu, Sep 19, 2013 at 11:45 PM, Jorge Cardoso Leitão wrote:

> Hi all.
>
> I summarise the options with some of the issues raised, and I add my own
> concern.
>
>
> One option presented here is to have both models in d.c.auth. As pointed
> out by Russell and others, this causes the problem on how to define the
> models such that they are not installed if they are not being
> the AUTH_USER_MODEL. Assuming that this problem could be solved, the
> user's procedure would be:
>
> To use User (+ Permissions):
> install django.contrib.auth
>
> To use MailUser (+ Permissions):
> install django.contrib.auth
> set AUTH_USER_MODEL to MailUser
>
> The second option presented here is to add a separate app (e.g.
> d.c.mailuser), which would require some imports from django.contrib.auth
> (forms, views). The user's procedure would be:
>
> To use User (+ Permissions):
> install django.contrib.auth
>
> To use MailUser (+ Permissions):
> install django.contrib.auth
> install django.contrib.mailauth
> set AUTH_USER_MODEL to MailUser
>
> Is this correct or I'm missing something important here?
>
>
You've correctly described the two options under discussion. There is an
open question on exactly how to implement option 1, but from a public API
perspective, you're correct.


> If correct, I think the second option is cleaner, even though there is the
> issue to minimise the repetition of code for forms, views, which was
> pointed out to be easier to solve than the problem of model installation.
>
>
> What I want to emphasise here is that django.contrib.auth has to be
> installed in the case the custom model (email or other) is subclassed
> from PermissionsMixin.
>
> This is something that I'm bringing because it seems odd that d.c.auth has
> to be installed even if we want to use mailuser (or any user). Wouldn't be
> possible to detach permissions to a new app, say django.contrib.perms, and
> remove the step of installing d.c.auth? I mean, we are in authentication;
> permissions are not related with authentication.
>

This has been discussed in the past -- largely because there's a difference
between authentication and authorisation. If we had our time over, we'd
probably make this distinction better, and put permissions into a separate
app. However, we're stuck with 8 years of legacy now, and it's hard to
break this.

Unless you can propose a backwards-compatible approach to this problem -
i.e., making sure that every existing project that *doesn't* have
contrib.permissions in INSTALLED_APPS continues to work -- splitting
permissions into a separate app isn't really a viable option.

Also - I'm not sure I completely agree with the argument that if you're
using contrib.emailauth, you shouldn't have to put contrib.auth in
INSTALLED_APPS. This comes down to the idea of "what is an app" -- is it
just models, or is it views, URLS, templates, and more? If it's just models
I might agree with you, but an app is much more than that. If you're using
contrib.emailauth, you're still using auth views, middlewares, and so on.

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: AbstractUser to get more abstract

2013-09-19 Thread Russell Keith-Magee
Hi Bruno,

On Fri, Sep 20, 2013 at 1:26 AM, Bruno Ribeiro da Silva <
cont...@brunoribeiro.org> wrote:

> Hi, I develop applications in Django and I started to read the developer
> mailing list and got interested by this topic about MailUser problem and I
> want to add my 50 cents, sorry if I'm taking a completely wrong approach,
> since I never developed the internals of Django.
>
> But what if we just add a AUTH_USERNAME_FIELD to django settings and with
> this modify the behaviour of User model?
>
> I did some fast coding to show you what I'm thinking:
> https://github.com/loop0/django/compare/email_user
>
> Isn't this a simpler approach? Does it have too much implications? I
> tested changing AUTH_USERNAME_FIELD on a sample project settings to email
> and it worked login to admin.
>
> I like Jorge's idea to separate permissions into it's own app too.
>
> Sorry if I wasn't supposed to post to this mailing group.
>
> You're certainly allowed to post to the group -- we accept ideas from
anyone.

However, in this case, I'm gonna say No. We've deliberately introduced a
setting to allow people to swap in an arbitrary User model, because while
the username field is a *common* thing to want to change, it isn't the
*only* thing people want to change, and email vs username isn't the only
choice. Introducing a second setting that covers part of the potential use
case doesn't appeal to me at all.

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: AbstractUser to get more abstract

2013-09-19 Thread Russell Keith-Magee
Hi Aaron.

On Fri, Sep 20, 2013 at 8:00 AM, Aaron Merriam wrote:

> Russell, my reply there isn't meant to be confrontational and yet
> re-reading it I can see it being easily interpreted as such.
>

No worries - I didn't read your response as confrontational, but I'll make
sure I continue to not do so :-)


> My intention is to ask whether there is something about swappable that I
> am missing.  It seems to work the way I stated above but your statement
> seems to imply that their is some missing piece that needs extra mechanisms
> surrounding model loading.  So, am I missing something?
>

I think you are.

Lets try via example. django.contrib.auth contains 2 User models -- User,
and EmailUser. The (massively truncated) definitions for these models are:

class User(Model):
   username = models.CharField()

   USERNAME_FIELD = 'username'

   class Meta:
   swappable = 'AUTH_USER_MODEL'

class EmailUser(Model):
email = models.CharField()

   USERNAME_FIELD = 'email'

Note that EmailUser *doesn't* have a Meta: swappable definition. There is
nothing on this model that says "I am swappable", or "I am an appropriate
substitute for User".

So - Lets look at two cases.

Case 1: You set AUTH_USER_MODEL = 'auth.EmailModel'. You run syncdb.
EmailUser is synchronised because it's a model in an app. User *isn't*
synchronised because it's explicitly marked as being swappable, and the
swappable attribute doesn't point at itself. Problem solved, right? We've
got the right models syncronized!

Well, no - we still have a problem -- We can't just say
"contrib.auth.forms.AuthenticationForm", because this form needs to change
depending on the model that is in use. We have similar problems with admin,
and potentially with views and URLs.

Case 2: The default case: I just want the default User model. I install
django.contrib.auth in INSTALLED_APPS. I run syncdb. User is synchronised,
as we expect.

Question: How does Django identify that EmailUser *shouldn't* be
synchronised to the database? EmailUser isn't mentioned in any setting
(because AUTH_USER_MODEL == 'auth.User'). There's nothing on EmailUser that
flags it as being a "User" model. It's in a models file with other models
(Group, Permission) which *will* be synchronized. Absent of any additional
information, EmailUser will be synchronised, because it looks like a normal
model.

Answer: We have 2 options. We need to modify contrib/auth/models.py to read:

class User(Model):
   …

if settings.AUTH_USER_MODEL == 'auth.EmailUser':
class EmailUser(Model):
….

and make the same changes in forms, admin, views, URLs etc.

Alternatively, we need to add something to the definition of EmailUser that
says "I'm a User Model. Don't sync me unless I'm active":

class EmailUser(Model):
email = models.CharField()

class Meta:
swappable_using = 'AUTH_USER_MODEL'

And then we *still* need to do the "if settings.AUTH_USER_MODEL ==
auth.EmailUser" dance with views, forms, admin, etc.

However, this also means:

 1) We need to a bunch of internal plumbing in the app loader, Meta classes
and so on to determine whether or not a model should be synchronised, and
"hide" it in some way if it shouldn't be there. If you think this logic is
simple, I've got a bridge to sell you :-)

 2) We have to introduce a backwards incompatible change to pluggable
models. At present you don't have to pre-declare "my model is a User
model". All existing swappable User models in the wild need to be updated
to be "1.7 compliant".

 3) We introduce an implicit requirement for predeclaration. I need to know
ahead of time that my model is going to be a User model. In the case of
User, this probably isn't a huge impediment -- after all, I'm gonna know if
I'm a User model. But looking longer term: the infrastructure for swappable
Users isn't User specific. By design, swappable is a generic mechanism. You
*could* use it as a limited substitute for generic foreign keys (e.g., a
comments model pointing at a specific content type). But now you have to
pre-declare that my model "can be swapped for X" -- which means that as a
model designer, you need to know all the models that you *might* be swapped
out as. Consider the comments example -- I want people to be able to
comment on Users.  But I can't, because I haven't pre-declared User to be
"swappable as" a content object for comments. Predeclaration imposes a
restriction on one side of the swappable agreement that isn't always under
your control.

The alternative -- we put EmailUser in a separate app. That app contains
it's own forms, admin, views and URLs. No extra Meta attributes are
required. No special "if" logic is required internal to models, views,
forms, admin, etc. It's completely backwards compatible. It doesn't impose
any pre-declaration restrictions. And it requires no modifications to the
app loading internals. The only overhead - you have to declare an extra app
in INSTALLED_APPS -- which is hardly surprising to me, given that

Re: Is "transaction.atomic" in 1.6 supposed to work this way?

2013-09-19 Thread Curtis Maloney
I feel compelled to comment here...


On 20 September 2013 09:32, Richard Ward  wrote:
>
> I don't think that what I was trying to do (continue using a transaction
> after an insert failed) was too outlandish. I'm not demanding that Django
> should allow me to do it (tho that would be nice), but I can see other
> people trying to do it - especially if they are mostly used to MySQL, which
> allows it. It may not be correct, but it is (IMO) intuitive - why should a
> transaction have to be considered completely ruined just because an insert
> failed, when I expected that it may happen? (I don't need an answer to
> that, just explaining my/others thought process).
>

So you don't think that "one step of an atomic operation failing" is reason
to fail the atomic operation?  Doesn't sound intuitive to me.

Django is trying to enforce the _logical_ behavior.  The concept of an
"atomic" operation, even outside the context of DBMSs, is pretty much the
consistent throughout CS.

If you are expecting the step could fail, then you should wrap it in a
sub-transaction - the very reason they exist.  Again, you have an atomic
operation [the insert] that may fail, so you want to gracefully handle it.


> If Django must enforce PostgreSQL style behavior, then an exception at
> some point telling me off for my bad behavior would be useful. IIUC
> correctly I'd get that exception with Django+PostgreSQL but I don't get any
> with Django+MySQL.
>

I absolutely agree that Django should behave consistently in this, despite
the quirks of the DBMS chosen.

Either way I think the docs could be improved: "Wrapping atomic in a
> try/except block allows for natural handling of integrity errors" is not
> the same as "DatabaseErrors must be caught outside the atomic block". Also
> "If the block of code is successfully completed, the changes are committed
> to the database. If there is an exception, the changes are rolled back"
> implies to me that the transaction will be rolled back iff an exception
> causes the stack to unwind past transaction.atomic, but the intent is "If
> there is any DatabaseError [or any other exception raised inside a
> transaction.atomic(savepoint=False)], even if it is caught by user code,
> the changes are rolled back."
>

Always a fan of improving the clarity of the docs :)

[and I blame your expectations of transactions on MySQL... who seem to
delight in ignoring the sane behavior :)]

--
Curtis

-- 
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: AbstractUser to get more abstract

2013-09-19 Thread Aaron Merriam
Russell, my reply there isn't meant to be confrontational and yet 
re-reading it I can see it being easily interpreted as such.  My intention 
is to ask whether there is something about swappable that I am missing.  It 
seems to work the way I stated above but your statement seems to imply that 
their is some missing piece that needs extra mechanisms surrounding model 
loading.  So, am I missing something?

Marc,

Thank you for your reply, and I respect the difference of opinion.  If I 
understand you correctly, you feel that this belongs in a separate contrib 
application because to refactor auth to be generic enough to handle both 
the built in User and EmailUser will involve too much magic.

I'm curious if you've taken a look at the code behind django-authtools.  I 
feel that it is a good example of what some of the changes to auth.forms or 
auth.views might look like, and that there is less 'magic' involved than 
you might think.

-- 
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: AbstractUser to get more abstract

2013-09-19 Thread Aaron Merriam

>
> However, if auth.User (or any other model for that matter) is set as 
> AUTH_USER_MODEL, nothing prevents EmailUser from being installed.
>

The current behavior of swappable is that if two models are swappable on 
the same settings value, only the one designated by the settings value will 
be installed.  So, currently, in django if I add the concrete model 
EmailUser to django.contrib.auth.models with swappable='AUTH_USER_MODEL', 
and do nothing else, this model is not installed.  If I reset my database 
and set my settings.AUTH_USER_MODEL to 'auth.EmailUser', then EmailUser 
will be installed and 'django.contrib.auth.User' will not be installed. 
 This is the current behavior of swappable based on my testing.

So there is no need for new internal mechanisms to control whether a model 
is installed or not, as that mechanism already exists within the current 
behavior of swappable.

On Thursday, September 19, 2013 5:09:10 PM UTC-6, Russell Keith-Magee wrote:
>
>
> On Thu, Sep 19, 2013 at 9:41 PM, Marc Tamlyn 
> > wrote:
>
>> The problem you've got here is how it knows to *not* install EmailUser. 
>> If it's a model defined in d.c.auth.models, it will get installed, 
>> irrespective of whether it is AUTH_USER_MODEL or not. This is where we have 
>> to special case the new model somehow so it is only installed if we are 
>> using it. This is far from straightforwards! By putting it in a separate 
>> app which is not installed by default, then we solve this issue trivially 
>> without more magic.
>>
> Elaborating on this point: it's not just the models, either -- it's views, 
> admin registrations, forms, and potentially URLs.
>
> There's also a matter of consistency. We have to ship a default User 
> model. We've set up a system that encourages people to develop their own 
> user models. If you develop a third party User model, you need to do two 
> things -- register the app, and declare AUTH_USER_MODELS. It seems 
> completely logical to me that an EmailUser app that we're shipping as part 
> of Django should be expected to do the same. 
>
> If nothing else, it means we can point at contrib.email_auth and say 
> "that's how you do it!".
>  
>
>>  On 19 Sep 2013 05:27, "Luke Sneeringer" > 
>> wrote:
>>
>>> Bah, I should have planned my e-mailing better. Sorry for sending a 
>>> third in such rapid succession.
>>>
>>> I just did some checking into my assumptions (quoted below) on this. It 
>>> turns out that Django core already does some of what I suggest: in 
>>> particular, it doesn't install the User model if django.contrib.auth is 
>>> installed but another AUTH_USER_MODEL is invoked, based on a Meta option 
>>> called "swappable" (see [here][1] and [here][2]).
>>>
>>> Am I misunderstanding? It seems like we are already pretty well down my 
>>> proposed path.
>>>
>>  
> You've correctly identified the current behaviour, but you're 
> extrapolating in a direction that isn't possible (at least, not with the 
> current infrastructure).
>
> You're correct in saying auth.User won't be installed if another model is 
> set as AUTH_USER_MODEL. However, if auth.User (or any other model for that 
> matter) is set as AUTH_USER_MODEL, nothing prevents EmailUser from being 
> installed. This is because there's no artefact on the "plugged" model that 
> says "I'm a candidate for being a pluggable User. *Any* model *could* be 
> installed as a User -- you don't have to predeclare "I'm a candidate for 
> being a User". Therefore, we have no metadata basis on which to say "Oh, 
> I'm not being used as a pluggable model, don't install me." 
>
> We *could* add this metadata, but again -- we'd be adding metadata and a 
> bunch of complex internals as a workaround for asking users to put an app 
> in INSTALLED_APPS, and set AUTH_USER_MODEL. For my money, the cost vastly 
> exceeds the questionable benefit.
>
> 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: Is "transaction.atomic" in 1.6 supposed to work this way?

2013-09-19 Thread Richard Ward
Thanks for the explanations. I wasn't aware that particular difference 
between PostgreSQL and MySQL (I've not done much with PostgreSQL).

Perhaps I could have been clearer in my original post: Ansii is right about 
silence... when I call my_func the transaction.atomic decorator rolls back 
the transaction and then returns, rather than (eg) rolling back the 
transaction and then raising/propagating an exception (when using MySQL).

I don't think that what I was trying to do (continue using a transaction 
after an insert failed) was too outlandish. I'm not demanding that Django 
should allow me to do it (tho that would be nice), but I can see other 
people trying to do it - especially if they are mostly used to MySQL, which 
allows it. It may not be correct, but it is (IMO) intuitive - why should a 
transaction have to be considered completely ruined just because an insert 
failed, when I expected that it may happen? (I don't need an answer to 
that, just explaining my/others thought process). If Django must enforce 
PostgreSQL style behavior, then an exception at some point telling me off 
for my bad behavior would be useful. IIUC correctly I'd get that exception 
with Django+PostgreSQL but I don't get any with Django+MySQL.

Either way I think the docs could be improved: "Wrapping atomic in a 
try/except block allows for natural handling of integrity errors" is not 
the same as "DatabaseErrors must be caught outside the atomic block". Also 
"If the block of code is successfully completed, the changes are committed 
to the database. If there is an exception, the changes are rolled back" 
implies to me that the transaction will be rolled back iff an exception 
causes the stack to unwind past transaction.atomic, but the intent is "If 
there is any DatabaseError [or any other exception raised inside a 
transaction.atomic(savepoint=False)], even if it is caught by user code, 
the changes are rolled back."

Thanks,
Richard

-- 
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.


FormSetView and ModelFormSetView

2013-09-19 Thread Gert Steyn
Hi All

What are the chances of adding FormSetView and ModelFormSetView classes to 
the standard set of generic views?

I know its not to much effort to do it yourself, but it will help a lot 
with consistency across different Django applications. Currently there are 
multiple 3rd party versions of this available, all of them are slightly 
different and none of them seem to stand out as an obvious choice.

Most Django developers don't have the same skills as the core developers, 
please provide your guidance with this.

Regards
Gert Steyn

-- 
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: AbstractUser to get more abstract

2013-09-19 Thread Russell Keith-Magee
On Thu, Sep 19, 2013 at 9:41 PM, Marc Tamlyn  wrote:

> The problem you've got here is how it knows to *not* install EmailUser. If
> it's a model defined in d.c.auth.models, it will get installed,
> irrespective of whether it is AUTH_USER_MODEL or not. This is where we have
> to special case the new model somehow so it is only installed if we are
> using it. This is far from straightforwards! By putting it in a separate
> app which is not installed by default, then we solve this issue trivially
> without more magic.
>
Elaborating on this point: it's not just the models, either -- it's views,
admin registrations, forms, and potentially URLs.

There's also a matter of consistency. We have to ship a default User model.
We've set up a system that encourages people to develop their own user
models. If you develop a third party User model, you need to do two things
-- register the app, and declare AUTH_USER_MODELS. It seems completely
logical to me that an EmailUser app that we're shipping as part of Django
should be expected to do the same.

If nothing else, it means we can point at contrib.email_auth and say
"that's how you do it!".


> On 19 Sep 2013 05:27, "Luke Sneeringer"  wrote:
>
>> Bah, I should have planned my e-mailing better. Sorry for sending a third
>> in such rapid succession.
>>
>> I just did some checking into my assumptions (quoted below) on this. It
>> turns out that Django core already does some of what I suggest: in
>> particular, it doesn't install the User model if django.contrib.auth is
>> installed but another AUTH_USER_MODEL is invoked, based on a Meta option
>> called "swappable" (see [here][1] and [here][2]).
>>
>> Am I misunderstanding? It seems like we are already pretty well down my
>> proposed path.
>>
>
You've correctly identified the current behaviour, but you're extrapolating
in a direction that isn't possible (at least, not with the current
infrastructure).

You're correct in saying auth.User won't be installed if another model is
set as AUTH_USER_MODEL. However, if auth.User (or any other model for that
matter) is set as AUTH_USER_MODEL, nothing prevents EmailUser from being
installed. This is because there's no artefact on the "plugged" model that
says "I'm a candidate for being a pluggable User. *Any* model *could* be
installed as a User -- you don't have to predeclare "I'm a candidate for
being a User". Therefore, we have no metadata basis on which to say "Oh,
I'm not being used as a pluggable model, don't install me."

We *could* add this metadata, but again -- we'd be adding metadata and a
bunch of complex internals as a workaround for asking users to put an app
in INSTALLED_APPS, and set AUTH_USER_MODEL. For my money, the cost vastly
exceeds the questionable benefit.

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: Is "transaction.atomic" in 1.6 supposed to work this way?

2013-09-19 Thread Marc Tamlyn
This is what nested atomics do. In effect savepoints are just creating
another transaction inside the existing one (if my memories of Aymeric's
talk are correct).
On 19 Sep 2013 21:48, "Hannu Krosing"  wrote:

> On 09/19/2013 04:52 PM, Anssi Kääriäinen wrote:
> > After some investigation it turns out that this isn't about
> > IntegrityErrors at all. Instead the problem is that inside @atomic block
> > Model.save() uses @atomic(savepoint=False) internally. And
> > @atomic(savepoint=False) forces the outer atomic block to roll back if
> > errors
> > happen.
> >
> > If I recall correctly there is transaction.set_rollback(False) which
> > can be used to remove forced rollback.
> >
> > In general, there are three ways to respond to errors inside
> transactions:
> >   1. allow usage of the TX (MySQL, SQLite etc), allow user to decide
> > commit/rollback
> >   2. forbid usage of the TX (PostgreSQL), force it to be rolled back.
> Actually you can use subtransactions in postgresql if you want to manage
> failed queries yourself
>
> BEGIN;
> ;
> SAVEPOINT sp1;
> ;
> ROLLBACK TO SAVEPOINT sp1;
> ;
> COMMIT:
>
> this will commit ; and ; while rolling
> back the failed ;
> you can read more on this in
> http://www.postgresql.org/docs/9.1/static/sql-rollback-to.html
>
> Cheers
>
> --
> Hannu Krosing
> PostgreSQL Consultant
> Performance, Scalability and High Availability
> 2ndQuadrant Nordic OÜ
>
> --
> 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.
>

-- 
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: AbstractUser to get more abstract

2013-09-19 Thread Aaron Merriam

>
> I'm not sure it was the intention in the design of swappable.


I don't know the intended use of swappable, but this seems like exactly the 
type of thing that a 'swappable' model should be able to do.
 

> However, we still have the issue of different forms, urls, views etc all 
> based off the changed model.


Marc, can you explain why you feel we need different forms, urls, views, 
etc.  The only forms that would not work appropriately with EmailUser would 
be UserCreation and UserChange forms, both of which can be modified to work 
correctly with EmailUser or other custom user models .  The views in 
d.c.auth all work correctly with the proposed EmailUser and most custom 
user models.   The urls in d.c.auth are not concerned with the username 
field so I don't see an issue there.  The ModelAdmin class in d.c.auth can 
also be modified to respect USERNAME_FIELD and work with EmailUser.

 Making these change automatically would be horrible.


Can you explain why this would be horrible.  The changes proposed in the 
'authtools' style approach make d.c.auth more abstract in ways that make 
using a custom user model easier, while still keeping d.c.auth functioning 
the same when the user has not been swapped out.  To me, this is a step 
towards making custom user models easier and thus, a bonus.

-- 
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: Is "transaction.atomic" in 1.6 supposed to work this way?

2013-09-19 Thread Anssi Kääriäinen
I think there is a case of talking past each other going on here.

On Thursday, September 19, 2013 11:33:00 PM UTC+3, Aymeric Augustin wrote:

> Nonetheless, in my opinion, the root cause of the perceived misbehavior is 
> that user code (not Django code) catches IntegrityError inside an atomic 
> block, preventing __exit__ from detecting that a database exception has 
> happened.
>

This is *not* what is causing the problem in this case. The problem here is 
that if you do this inside atomic block:
try:
obj.save()
except IntegrityError:
pass

you can continue to use the connection, but after the outer atomic block is 
exited the transaction is rolled back. The combination of allowing usage of 
connection but rolling it back is the foot gun I have been talking about..

Note that this isn't about DatabaseErrors. *Any* exception bubbling from 
atomic(savepoint=False) block will cause this issue. 

DatabaseErrors must be caught outside the atomic block, as documented. 
> Otherwise transaction.atomic will fail in various interesting and database 
> dependent ways. It's harder to diagnose on databases that don't fully 
> enforce transactional behavior ie. all but PostgreSQL. 
>

Again, this is not about DatabaseErrors at all. DatabaseErrors will mark 
"errors_occured". Errors bubbling from atomic(savepoint=False) will mark 
needs_rollback.

> If I recall correctly there is transaction.set_rollback(False) which can 
> be used to remove forced rollback. 
>
> This is a private API for a reason. Someone deeply familiar with the 
> implementation details of transaction management in Django 1.6 and with an 
> esoteric transaction management system might find this private API useful 
> but it's very obviously not the answer to the original question. Please, 
> everyone, don't do this. 
>

This is documented API... 

>
> > In general, there are three ways to respond to errors inside 
> transactions: 
> >   1. allow usage of the TX (MySQL, SQLite etc), allow user to decide 
> commit/rollback 
> >   2. forbid usage of the TX (PostgreSQL), force it to be rolled back. 
> >   3. allow usage of the TX, but force rollback (Django) 
>
> You're comparing apples to oranges here. If Django is backed by 
> PostgreSQL, how can Django allow using the transaction if PostgreSQL 
> forbids it?
>

Yes, you get #1 on PostgreSQL. But if you are using something other than 
PostgreSQL + Django you will get #3 I don't want that, and it seems you 
don't either. Yet this is what is provided. We can change this to #1 on all 
databases.

>
> > To me it seems explicit error when using connection in "forced to 
> rollback" state is better than allowing saving more data, then silently 
> rolling back the transaction. As you said this is useless. 
>
> Django doesn't allow saving more data in a broken transaction. If a 
> DatabaseError occurs, execution jumps straight to __exit__ which sees an 
> exception and triggers a rollback.


No, this doesn't happen. When using savepoint=False, you get needs_rollback 
flagged for the connection. The connection is still usable. Anything you do 
will be silently rolled back.
 

> Of course, you can disregard the documentation and break this expectation 
> by catching the exception. But there's no way around this in Python. A 
> context manager cannot prevent catching exceptions. 
>

> > To be clear, this is about exceptions (database or other) in 
> atomic(savepoint=False) blocks, not about caught IntegrityErrors - Django 
> will allow you to continue and commit the TX in the caught IntgerityError 
> case assuming the DB allows that. 
>
> I designed this part of Django to forbid continuing and committing the 
> transaction. The reason is cross-database compatibility. I had to enforce 
> the behavior of the most restrictive database, which is also the most 
> correct, namely PostgreSQL. 
>

>
> Sure, you can do virtually anything by disregarding the docs and abusing 
> private APIs, but I wouldn't describe that as "Django will allow you to…" 
>
> If I still haven't convinced you that I know what I'm talking about, 
> here's an example with the exact same problem but without 
> atomic(savepoint=False). 
>
> with transaction.atomic: 
> try: 
> do_stuff_in_db()# raises DatabaseError 
> except DatabaseError: 
> pass 
> do_more_stuff_in_db()# works only on lax databases eg. MySQL 
>   
>
In this example, it's totally possible that the transaction.atomic block 
> will end up with a rollback even if do_more_stuff_in_db() succeed. Why? 
> Because the commit is likely to fail if a statement failed during the 
> transaction, and that will result in an implicit rollback!


So, why allow using the connection then?

But this case isn't as bad as:
with transaction.atomic():
try:
obj.save()
except IntegrityError:
pass
another_obj.save()

Here, another_obj.save() will succeed. But the transaction will be *always* 
rolled back, silently. I don

Re: AbstractUser to get more abstract

2013-09-19 Thread Marc Tamlyn
I think you'll find that at least the Authentication form is wrong as well
- at least in the sense that it calls the field username and it doesn't
validate as an email. Once we get better at html5 fields as well, it won't
be the correct input type.

Admin registration, the management commands, and the manager would also all
need to be different.

I'd rather see auth get less magical and abstract, not more. I already find
the existence of USERNAME_FIELD rather ugly. This whole approach is very
implicit.
On 19 Sep 2013 22:13, "Aaron Merriam"  wrote:

> I'm not sure it was the intention in the design of swappable.
>
>
> I don't know the intended use of swappable, but this seems like exactly
> the type of thing that a 'swappable' model should be able to do.
>
>
>> However, we still have the issue of different forms, urls, views etc all
>> based off the changed model.
>
>
> Marc, can you explain why you feel we need different forms, urls, views,
> etc.  The only forms that would not work appropriately with EmailUser would
> be UserCreation and UserChange forms, both of which can be modified to work
> correctly with EmailUser or other custom user models .  The views in
> d.c.auth all work correctly with the proposed EmailUser and most custom
> user models.   The urls in d.c.auth are not concerned with the username
> field so I don't see an issue there.  The ModelAdmin class in d.c.auth can
> also be modified to respect USERNAME_FIELD and work with EmailUser.
>
>  Making these change automatically would be horrible.
>
>
> Can you explain why this would be horrible.  The changes proposed in the
> 'authtools' style approach make d.c.auth more abstract in ways that make
> using a custom user model easier, while still keeping d.c.auth functioning
> the same when the user has not been swapped out.  To me, this is a step
> towards making custom user models easier and thus, a bonus.
>
> --
> 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.
>

-- 
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: AbstractUser to get more abstract

2013-09-19 Thread Marc Tamlyn
Whilst this works, I'm not sure it was the intention in the design of
swappable. Clever though and it would allow the single-app approach.

However, we still have the issue of different forms, urls, views etc all
based off the changed model. Making these change automatically would be
horrible.

I still think a second app is a better plan.

If we are going to do this I think it is worth revisiting class basing the
with views. A decent chunk of work was done on this at some point, and
there are other implementations in the wild. It makes having the two
separate versions much simpler in terms of swapping out form classes etc.
One if the major issues was the lack of quality tests for some of the views.

Marc
On 19 Sep 2013 19:25, "Luke Sneeringer"  wrote:

>
>
> On Sep 19, 2013, at 11:33 AM, Aaron Merriam 
> wrote:
>
> I and my colleague (gavinw...@gmail.com) have made some edits to the wiki
> to clarify the purpose of authtools, and more accurately explain what the
> 'authtools' approach would look like.  If you previously have examined
> 'option 2', I would ask that you go and reread that section in the wiki.
>
> https://code.djangoproject.com/wiki/ContribEmailAuth
>
> The problem you've got here is how it knows to *not* install EmailUser. If
>> it's a model defined in d.c.auth.models, it will get installed,
>> irrespective of whether it is AUTH_USER_MODEL or not.
>
>
> I wanted to verify this so I ran some basic tests and turns out that
> swappable already does the correct thing.  If two models have the same
> swappable value, only the one referenced in settings will be
> used/installed.  This makes it practical to include a second user model in
> d.c.auth.models without any changes to model loading or the swappable
> mechanism.
>
>
> Precisely. I think I originally stated my thinking quite poorly, which
> caused a great deal of confusion (sorry about that). I didn't know that
> swappable existed, and was proposing a mechanism that acted similarly to
> that. But, since swappable *does* exist, it seems like it's the best
> mechanism. I had originally made the same error that Marc made.
>
> --
> 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.
>

-- 
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: Is "transaction.atomic" in 1.6 supposed to work this way?

2013-09-19 Thread Hannu Krosing
On 09/19/2013 04:52 PM, Anssi Kääriäinen wrote:
> After some investigation it turns out that this isn't about
> IntegrityErrors at all. Instead the problem is that inside @atomic block
> Model.save() uses @atomic(savepoint=False) internally. And
> @atomic(savepoint=False) forces the outer atomic block to roll back if
> errors
> happen.
>
> If I recall correctly there is transaction.set_rollback(False) which
> can be used to remove forced rollback.
>
> In general, there are three ways to respond to errors inside transactions:
>   1. allow usage of the TX (MySQL, SQLite etc), allow user to decide
> commit/rollback
>   2. forbid usage of the TX (PostgreSQL), force it to be rolled back.
Actually you can use subtransactions in postgresql if you want to manage
failed queries yourself

BEGIN;
;
SAVEPOINT sp1;
;
ROLLBACK TO SAVEPOINT sp1;
;
COMMIT:

this will commit ; and ; while rolling
back the failed ;
you can read more on this in
http://www.postgresql.org/docs/9.1/static/sql-rollback-to.html

Cheers

-- 
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic OÜ

-- 
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: [GSoC] Composite fields once again

2013-09-19 Thread Duke Nukem 3D is coming!
This sounds exciting. I can't wait for this to land in master. Thanks, 
Michal!

On Sunday, June 2, 2013 10:22:36 AM UTC-4, Michal Petrucha wrote:
>
> Hello everyone, 
>
> I'm happy to announce that throughout the summer I'll be officially 
> working on composite fields under the supervision of Andrew Godwin as 
> part of this year's GSoC. 
>
> I'll use this thread to post all my weekly status updates regarding 
> this project. This email gives a brief overview of the current status 
> and a rough outline of my plans, as well as a few questions I'd like 
> to raise with the community. 
>
> So, as far as currently available code goes, there's a github repo 
> containing my work from the GSoC two years ago, which is outdated and 
> incomplete, but still contains a large amount of working code, most of 
> which should be possible to adapt to the current state of Django. 
>
> The work can be outlined roughly in the following four steps: 
>
> 1) create a solid implementation of virtual fields 
> 2) refactor ForeignKey/OneToOneField to become virtual fields 
> 3) implement CompositeField 
> 4) make CompositeField work with as many additional parts of Django as 
>possible, including GenericForeignKey and inspectdb 
> 5) possibly implement a mechanism to modify the primary key of a model 
>instance 
>
> All of the above steps are explained in more detail in the proposal I 
> submitted for the GSoC, available as a gist [1]. 
>
> Now, for the questions I wanted to raise. 
>
>
> ForeignKey customization API 
>  
>
> This one is mostly about the extent to which we want the internal 
> implementation of ForeignKey to affect the public API. To keep things 
> backwards compatible, attributes such as db_column or db_index will be 
> just passed to the auto-generated auxiliary field. 
>
> The question is, do we want to make it possible to specify a custom 
> auxiliary field to a ForeignKey instead of always creating one? 
>
> A related question, how should it be possible to customize the 
> database columns of composite ForeignKeys? Just make ForeignKey accept 
> a tuple instead of a string? Or just require the user to create the 
> fields (along with a CompositeField) by hand and pass that as the 
> auxiliary field? Any nicer possibility? 
>
> Either option is rather simple to implement, I just don't really have 
> a strong opinion, although I find both a little bit unpleasant. 
>
>
> GenericForeignKey and nontrivial field types 
>  
>
> As I've indicated in my proposal, just casting any value to a string 
> and then performing a reversible transformation on such strings may 
> work well enough for string and integer database columns, not so much 
> for things like dates, timestamps IP addresses or other similar types. 
>
> Any ideas on how to make this work? Should I try to extend the backend 
> API to include explicit casts for each nontrivial column type to a 
> string representation equal to the one used by Python? Or should I 
> just document this as unsupported? 
>
>
> Updatable primary keys 
> -- 
>
> This feature is not directly related to the main objective of this 
> project, which is to implement composite fields. It is just easier for 
> people to get into a situation where this might be required when using 
> composite fields. 
>
> Full support for this feature would require quite massive changes to 
> the collectors used cascading deletes -- they'd have to be generalized 
> to support cascading updates as well. This would introduce a lot of 
> complexity, obviously. 
>
> Jeremy Tillman voiced his opinion against this feature in a comment to 
> my proposal. He gives valid arguments -- an update of a primary key 
> would be an expensive operation with potential to kill the performance 
> of any apps doing it. However, the argument that it is easily done 
> with a Model.objects.filter(...).update(...) call is not entirely true 
> as a full implementation of this feature would also cascade. Moreover, 
> it would make this possible in the admin without much hassle. 
>
> So, seeing that there is at least one voice against this feature, I 
> think it's better to let the community decide if we want it at all. 
>
> Either way, it's highly unlikely I'd be able to deliver this feature 
> as part of this GSoC, the best I can promise is an initial proof of 
> concept implementation. 
>
>
> Michal 
>
>
> [1] https://gist.github.com/konk/5408673 
>

-- 
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: Is "transaction.atomic" in 1.6 supposed to work this way?

2013-09-19 Thread Aymeric Augustin
On 19 sept. 2013, at 16:52, Anssi Kääriäinen  wrote:

> After some investigation it turns out that this isn't about IntegrityErrors 
> at all. Instead the problem is that inside @atomic block
> Model.save() uses @atomic(savepoint=False) internally. And 
> @atomic(savepoint=False) forces the outer atomic block to roll back if errors
> happen.

Yes, that's the actual code path — which I introduced after you requested it 
for performance reasons, in spite of my strong concerns that it could introduce 
weird edge cases ;-)

Nonetheless, in my opinion, the root cause of the perceived misbehavior is that 
user code (not Django code) catches IntegrityError inside an atomic block, 
preventing __exit__ from detecting that a database exception has happened.

DatabaseErrors must be caught outside the atomic block, as documented. 
Otherwise transaction.atomic will fail in various interesting and database 
dependent ways. It's harder to diagnose on databases that don't fully enforce 
transactional behavior ie. all but PostgreSQL.

> If I recall correctly there is transaction.set_rollback(False) which can be 
> used to remove forced rollback.

This is a private API for a reason. Someone deeply familiar with the 
implementation details of transaction management in Django 1.6 and with an 
esoteric transaction management system might find this private API useful but 
it's very obviously not the answer to the original question. Please, everyone, 
don't do this.

> In general, there are three ways to respond to errors inside transactions:
>   1. allow usage of the TX (MySQL, SQLite etc), allow user to decide 
> commit/rollback
>   2. forbid usage of the TX (PostgreSQL), force it to be rolled back.
>   3. allow usage of the TX, but force rollback (Django)

You're comparing apples to oranges here. If Django is backed by PostgreSQL, how 
can Django allow using the transaction if PostgreSQL forbids it?

> To me it seems explicit error when using connection in "forced to rollback" 
> state is better than allowing saving more data, then silently rolling back 
> the transaction. As you said this is useless.

Django doesn't allow saving more data in a broken transaction. If a 
DatabaseError occurs, execution jumps straight to __exit__ which sees an 
exception and triggers a rollback.

Of course, you can disregard the documentation and break this expectation by 
catching the exception. But there's no way around this in Python. A context 
manager cannot prevent catching exceptions.

> To be clear, this is about exceptions (database or other) in 
> atomic(savepoint=False) blocks, not about caught IntegrityErrors - Django 
> will allow you to continue and commit the TX in the caught IntgerityError 
> case assuming the DB allows that.

I designed this part of Django to forbid continuing and committing the 
transaction. The reason is cross-database compatibility. I had to enforce the 
behavior of the most restrictive database, which is also the most correct, 
namely PostgreSQL.

Sure, you can do virtually anything by disregarding the docs and abusing 
private APIs, but I wouldn't describe that as "Django will allow you to…"

If I still haven't convinced you that I know what I'm talking about, here's an 
example with the exact same problem but without atomic(savepoint=False).

with transaction.atomic:
try:
do_stuff_in_db()# raises DatabaseError
except DatabaseError:
pass
do_more_stuff_in_db()   # works only on lax databases eg. MySQL

In this example, it's totally possible that the transaction.atomic block will 
end up with a rollback even if do_more_stuff_in_db() succeed. Why? Because the 
commit is likely to fail if a statement failed during the transaction, and that 
will result in an implicit rollback!

At the highest level, my design is based on avoiding implicit rollbacks because 
explicit is better than implicit. But you can still force one if you try hard 
enough.

-- 
Aymeric.

-- 
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: Default session data serializer doesn't support extended data types

2013-09-19 Thread Tim Graham
Hi Davide,

Did you take a look at the design decisions as described in the ticket and 
pull request? We made these decisions in order to push the community toward 
developing more secure apps and the transition isn't expected to be 
painless. We had several core developers review the patch and discuss the 
decisions. It seems like you are asking us to reconsider, but you haven't 
presented anything we haven't discussed. Would you like me to address 
anything specific?

Tim 

On Thursday, September 19, 2013 10:46:44 AM UTC-4, Davide Rizzo wrote:
>
> #20922  introduced the 
> option to choose a custom session data serializer. The default option is to 
> use the new JSONSerializer starting from 1.6, since using pickle would lead 
> to a remote code execution vulnerability when session data is stored in 
> cookies.
>
> While this can be considered a sensible security choice, it becomes 
> inconvenient as the JSON encoder used by JSONSerializer is not the same 
> used elsewhere in Django, as it only support basic data types: string, 
> int/floats, booleans, nested dicts and lists, None.
>
> The inconvenience is breaking compatibility with all third party apps that 
> rely on storing extended data types (such as those supported by 
> DjangoJSONEncoder) with the default settings. Properly serializing datetime 
> (possibly tz-aware) can be hard, and changing the default puts the burden 
> on third party apps coders.
>
> They would have the option to either add two complexity layers (properly 
> serializing/deserializing datetime objects, and not breaking compatibility 
> with the previous versions of the same app), or to break compatibility with 
> Django default settings.
>
> As an example of commonly used data types that can't be stored anymore 
> with default settings:
>
>- datetime, timedelta objects (supported by DjangoJSONEncoder)
>- decimal objects (supported by DjangoJSONEncoder)
>- arbitrary binary strings
>- Geometry objects
>
> I think the option of reverting the default to pickle should be also 
> considered.
>
> [[I originally posted this as 
> #21124, 
> where it was closed as not a bug. What follows is the response I got, for 
> reference:
>
> by timo (core developer)
>
> Thanks for your thoughts. I think most of the points you've raised were 
> discussed during the implementation of this, either on the ticket 
> (#20922) 
> or on the linked pull request (or 
> the documentation 
> itself).
>  
> Could you please take a look at the discussion there if you haven't? If 
> after reading that you still have disagreements, please raise the issue on 
> django-developers rather than this ticket tracker. Thanks!
>
> Suggestions for documentations edits or additions would also be welcome.
>
> p.s. To address one of your points, one of the decisions was indeed to put 
> the burden on third party app coders to serialize session data as simple 
> data types like strings which would be compatible with JSON. We made this 
> change to contrib.messages for example.
> ]]
>

-- 
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: AbstractUser to get more abstract

2013-09-19 Thread Luke Sneeringer
But Django already has the mechanism to know not to install the model. It's the 
"swappable" Meta option. That is what causes User not to be installed if it is 
not used (contra my initial statement yesterday; I was flat wrong).

Right now, User has a meta option:

swappable = 'AUTH_USER_MODEL'

...that does exactly what we are describing. It seems EmailUser (or whatever we 
call it) should just do this also.

> On Sep 19, 2013, at 7:41 AM, Marc Tamlyn  wrote:
> 
> The problem you've got here is how it knows to *not* install EmailUser. If 
> it's a model defined in d.c.auth.models, it will get installed, irrespective 
> of whether it is AUTH_USER_MODEL or not. This is where we have to special 
> case the new model somehow so it is only installed if we are using it. This 
> is far from straightforwards! By putting it in a separate app which is not 
> installed by default, then we solve this issue trivially without more magic.
> 
>> On 19 Sep 2013 05:27, "Luke Sneeringer"  wrote:
>> Bah, I should have planned my e-mailing better. Sorry for sending a third in 
>> such rapid succession.
>> 
>> I just did some checking into my assumptions (quoted below) on this. It 
>> turns out that Django core already does some of what I suggest: in 
>> particular, it doesn't install the User model if django.contrib.auth is 
>> installed but another AUTH_USER_MODEL is invoked, based on a Meta option 
>> called "swappable" (see [here][1] and [here][2]).
>> 
>> Am I misunderstanding? It seems like we are already pretty well down my 
>> proposed path.
>> 
>> Best Regards,
>> Luke
>> 
>>   [1]: 
>> https://github.com/django/django/blob/b2b763448f726ee952743596e9a34fcb154bdb12/django/contrib/auth/models.py#L406-L414
>>   [2]: 
>> https://github.com/django/django/blob/master/django/db/models/loading.py#L296
>> 
>> 
>>> On Sep 18, 2013, at 10:03 PM, Luke Sneeringer  wrote:
>>> 
 On Wednesday, September 18, 2013 6:39:13 PM UTC-6, Russell Keith-Magee 
 wrote:
 
> On Thu, Sep 19, 2013 at 3:39 AM, Luke Sneeringer  
> wrote:
> I added the authtools approach to the wiki for completion, although I 
> believe it to be an inferior approach.
> 
> One thing I dislike is having a separate app (e.g. d.c.auth_email) that 
> has to be installed separately. That feels pretty impure to me. I'm doing 
> a thought exercise about potential solutions, though, and not exactly 
> coming up aces.
> 
> The best solution I can currently think of is to have User and EmailUser 
> which are both models that live in django.contrib.auth. Then, we would 
> have to add code to our model detection that says that *if* a model is a 
> subclass of AbstractBaseUser, include it if and only if it is the 
> AUTH_USER_MODEL.
> 
> I can't decide if that solution is better or worse than the disease. It 
> makes for a much more attractive set of steps to follow for people who 
> want to use it, though -- basically, just set AUTH_USER_MODEL to 
> 'auth.EmailUser', and done.
 
 My opinion - it's worse than the disease. 
 
 Option 1 involves a clean auth app that just contains a stub user, and a 
 clean extension app providing an alternative user. You install the 
 extension app, and say you want to use it.
 
 Option 2 makes a special case of *one particular* extension user, and 
 makes all the internals of models, forms, views, etc embedded inside an if 
 statement.
>>> 
>>> I think where I part from you is on the "one particular" aspect of the 
>>> discussion. I actually see the "option 2" approach as having very clean 
>>> side effects for some other use cases.
>>> 
>>> Allow me to explain:
>>> There is, as you have pointed out, a lot more to auth than just the User 
>>> model. There are forms and the like, which you mentioned, but there are 
>>> also other aspects: the permissions models and functionality, for instance. 
>>> Subclasses of AbstractBaseUser which also subclass PermissionsMixin still 
>>> get the permissions things out of the box, and that is a documented use 
>>> case in the official documentation.
>>> 
>>> So, right now (if I understand correctly), if you install the auth app, you 
>>> get the Group and Permission models (and supporting code, natch), as well 
>>> as the User model. If you decide to set a different User model using 
>>> AUTH_USER_MODEL, but you keep auth installed for these other aspects 
>>> (Group, Permission, etc.) then Django installs an auth_user table that 
>>> isn't used (note to self: verify this).
>>> 
>>> If you set AUTH_USER_MODEL to something other than auth.User, you are 
>>> making a statement that you do not want the stock model. This is true 
>>> whether you change it to the upcoming, included "EmailUser" model, or to 
>>> something else entirely.
>>> 
>>> So, I don't think that option 2 "makes a special case of one particular 
>>> extension user". If anything, I assert it would do the opposite:

Re: AbstractUser to get more abstract

2013-09-19 Thread Luke Sneeringer


> On Sep 19, 2013, at 11:33 AM, Aaron Merriam  wrote:
> 
> I and my colleague (gavinw...@gmail.com) have made some edits to the wiki to 
> clarify the purpose of authtools, and more accurately explain what the 
> 'authtools' approach would look like.  If you previously have examined 
> 'option 2', I would ask that you go and reread that section in the wiki.
> 
> https://code.djangoproject.com/wiki/ContribEmailAuth
> 
>> The problem you've got here is how it knows to *not* install EmailUser. If 
>> it's a model defined in d.c.auth.models, it will get installed, irrespective 
>> of whether it is AUTH_USER_MODEL or not.
> 
> I wanted to verify this so I ran some basic tests and turns out that 
> swappable already does the correct thing.  If two models have the same 
> swappable value, only the one referenced in settings will be used/installed.  
> This makes it practical to include a second user model in d.c.auth.models 
> without any changes to model loading or the swappable mechanism.

Precisely. I think I originally stated my thinking quite poorly, which caused a 
great deal of confusion (sorry about that). I didn't know that swappable 
existed, and was proposing a mechanism that acted similarly to that. But, since 
swappable *does* exist, it seems like it's the best mechanism. I had originally 
made the same error that Marc made.

-- 
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: AbstractUser to get more abstract

2013-09-19 Thread Bruno Ribeiro da Silva
Hi, I develop applications in Django and I started to read the developer
mailing list and got interested by this topic about MailUser problem and I
want to add my 50 cents, sorry if I'm taking a completely wrong approach,
since I never developed the internals of Django.

But what if we just add a AUTH_USERNAME_FIELD to django settings and with
this modify the behaviour of User model?

I did some fast coding to show you what I'm thinking:
https://github.com/loop0/django/compare/email_user

Isn't this a simpler approach? Does it have too much implications? I tested
changing AUTH_USERNAME_FIELD on a sample project settings to email and it
worked login to admin.

I like Jorge's idea to separate permissions into it's own app too.

Sorry if I wasn't supposed to post to this mailing group.

Regars,
Bruno


On Thu, Sep 19, 2013 at 12:45 PM, Jorge Cardoso Leitão wrote:

> Hi all.
>
> I summarise the options with some of the issues raised, and I add my own
> concern.
>
>
> One option presented here is to have both models in d.c.auth. As pointed
> out by Russell and others, this causes the problem on how to define the
> models such that they are not installed if they are not being
> the AUTH_USER_MODEL. Assuming that this problem could be solved, the
> user's procedure would be:
>
> To use User (+ Permissions):
> install django.contrib.auth
>
> To use MailUser (+ Permissions):
> install django.contrib.auth
> set AUTH_USER_MODEL to MailUser
>
> The second option presented here is to add a separate app (e.g.
> d.c.mailuser), which would require some imports from django.contrib.auth
> (forms, views). The user's procedure would be:
>
> To use User (+ Permissions):
> install django.contrib.auth
>
> To use MailUser (+ Permissions):
> install django.contrib.auth
> install django.contrib.mailauth
> set AUTH_USER_MODEL to MailUser
>
> Is this correct or I'm missing something important here?
>
> If correct, I think the second option is cleaner, even though there is the
> issue to minimise the repetition of code for forms, views, which was
> pointed out to be easier to solve than the problem of model installation.
>
>
> What I want to emphasise here is that django.contrib.auth has to be
> installed in the case the custom model (email or other) is subclassed
> from PermissionsMixin.
>
> This is something that I'm bringing because it seems odd that d.c.auth has
> to be installed even if we want to use mailuser (or any user). Wouldn't be
> possible to detach permissions to a new app, say django.contrib.perms, and
> remove the step of installing d.c.auth? I mean, we are in authentication;
> permissions are not related with authentication.
>
> Even though I think this is because historically auth was the contrib for
> all these, which is good, I don't see why it should still be since now
> there are custom models: now there is a specific reason where I can see an
> advantage in separating these things; we could have a third option:
>
> Django's state is:
> django.contrib.auth for the standard user shipped by Django
> django.contrib.mail_auth for the mail user shipped by Django
> django.contrib.perms for permissions
>
> To use AnyUser (shipped or not shipped):
> install app_for_anyuser
> set AUTH_USER_MODEL to AnyUser
>
> To use AnyUser + Permissions:
> install app_for_user
> install django.contrib.perms
> set AUTH_USER_MODEL to AnyUser
>
> The settings for a new project would have AUTH_USER_MODEL =
> 'django.contrib.auth.models.User' and would have a new installed app
> "django.contrib.perms"
>
> I think this is nice and clean because:
>
> 1. is explicit
> 2. is intuitive how to change the auth model just by looking at the
> settings
> 3. keeps the separation that Russell pointed out between installing an app
> and setting the auth model
> 4. (not sure, don't know enough): "swappable" can be deprecated: we just
> use the one set in AUTH_USER_MODEL (and install its app, naturally)
> 5. Django's shipped user is no longer "special"; it is just the default in
> the settings.
>
> I have to admit this can be more serious than I'm seeing, but from the
> code I saw in auth, it does seem to make some sense. Naturally there are
> required migrations that have to be applied and so on, so, I'm not sure
> this should deserve a discussion on its own.
>
> In conclusion, I think that the discussion we are having is very related
> with the fact that the standard User model is defined on the same app as
> Permissions. Because permissions are required for any User that uses Admin,
> d.c.auth has to be installed. Thus, we need to avoid installing User if we
> want another User (i.e. we have to swap it).
>
> My claim is that If we detach permissions from auth, we no longer have to
> install User in the first place (we just install permissions), which means
> we can select the user (custom or shipped) by installing its app and
> selecting it in AUTH_USER_MODEL. I think this deprecates swappable, and
> makes the whole thing more explicit and flexible.
>
>

Re: AbstractUser to get more abstract

2013-09-19 Thread Aaron Merriam
I and my colleague (gavinw...@gmail.com) have made some edits to the wiki 
to clarify the purpose of authtools, and more accurately explain what the 
'authtools' approach would look like.  If you previously have examined 
'option 2', I would ask that you go and reread that section in the wiki.

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

The problem you've got here is how it knows to *not* install EmailUser. If 
> it's a model defined in d.c.auth.models, it will get installed, 
> irrespective of whether it is AUTH_USER_MODEL or not.


I wanted to verify this so I ran some basic tests and turns out that 
swappable already does the correct thing.  If two models have the same 
swappable value, only the one referenced in settings will be 
used/installed.  This makes it practical to include a second user model in 
d.c.auth.models without any changes to model loading or the swappable 
mechanism.

Refactoring d.c.auth is a low impact way to facilitate custom user models 
including and beyond email as username.  We would much rather see d.c.auth 
become more generic and more abstract, than see a new contrib app built 
specifically for email as username.  Russell stated that this issue was 
more than just about the user model, that it included forms, views, urls, 
etc.  If there is a solution which allows for views, forms, urls, etc to be 
shared, I would prefer that approach over adding a new app.

-- 
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: Default session data serializer doesn't support extended data types

2013-09-19 Thread Florian Apolloner
Hi Davide,

On Thursday, September 19, 2013 4:46:44 PM UTC+2, Davide Rizzo wrote:
>
> The inconvenience is breaking compatibility with all third party apps that 
> rely on storing extended data types (such as those supported by 
> DjangoJSONEncoder) with the default settings. Properly serializing datetime 
> (possibly tz-aware) can be hard, and changing the default puts the burden 
> on third party apps coders.
>

In all fairness, we didn't just break third party apps, we broke our code 
too… We always said that security will trump inconvenience, the only 
inconvenience I can see here is that users have to switch to the pickle 
backend if they use old third party apps (although only for those which use 
the session at all…).

They would have the option to either add two complexity layers (properly 
> serializing/deserializing datetime objects, and not breaking compatibility 
> with the previous versions of the same app), or to break compatibility with 
> Django default settings.
>

We added those two complexity layers for our code too, it was merely one 
extra line to ensure backwards compatibility.
 

> I think the option of reverting the default to pickle should be also 
> considered.
>

No, this has been discussed and security will trump minor inconvenience.

Florian

-- 
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: AbstractUser to get more abstract

2013-09-19 Thread Jorge Cardoso Leitão
Hi all.

I summarise the options with some of the issues raised, and I add my own 
concern.


One option presented here is to have both models in d.c.auth. As pointed out by 
Russell and others, this causes the problem on how to define the models such 
that they are not installed if they are not being the AUTH_USER_MODEL. Assuming 
that this problem could be solved, the user's procedure would be:

To use User (+ Permissions):
install django.contrib.auth

To use MailUser (+ Permissions):
install django.contrib.auth
set AUTH_USER_MODEL to MailUser

The second option presented here is to add a separate app (e.g. d.c.mailuser), 
which would require some imports from django.contrib.auth (forms, views). The 
user's procedure would be:

To use User (+ Permissions):
install django.contrib.auth

To use MailUser (+ Permissions):
install django.contrib.auth
install django.contrib.mailauth
set AUTH_USER_MODEL to MailUser

Is this correct or I'm missing something important here?

If correct, I think the second option is cleaner, even though there is the 
issue to minimise the repetition of code for forms, views, which was pointed 
out to be easier to solve than the problem of model installation.


What I want to emphasise here is that django.contrib.auth has to be installed 
in the case the custom model (email or other) is subclassed from 
PermissionsMixin.

This is something that I'm bringing because it seems odd that d.c.auth has to 
be installed even if we want to use mailuser (or any user). Wouldn't be 
possible to detach permissions to a new app, say django.contrib.perms, and 
remove the step of installing d.c.auth? I mean, we are in authentication; 
permissions are not related with authentication.

Even though I think this is because historically auth was the contrib for all 
these, which is good, I don't see why it should still be since now there are 
custom models: now there is a specific reason where I can see an advantage in 
separating these things; we could have a third option:

Django's state is:
django.contrib.auth for the standard user shipped by Django
django.contrib.mail_auth for the mail user shipped by Django
django.contrib.perms for permissions

To use AnyUser (shipped or not shipped):
install app_for_anyuser
set AUTH_USER_MODEL to AnyUser

To use AnyUser + Permissions:
install app_for_user
install django.contrib.perms
set AUTH_USER_MODEL to AnyUser  

The settings for a new project would have AUTH_USER_MODEL = 
'django.contrib.auth.models.User' and would have a new installed app 
"django.contrib.perms"

I think this is nice and clean because:

1. is explicit
2. is intuitive how to change the auth model just by looking at the settings
3. keeps the separation that Russell pointed out between installing an app and 
setting the auth model
4. (not sure, don't know enough): "swappable" can be deprecated: we just use 
the one set in AUTH_USER_MODEL (and install its app, naturally)
5. Django's shipped user is no longer "special"; it is just the default in the 
settings.

I have to admit this can be more serious than I'm seeing, but from the code I 
saw in auth, it does seem to make some sense. Naturally there are required 
migrations that have to be applied and so on, so, I'm not sure this should 
deserve a discussion on its own.

In conclusion, I think that the discussion we are having is very related with 
the fact that the standard User model is defined on the same app as 
Permissions. Because permissions are required for any User that uses Admin, 
d.c.auth has to be installed. Thus, we need to avoid installing User if we want 
another User (i.e. we have to swap it).

My claim is that If we detach permissions from auth, we no longer have to 
install User in the first place (we just install permissions), which means we 
can select the user (custom or shipped) by installing its app and selecting it 
in AUTH_USER_MODEL. I think this deprecates swappable, and makes the whole 
thing more explicit and flexible.

Dos this make any sense?

Meanwhile or not, I agree that having separate apps is a solution worth 
pursuing.

Regards,
Jorge

On Sep 19, 2013, at 3:41 PM, Marc Tamlyn  wrote:

> The problem you've got here is how it knows to *not* install EmailUser. If 
> it's a model defined in d.c.auth.models, it will get installed, irrespective 
> of whether it is AUTH_USER_MODEL or not. This is where we have to special 
> case the new model somehow so it is only installed if we are using it. This 
> is far from straightforwards! By putting it in a separate app which is not 
> installed by default, then we solve this issue trivially without more magic.
> 
> On 19 Sep 2013 05:27, "Luke Sneeringer"  wrote:
> Bah, I should have planned my e-mailing better. Sorry for sending a third in 
> such rapid succession.
> 
> I just did some checking into my assumptions (quoted below) on this. It turns 
> out t

Default session data serializer doesn't support extended data types

2013-09-19 Thread Davide Rizzo
#20922  introduced the option 
to choose a custom session data serializer. The default option is to use 
the new JSONSerializer starting from 1.6, since using pickle would lead to 
a remote code execution vulnerability when session data is stored in 
cookies.

While this can be considered a sensible security choice, it becomes 
inconvenient as the JSON encoder used by JSONSerializer is not the same 
used elsewhere in Django, as it only support basic data types: string, 
int/floats, booleans, nested dicts and lists, None.

The inconvenience is breaking compatibility with all third party apps that 
rely on storing extended data types (such as those supported by 
DjangoJSONEncoder) with the default settings. Properly serializing datetime 
(possibly tz-aware) can be hard, and changing the default puts the burden 
on third party apps coders.

They would have the option to either add two complexity layers (properly 
serializing/deserializing datetime objects, and not breaking compatibility 
with the previous versions of the same app), or to break compatibility with 
Django default settings.

As an example of commonly used data types that can't be stored anymore with 
default settings:

   - datetime, timedelta objects (supported by DjangoJSONEncoder)
   - decimal objects (supported by DjangoJSONEncoder)
   - arbitrary binary strings
   - Geometry objects

I think the option of reverting the default to pickle should be also 
considered.

[[I originally posted this as 
#21124, 
where it was closed as not a bug. What follows is the response I got, for 
reference:

by timo (core developer)

Thanks for your thoughts. I think most of the points you've raised were 
discussed during the implementation of this, either on the ticket 
(#20922) 
or on the linked pull request  (or 
the documentation 
itself).
 
Could you please take a look at the discussion there if you haven't? If 
after reading that you still have disagreements, please raise the issue on 
django-developers rather than this ticket tracker. Thanks!

Suggestions for documentations edits or additions would also be welcome.

p.s. To address one of your points, one of the decisions was indeed to put 
the burden on third party app coders to serialize session data as simple 
data types like strings which would be compatible with JSON. We made this 
change to contrib.messages for example.
]]

-- 
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: Is "transaction.atomic" in 1.6 supposed to work this way?

2013-09-19 Thread Anssi Kääriäinen
After some investigation it turns out that this isn't about IntegrityErrors 
at all. Instead the problem is that inside @atomic block
Model.save() uses @atomic(savepoint=False) internally. And 
@atomic(savepoint=False) forces the outer atomic block to roll back if 
errors
happen.

If I recall correctly there is transaction.set_rollback(False) which can be 
used to remove forced rollback.

In general, there are three ways to respond to errors inside transactions:
  1. allow usage of the TX (MySQL, SQLite etc), allow user to decide 
commit/rollback
  2. forbid usage of the TX (PostgreSQL), force it to be rolled back.
  3. allow usage of the TX, but force rollback (Django)

To me it seems explicit error when using connection in "forced to rollback" 
state is better than allowing saving more data, then silently
rolling back the transaction. As you said this is useless.

To be clear, this is about exceptions (database or other) in 
atomic(savepoint=False) blocks, not about caught IntegrityErrors - Django
will allow you to continue and commit the TX in the caught IntgerityError 
case assuming the DB allows that.

 - Anssi


On Thursday, September 19, 2013 5:29:39 PM UTC+3, Aymeric Augustin wrote:
>
> 2013/9/19 Anssi Kääriäinen >
>
>> There is a definite disparency about what the code does and what the docs 
>> say:
>> """
>> If the block of code is successfully completed, the changes are committed 
>> to the database. If there is an exception, the changes are rolled back.
>> """
>>
>> If an exception is catched inside the block, then there should be no 
>> rollback.
>>
>
> I'm not following you; as far as I can tell, that's what the docs say and 
> what the code does.
>
> With PostgreSQL you can't continue to use the transaction, but in other 
>> databases you can.
>>
>
> You... may... might... Truth be said, you shan't.
>  
>
>> For these other databases allowing use of the transaction, yet still 
>> rolling back it later on seems to be a footgun - why are you allowed to 
>> continue to use the transaction if a rollback is going to happen anyways?
>>
>
> Allowing a transaction to proceed after a statement has failed is:
> - stupid if the transaction is allowed to commit — you would end up with a 
> successful transaction where not all statements have executed, which is 
> exactly the situation transactions are designed to prevent!
> - useless if it isn't allowed to commit.
>
> That's why PostgreSQL forbids it.
>
> That's also what the SQLite docs say:
>  http://www.sqlite.org/lang_transaction.html
> "It is recommended that applications respond to the errors listed above by 
> explicitly issuing a ROLLBACK command."
>
> Extracting the same information from the MySQL and Oracle docs is left as 
> an exercise for the reader.
>
> -- 
> Aymeric. 
>

-- 
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: Is "transaction.atomic" in 1.6 supposed to work this way?

2013-09-19 Thread Aymeric Augustin
2013/9/19 Anssi Kääriäinen 

> There is a definite disparency about what the code does and what the docs
> say:
> """
> If the block of code is successfully completed, the changes are committed
> to the database. If there is an exception, the changes are rolled back.
> """
>
> If an exception is catched inside the block, then there should be no
> rollback.
>

I'm not following you; as far as I can tell, that's what the docs say and
what the code does.

With PostgreSQL you can't continue to use the transaction, but in other
> databases you can.
>

You... may... might... Truth be said, you shan't.


> For these other databases allowing use of the transaction, yet still
> rolling back it later on seems to be a footgun - why are you allowed to
> continue to use the transaction if a rollback is going to happen anyways?
>

Allowing a transaction to proceed after a statement has failed is:
- stupid if the transaction is allowed to commit — you would end up with a
successful transaction where not all statements have executed, which is
exactly the situation transactions are designed to prevent!
- useless if it isn't allowed to commit.

That's why PostgreSQL forbids it.

That's also what the SQLite docs say:
http://www.sqlite.org/lang_transaction.html
"It is recommended that applications respond to the errors listed above by
explicitly issuing a ROLLBACK command."

Extracting the same information from the MySQL and Oracle docs is left as
an exercise for the reader.

-- 
Aymeric.

-- 
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: Is "transaction.atomic" in 1.6 supposed to work this way?

2013-09-19 Thread Anssi Kääriäinen
There is a definite disparency about what the code does and what the docs 
say:
"""
If the block of code is successfully completed, the changes are committed 
to the database. If there is an exception, the changes are rolled back.
"""

If an exception is catched inside the block, then there should be no 
rollback. With PostgreSQL you can't continue to use the transaction, but in 
other databases you can. For these other databases allowing use of the 
transaction, yet still rolling back it later on seems to be a footgun - why 
are you allowed to continue to use the transaction if a rollback is going 
to happen anyways?

 - Anssi


On Thursday, September 19, 2013 4:33:53 PM UTC+3, Aymeric Augustin wrote:
>
> 2013/9/19 Richard Ward >
>
>> So what is the problem here? I assume it is one of:
>>
>
>>1. 'innocent_looking_function' is badly written: it should not be 
>>catching IntegrityErrors under any circumstances (though that seems like 
>> a 
>>valid thing to do), it should instead use something like get_or_create.
>>
>>
> It's possible to catch IntegrityErrors. The correct pattern — regardless 
> of function calls — is:
>
> try:
> with transaction.atomic():
> do_stuff()
> except IntegrityError:
> # transaction.atomic has already performed the rollback at this point
> handle_error()
>
>  It's documented: 
> https://docs.djangoproject.com/en/dev/topics/db/transactions/#django.db.transaction.atomic
>
>>
>>1. 'innocent_looking_function' should have 'with 
>>transaction.atomic():' just inside the 'try' block, and presumably so 
>>should every other bit of code where an IntegrityError is caught. But 
>> what 
>>if 'innocent_looking_function' also gets used elsewhere that may not be 
>>inside an atomic block?
>>
>> Indeed that would implement the pattern shown above and resolve the 
> problem.
>
>>
>>1. transaction.atomic is in some way buggy / oddly designed.
>>
>> As the designer of this API, I say: suggestions welcome :) Read below 
> before jumping to the drawing board, though.
>
>> IMO this behaviour makes code that looks valid (to me) not work in the 
>> expected way.
>>
> Catching IntegrityError inside transaction.atomic is wrong because it 
> hides the exception from transaction.atomic.
>
> As a consquence, transaction.atomic believe that everything went well and 
> merrily commits. But the database has a broken transaction in progress; it 
> can't commit.. transaction.atomic fails to commit, rolls back, and raises 
> an exception — which isn't the original IntegrityError, adding to confusion.
>
>> In case its relevant I'm using InnoDB tables on MySQL.
>>
> You might have received more meaningful errors with another database 
> engine, but overall you've described the intended, cross-database behavior 
> of transaction.atomic. 
>
>> So Is there a bug? Or am I using this incorrectly, or misunderstanding 
>> something?
>>
> The code works as designed. It relies on exceptions to distinguish failure 
> from success. This is the cleanest way to implement transactions as a 
> context manager. If you swallow the exception, you make a failure look like 
> a success, and things go wrong.
>
> The docs talk a lot about exceptions but they don't warn 
> specifically about this pitfall. It would be a worthy addition. Can you 
> file a ticket?
>
> Thank you,
>  
> -- 
> Aymeric. 
>

-- 
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: AbstractUser to get more abstract

2013-09-19 Thread Marc Tamlyn
The problem you've got here is how it knows to *not* install EmailUser. If
it's a model defined in d.c.auth.models, it will get installed,
irrespective of whether it is AUTH_USER_MODEL or not. This is where we have
to special case the new model somehow so it is only installed if we are
using it. This is far from straightforwards! By putting it in a separate
app which is not installed by default, then we solve this issue trivially
without more magic.
On 19 Sep 2013 05:27, "Luke Sneeringer"  wrote:

> Bah, I should have planned my e-mailing better. Sorry for sending a third
> in such rapid succession.
>
> I just did some checking into my assumptions (quoted below) on this. It
> turns out that Django core already does some of what I suggest: in
> particular, it doesn't install the User model if django.contrib.auth is
> installed but another AUTH_USER_MODEL is invoked, based on a Meta option
> called "swappable" (see [here][1] and [here][2]).
>
> Am I misunderstanding? It seems like we are already pretty well down my
> proposed path.
>
> Best Regards,
> Luke
>
>   [1]:
> https://github.com/django/django/blob/b2b763448f726ee952743596e9a34fcb154bdb12/django/contrib/auth/models.py#L406-L414
>   [2]:
> https://github.com/django/django/blob/master/django/db/models/loading.py#L296
>
>
> On Sep 18, 2013, at 10:03 PM, Luke Sneeringer  wrote:
>
> On Wednesday, September 18, 2013 6:39:13 PM UTC-6, Russell Keith-Magee
> wrote:
>>
>>
>> On Thu, Sep 19, 2013 at 3:39 AM, Luke Sneeringer wrote:
>>
>>> I added the authtools approach to the wiki for completion, although I
>>> believe it to be an inferior approach.
>>>
>>> One thing I dislike is having a separate app (e.g. d.c.auth_email) that
>>> has to be installed separately. That feels pretty impure to me. I'm doing a
>>> thought exercise about potential solutions, though, and not exactly coming
>>> up aces.
>>>
>>> The best solution I can currently think of is to have User and EmailUser
>>> which are both models that live in django.contrib.auth. Then, we would have
>>> to add code to our model detection that says that *if* a model is a
>>> subclass of AbstractBaseUser, include it if and only if it is the
>>> AUTH_USER_MODEL.
>>>
>>> I can't decide if that solution is better or worse than the disease. It
>>> makes for a much more attractive set of steps to follow for people who want
>>> to use it, though -- basically, just set AUTH_USER_MODEL to
>>> 'auth.EmailUser', and done.
>>>
>>>
>> My opinion - it's worse than the disease.
>>
>> Option 1 involves a clean auth app that just contains a stub user, and a
>> clean extension app providing an alternative user. You install the
>> extension app, and say you want to use it.
>>
>> Option 2 makes a special case of *one particular* extension user, and
>> makes all the internals of models, forms, views, etc embedded inside an if
>> statement.
>>
>>
> I think where I part from you is on the "one particular" aspect of the
> discussion. I actually see the "option 2" approach as having very clean
> side effects for some other use cases.
>
> Allow me to explain:
> There is, as you have pointed out, a lot more to auth than just the User
> model. There are forms and the like, which you mentioned, but there are
> also other aspects: the permissions models and functionality, for instance.
> Subclasses of AbstractBaseUser which also subclass PermissionsMixin still
> get the permissions things out of the box, and that is a documented use
> case in the official documentation.
>
> So, right now (if I understand correctly), if you install the auth app,
> you get the Group and Permission models (and supporting code, natch), as
> well as the User model. If you decide to set a different User model using
> AUTH_USER_MODEL, but you keep auth installed for these other aspects
> (Group, Permission, etc.) then Django installs an auth_user table that
> isn't used (note to self: verify this).
>
> If you set AUTH_USER_MODEL to something other than auth.User, you are
> making a statement that you *do not want* the stock model. This is true
> whether you change it to the upcoming, included "EmailUser" model, or to
> something else entirely.
>
> So, I don't think that option 2 "makes a special case of one particular
> extension user". If anything, I assert it would do the opposite: it
> actually performs the most expected behavior in all cases. If the plain
> User model is the AUTH_USER_MODEL and d.c.auth is in INSTALLED APPS, then
> it is installed. If you choose to use an included-in-core e-mail model and
> install d.c.auth, you get that model and table *instead of* the current
> User model. And, if you use your own, special custom User model, but
> install auth for the remaining aspects, then you get your custom user
> model, and not either of the stock ones.
>
> Basically, my understanding is that you start to get unclean and
> counter-intuitive behavior when you want to install the auth app for
> everything other than the User model (and forms, etc.), th

Re: Is "transaction.atomic" in 1.6 supposed to work this way?

2013-09-19 Thread Aymeric Augustin
2013/9/19 Richard Ward 

> So what is the problem here? I assume it is one of:
>

>1. 'innocent_looking_function' is badly written: it should not be
>catching IntegrityErrors under any circumstances (though that seems like a
>valid thing to do), it should instead use something like get_or_create.
>
>
It's possible to catch IntegrityErrors. The correct pattern — regardless of
function calls — is:

try:
with transaction.atomic():
do_stuff()
except IntegrityError:
# transaction.atomic has already performed the rollback at this point
handle_error()

It's documented:
https://docs.djangoproject.com/en/dev/topics/db/transactions/#django.db.transaction.atomic

>
>1. 'innocent_looking_function' should have 'with
>transaction.atomic():' just inside the 'try' block, and presumably so
>should every other bit of code where an IntegrityError is caught. But what
>if 'innocent_looking_function' also gets used elsewhere that may not be
>inside an atomic block?
>
> Indeed that would implement the pattern shown above and resolve the
problem.

>
>1. transaction.atomic is in some way buggy / oddly designed.
>
> As the designer of this API, I say: suggestions welcome :) Read below
before jumping to the drawing board, though.

> IMO this behaviour makes code that looks valid (to me) not work in the
> expected way.
>
Catching IntegrityError inside transaction.atomic is wrong because it hides
the exception from transaction.atomic.

As a consquence, transaction.atomic believe that everything went well and
merrily commits. But the database has a broken transaction in progress; it
can't commit.. transaction.atomic fails to commit, rolls back, and raises
an exception — which isn't the original IntegrityError, adding to confusion.

> In case its relevant I'm using InnoDB tables on MySQL.
>
You might have received more meaningful errors with another database
engine, but overall you've described the intended, cross-database behavior
of transaction.atomic.

> So Is there a bug? Or am I using this incorrectly, or misunderstanding
> something?
>
The code works as designed. It relies on exceptions to distinguish failure
from success. This is the cleanest way to implement transactions as a
context manager. If you swallow the exception, you make a failure look like
a success, and things go wrong.

The docs talk a lot about exceptions but they don't warn specifically about
this pitfall. It would be a worthy addition. Can you file a ticket?

Thank you,

-- 
Aymeric.

-- 
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: Is "transaction.atomic" in 1.6 supposed to work this way?

2013-09-19 Thread Gert Van Gool
If the `innocent_looking_function` would use transactions. And thus handles
the `IntegrityError by`
issuing a rollback, just like `get_or_create` does (
https://github.com/django/django/blob/1.6b4/django/db/models/query.py#L360-L390
).
Do you see the same behaviour in `my_func`?

-- Gert

Mobile: +32 498725202
Twitter: @gvangool 
Web: http://gertvangool.be


On Thu, Sep 19, 2013 at 2:38 PM, Richard Ward wrote:

> We're building a site that needs to use transactions, and have been doing
> so against the 1.6 beta as 1.6 is nearly out and we thought it would be
> easier to use the new transaction api, but we came across an unexpected
> problem.
>
> Basically if you catch an IntegrityError and 'recover' from it, then your
> transaction is still not committed.
>
> I'm posting this here rather than in django-users as its a feature in an
> unreleased version and it seems like it a bug to me (apologies if it is the
> wrong place).
>
> Take this code:
>
> def innocent_looking_function():
> "Makes sure that a MyModel with foo=1 exists"
> try:
> #imagine mymodel has a unique constraint on 'foo'
> MyModel(foo=1).save()
> except IntegrityError:
> pass
>
> @transaction.atomic
> def my_func():
> innocent_looking_function()
> MyOtherModel(bar="hi").save()
>
> If you call 'my_func' when a MyModel with foo=1 exists, then the
> transaction.atomic decorator will roll back the transaction, even though it
> seems like everything is alright. 'my_func' doesn't know that
> 'innocent_looking_func' may cause this behaviour, and as far as
> 'innocent_looking_func' is concerned it has not done anything wrong - it
> caught and ignored a harmless error.
>
> I had assumed that transaction.atomic would only roll back the transaction
> if an exception was propagated through it (eg if innocent_looking_function
> did not catch the error), that would seem like the natural way of doing
> things.
>
> So what is the problem here? I assume it is one of:
>
>1. 'innocent_looking_function' is badly written: it should not be
>catching IntegrityErrors under any circumstances (though that seems like a
>valid thing to do), it should instead use something like get_or_create.
>2. 'innocent_looking_function' should have 'with
>transaction.atomic():' just inside the 'try' block, and presumably so
>should every other bit of code where an IntegrityError is caught. But what
>if 'innocent_looking_function' also gets used elsewhere that may not be
>inside an atomic block?
>3. transaction.atomic is in some way buggy / oddly designed.
>
> IMO this behaviour makes code that looks valid (to me) not work in the
> expected way.
>
> In case its relevant I'm using InnoDB tables on MySQL.
>
> So Is there a bug? Or am I using this incorrectly, or misunderstanding
> something?
>
> Thanks,
>
> Richard
>
> --
> 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.
>

-- 
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.


Is "transaction.atomic" in 1.6 supposed to work this way?

2013-09-19 Thread Richard Ward
We're building a site that needs to use transactions, and have been doing 
so against the 1.6 beta as 1.6 is nearly out and we thought it would be 
easier to use the new transaction api, but we came across an unexpected 
problem. 

Basically if you catch an IntegrityError and 'recover' from it, then your 
transaction is still not committed.

I'm posting this here rather than in django-users as its a feature in an 
unreleased version and it seems like it a bug to me (apologies if it is the 
wrong place).

Take this code:

def innocent_looking_function():
"Makes sure that a MyModel with foo=1 exists"
try: 
#imagine mymodel has a unique constraint on 'foo'
MyModel(foo=1).save() 
except IntegrityError: 
pass

@transaction.atomic 
def my_func(): 
innocent_looking_function() 
MyOtherModel(bar="hi").save() 

If you call 'my_func' when a MyModel with foo=1 exists, then the 
transaction.atomic decorator will roll back the transaction, even though it 
seems like everything is alright. 'my_func' doesn't know that 
'innocent_looking_func' may cause this behaviour, and as far as 
'innocent_looking_func' is concerned it has not done anything wrong - it 
caught and ignored a harmless error.

I had assumed that transaction.atomic would only roll back the transaction 
if an exception was propagated through it (eg if innocent_looking_function 
did not catch the error), that would seem like the natural way of doing 
things.

So what is the problem here? I assume it is one of:

   1. 'innocent_looking_function' is badly written: it should not be 
   catching IntegrityErrors under any circumstances (though that seems like a 
   valid thing to do), it should instead use something like get_or_create.
   2. 'innocent_looking_function' should have 'with transaction.atomic():' 
   just inside the 'try' block, and presumably so should every other bit of 
   code where an IntegrityError is caught. But what if 
   'innocent_looking_function' also gets used elsewhere that may not be inside 
   an atomic block?
   3. transaction.atomic is in some way buggy / oddly designed.

IMO this behaviour makes code that looks valid (to me) not work in the 
expected way.

In case its relevant I'm using InnoDB tables on MySQL.

So Is there a bug? Or am I using this incorrectly, or misunderstanding 
something?

Thanks,

Richard

-- 
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.