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

2012-06-07 Thread Anssi Kääriäinen
On 8 kesä, 02:43, Russell Keith-Magee  wrote:
> Good idea. I haven't tried to see what it actually does; I'm guessing
> you're going to get Table Does Not Exist errors. A nicer error
> wouldn't hurt.

Yes, that is what happens.

> >  - For documentation: It should be suggested that the example MyUser
> > should define class Meta: swappable = 'AUTH_USER_MODEL'. Otherwise it
> > will be synced to the database even if it is not the chosen user model
> > for the project, which is likely not wanted. Maybe AbstractBaseUser
> > should define swappable = 'AUTH_USER_MODEL' so that anybody who
> > inherits from that gets the setting automatically?
>
> Ah - I think I see the problem now. The swappable Meta option isn't
> visible to to end user -- ever -- unless they go spelunking in the
> source code for auth.models.
>
> If I define MyUser, I *dont'* need to define it as swappable. I just
> need to define AUTH_USER_MODEL in my settings file to point at my
> replacement model definition. The only place where swappable is
> defined is on the base model that is being swapped out -- in this
> case, auth.User.
>
> I'm expecting that there will be *no* documentation of the swappable
> option (unless, at some point in the future, we decide to make
> swappable models an official feature). It exists purely to make the
> internals clean, and avoid making auth.User a special case in code.

The real problem comes when you have multiple swappable models defined
for the same swappable "slot". This could happen in a CMS for example
- they might define a couple of classes to use, you can choose which
one to use by using a setting. Or, maybe you have some library
installed which defines some reusable User models. Problem is, every
one of these models will be installed unless they provided the
swappable = 'AUTH_USER_MODEL' meta option. If you don't provide that
option, nothing ties the models to the same slot.

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

The model_label_for_default_model can't be defined anywhere in the
current implementation. The Model._meta.swapped uses:
return self.swappable and getattr(settings, self.swappable, None)
not in (None, model_label)

> >> > About the ORM capabilities of the interface: It seems there can not be
> >> > any assumptions about the swapped in model here: for example
> >> > SomeModel.filter(user__is_staff=True) or
> >> > SomeModel.order_by('user__lastname', 'user__firstname') works
> >> > currently, but there are no such lookups available for the interface.
> >> > Maybe there should be some way to order by "full_name" and
> >> > "short_name" at least. But, this gets us quickly to virtual fields or
> >> > virtual lookups territory...
>
> >> Imo it's fine if we require some fields on a Usermodel to be compatible
> >> with admin and friends. (eg a first/last_name field which is nullable [or
> >> not required in the form validation sense] shouldn't hurt anyone, the admin
> >> could check if the field is filled and if not display something else like
> >> the username).
>
> > Admin is actually somewhat easy to make work re the ordering, because
> > in there you can use get_full_name.admin_order_field =
> > 'somemodelfield'. This doesn't work outside Admin. This isn't a major
> > issue, it prevents things like ordering comments on the commenter's
> > "get_short_name" output. Even then, this is only an issue for reusable
> > code - in your project you know the concrete user model you have in
> > use, and can thus use the specific lookups your model has.
>
> I can see two approaches here.
>
> The first is the full audit of admin that I've mentioned previously.
> That will reveal everything that admin is depending on, and will allow
> us to define a full "admin user interface" that any swappable user
> must define.
>
> The second is to abstract away the ordering field in the same was as
> we've abstracted get_short_name -- i.e., don't specify what the field
> needs to be called, just require that the user provide an option that
> defines how it can be retrieved. This is 

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

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

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

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

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

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

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

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

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

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

Agreed -- error messages would be better.

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

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

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

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

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

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

Re: Proposed Field API additions

2012-06-07 Thread Anssi Kääriäinen
On Jun 7, 11:16 pm, Alex Ogier  wrote:
> This isn't particularly robust. The SQL string generated by a
> particular backend isn't considered part of any API, and might change
> formatting or semantics on minor updates. Some SQL is quite
> complicated and dubiously related to the particular field, such as
> extra INDEX or UNIQUE constraint statements. Certain backends elide
> some arguments, performing constraint checking in the application (the
> sqlite backend for example just drops many constraints). In general,
> you want the migrations to be DB-agnostic, and checking SQL output
> loses this feature.

Ah, so you will save the current state somewhere, but not necessarily
the DB being migrated. And yes, I can see how upgrading Django from
1.4 to 1.4.1 could change the SQL in subtle ways...

If you track changes by __init__ args, wouldn't then 'label' change
mark the field as changed? Is this a problem?

> >>  - Requiring all fields to be accessible by only their app label and
> >> class name/other unique name.
>
> >> This means either having to register custom fields (like admin classes,
> >> for example), or requiring fields to live in a fields.py or fields
> >> package (like models and models.py). This is to provide for a
> >> less-fragile way of referring to them than their full module path (which
> >> might change based on project name or package location).
>
> > As an idea, why not mimic the models.Model.__new__. You could have
> > Field.__new__ which does the registration. A field could have
> > app_label and "field name" as Meta (or just base level arguments). If
> > these are not supplied, they will be generated by the __new__ from the
> > field's class.__name__ and module. Once backwards compatibility period
> > is over the meta variables are required.
>
> > Any field which is never imported will never get registered, but I
> > don't see that as a problem - if the field is not in use in the
> > project, why should it need to be registered?
>
> Adding additional invariants to Fields like "the (trailing module
> path, ClassName) tuple must be unique over all fields" is arguably
> backwards incompatible. Registering has several benefits. For example,
> it doesn't impose any constraints until you decide that you want to
> use migrations, at which point a nice error can be thrown: "Model Foo
> has an unknown field: `field_name` of type: `project.app.CustomField`.
> Please register this field with
> `django.db.migrations.register_field()` before creating migrations on
> the Foo model." Also, you can support third-party fields by
> registering them in your own modules if you need to instead of hacking
> on their code if they haven't updated it.

I don't see the difference between doing the registration
automatically by Field.__new__() and automatically registering
anything living in fields.py or fields module.

 - Anssi

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



Re: Proposed Field API additions

2012-06-07 Thread Alex Ogier
On Thu, Jun 7, 2012 at 3:56 PM, Anssi Kääriäinen
 wrote:
> On Jun 7, 8:17 pm, Andrew Godwin  wrote:
>> Hi everyone,
>>
>> As part of my planning for adding schema alteration/migrations into
>> Django proper, I need to make a few changes to Fields to allow for
>> better serialisation of model definitions (pretty much a requirement for
>> any change-detecting migrations system).
>>
>> In particular, I propose:
>>
>>  - Requiring that all fields expose a method which says how to
>> reconstruct them.
>>
>> Essentially, it returns the positional and keyword arguments you would
>> have to pass into the constructor to make the field again (as there's no
>> way to get them directly). For those familiar with south_field_triple,
>> it would be a bit like that.
>
> Is the reason for this to be able to track changes to field by
> checking if its init arguments have changed? Why is it not possible to
> track changes by checking the SQL output the field will generate
> instead? This is guaranteed to be a string, and should be readily
> available for any field. I am just trying to get up to speed here...
>

This isn't particularly robust. The SQL string generated by a
particular backend isn't considered part of any API, and might change
formatting or semantics on minor updates. Some SQL is quite
complicated and dubiously related to the particular field, such as
extra INDEX or UNIQUE constraint statements. Certain backends elide
some arguments, performing constraint checking in the application (the
sqlite backend for example just drops many constraints). In general,
you want the migrations to be DB-agnostic, and checking SQL output
loses this feature.

>>  - Requiring all fields to be accessible by only their app label and
>> class name/other unique name.
>>
>> This means either having to register custom fields (like admin classes,
>> for example), or requiring fields to live in a fields.py or fields
>> package (like models and models.py). This is to provide for a
>> less-fragile way of referring to them than their full module path (which
>> might change based on project name or package location).
>
> As an idea, why not mimic the models.Model.__new__. You could have
> Field.__new__ which does the registration. A field could have
> app_label and "field name" as Meta (or just base level arguments). If
> these are not supplied, they will be generated by the __new__ from the
> field's class.__name__ and module. Once backwards compatibility period
> is over the meta variables are required.
>
> Any field which is never imported will never get registered, but I
> don't see that as a problem - if the field is not in use in the
> project, why should it need to be registered?
>

Adding additional invariants to Fields like "the (trailing module
path, ClassName) tuple must be unique over all fields" is arguably
backwards incompatible. Registering has several benefits. For example,
it doesn't impose any constraints until you decide that you want to
use migrations, at which point a nice error can be thrown: "Model Foo
has an unknown field: `field_name` of type: `project.app.CustomField`.
Please register this field with
`django.db.migrations.register_field()` before creating migrations on
the Foo model." Also, you can support third-party fields by
registering them in your own modules if you need to instead of hacking
on their code if they haven't updated it.

Best,
Alex Ogier

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



Re: Proposed Field API additions

2012-06-07 Thread Anssi Kääriäinen
On Jun 7, 8:17 pm, Andrew Godwin  wrote:
> Hi everyone,
>
> As part of my planning for adding schema alteration/migrations into
> Django proper, I need to make a few changes to Fields to allow for
> better serialisation of model definitions (pretty much a requirement for
> any change-detecting migrations system).
>
> In particular, I propose:
>
>  - Requiring that all fields expose a method which says how to
> reconstruct them.
>
> Essentially, it returns the positional and keyword arguments you would
> have to pass into the constructor to make the field again (as there's no
> way to get them directly). For those familiar with south_field_triple,
> it would be a bit like that.

Is the reason for this to be able to track changes to field by
checking if its init arguments have changed? Why is it not possible to
track changes by checking the SQL output the field will generate
instead? This is guaranteed to be a string, and should be readily
available for any field. I am just trying to get up to speed here...

>  - Requiring all fields to be accessible by only their app label and
> class name/other unique name.
>
> This means either having to register custom fields (like admin classes,
> for example), or requiring fields to live in a fields.py or fields
> package (like models and models.py). This is to provide for a
> less-fragile way of referring to them than their full module path (which
> might change based on project name or package location).

As an idea, why not mimic the models.Model.__new__. You could have
Field.__new__ which does the registration. A field could have
app_label and "field name" as Meta (or just base level arguments). If
these are not supplied, they will be generated by the __new__ from the
field's class.__name__ and module. Once backwards compatibility period
is over the meta variables are required.

Any field which is never imported will never get registered, but I
don't see that as a problem - if the field is not in use in the
project, why should it need to be registered?

 - Anssi

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



Re: Proposed Field API additions

2012-06-07 Thread Andrew Godwin
On 07/06/12 20:14, Alex Gaynor wrote:
>
>
> On Thu, Jun 7, 2012 at 12:17 PM, Andrew Godwin  > wrote:
>
>
> In particular, I propose:
>
>  - Requiring that all fields expose a method which says how to
> reconstruct them.
>
> Essentially, it returns the positional and keyword arguments you would
> have to pass into the constructor to make the field again (as
> there's no
> way to get them directly). For those familiar with south_field_triple,
> it would be a bit like that.
>
>
> This sounds similar to pickling's __getinitargs__, is there anyway we
> can reuse some of:
> http://docs.python.org/library/pickle.html#object.__getinitargs__

Hmm, it's close, but by not allowing keywords it's going to be very
unsightly for something with optional arguments. Additionally, we're
going to need to restrict the output from any function like this to
values we can serialise as text (basically, repr plus a few custom
handlers for things like model instances), and getinitargs doesn't
enforce that either.

Andrew

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



Re: Proposed Field API additions

2012-06-07 Thread Dalton Barreto
2012/6/7 Andrew Ingram :
> On 7 June 2012 18:17, Andrew Godwin  wrote:
>> This means either having to register custom fields (like admin classes,
>> for example), or requiring fields to live in a fields.py or fields
>> package (like models and models.py). This is to provide for a
>> less-fragile way of referring to them than their full module path (which
>> might change based on project name or package location).
>>
>> Neither of these two options is perfect - registration means a little
>> more "boilerplate" code, while requiring them to be in a certain module
>> is going to hurt apps that don't have them there already. For that
>> reason, I prefer the registration approach - it will only be one extra
>> line per field, it's a pattern already used for admin classes and
>> filters/tags, and it will allow for field classes to be renamed while
>> keeping the same registered name (if someone wants).
>
> I prefer the registration approach because it's more explicit. But
> there's also the third option of using both, auto-register anything in
> fields.py but allow explicit registration for anything found
> elsewhere.
>

Continuing on the auto-register approach. Would be a good idea to have
just one more
option on settings.py that would make all fields auto-registered?
Despite being an "all or nothing"
option, I don't know if it's that common choose to auto-migrate just
part of the application database.

This would also make the path more smooth to people wanting to change
their app to be auto-migratable.

Thanks,

-- 
Dalton Barreto
http://daltonmatos.com

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



Re: Proposed Field API additions

2012-06-07 Thread Alex Gaynor
On Thu, Jun 7, 2012 at 12:17 PM, Andrew Godwin  wrote:

> Hi everyone,
>
> As part of my planning for adding schema alteration/migrations into
> Django proper, I need to make a few changes to Fields to allow for
> better serialisation of model definitions (pretty much a requirement for
> any change-detecting migrations system).
>
> In particular, I propose:
>
>  - Requiring that all fields expose a method which says how to
> reconstruct them.
>
> Essentially, it returns the positional and keyword arguments you would
> have to pass into the constructor to make the field again (as there's no
> way to get them directly). For those familiar with south_field_triple,
> it would be a bit like that.
>
>
This sounds similar to pickling's __getinitargs__, is there anyway we can
reuse some of:
http://docs.python.org/library/pickle.html#object.__getinitargs__

Alex


> There will have to be some restrictions on what can be returned as valid
> values - no values that are pre-made anonymous functions, for example -
> but that can be cleared up later.
>
>
>  - Requiring all fields to be accessible by only their app label and
> class name/other unique name.
>
> This means either having to register custom fields (like admin classes,
> for example), or requiring fields to live in a fields.py or fields
> package (like models and models.py). This is to provide for a
> less-fragile way of referring to them than their full module path (which
> might change based on project name or package location).
>
> Neither of these two options is perfect - registration means a little
> more "boilerplate" code, while requiring them to be in a certain module
> is going to hurt apps that don't have them there already. For that
> reason, I prefer the registration approach - it will only be one extra
> line per field, it's a pattern already used for admin classes and
> filters/tags, and it will allow for field classes to be renamed while
> keeping the same registered name (if someone wants).
>
> I'd appreciate feedback on these general ideas - a more concrete API
> proposal will come later along with details about how I plan to approach
> the rest of the problem, but this is one of the few direct changes to
> Django's core and so needs dicussion first.
>
> Andrew
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers" group.
> To post to this group, send email to django-developers@googlegroups.com.
> To unsubscribe from this group, send email to
> django-developers+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.
>
>


-- 
"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero

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



Re: Proposed Field API additions

2012-06-07 Thread Andrew Ingram
On 7 June 2012 18:17, Andrew Godwin  wrote:
> This means either having to register custom fields (like admin classes,
> for example), or requiring fields to live in a fields.py or fields
> package (like models and models.py). This is to provide for a
> less-fragile way of referring to them than their full module path (which
> might change based on project name or package location).
>
> Neither of these two options is perfect - registration means a little
> more "boilerplate" code, while requiring them to be in a certain module
> is going to hurt apps that don't have them there already. For that
> reason, I prefer the registration approach - it will only be one extra
> line per field, it's a pattern already used for admin classes and
> filters/tags, and it will allow for field classes to be renamed while
> keeping the same registered name (if someone wants).

I prefer the registration approach because it's more explicit. But
there's also the third option of using both, auto-register anything in
fields.py but allow explicit registration for anything found
elsewhere.

- Andy

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



Proposed Field API additions

2012-06-07 Thread Andrew Godwin
Hi everyone,

As part of my planning for adding schema alteration/migrations into
Django proper, I need to make a few changes to Fields to allow for
better serialisation of model definitions (pretty much a requirement for
any change-detecting migrations system).

In particular, I propose:

 - Requiring that all fields expose a method which says how to
reconstruct them.

Essentially, it returns the positional and keyword arguments you would
have to pass into the constructor to make the field again (as there's no
way to get them directly). For those familiar with south_field_triple,
it would be a bit like that.

There will have to be some restrictions on what can be returned as valid
values - no values that are pre-made anonymous functions, for example -
but that can be cleared up later.


 - Requiring all fields to be accessible by only their app label and
class name/other unique name.

This means either having to register custom fields (like admin classes,
for example), or requiring fields to live in a fields.py or fields
package (like models and models.py). This is to provide for a
less-fragile way of referring to them than their full module path (which
might change based on project name or package location).

Neither of these two options is perfect - registration means a little
more "boilerplate" code, while requiring them to be in a certain module
is going to hurt apps that don't have them there already. For that
reason, I prefer the registration approach - it will only be one extra
line per field, it's a pattern already used for admin classes and
filters/tags, and it will allow for field classes to be renamed while
keeping the same registered name (if someone wants).

I'd appreciate feedback on these general ideas - a more concrete API
proposal will come later along with details about how I plan to approach
the rest of the problem, but this is one of the few direct changes to
Django's core and so needs dicussion first.

Andrew

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



Re: Django git guidelines

2012-06-07 Thread Aymeric Augustin
On 6 juin 2012, at 21:09, Anssi Kääriäinen wrote:
> I am sure there is still a lot to polish. I might have tried to change
> too big portion of the docs in one go. Still, I would like to commit
> what I have tomorrow, so that the sprinters at djangocon have the
> possibility to use the guidelines and begin the polishing work.
> Alternatively, if it is possible to build the docs and make them
> available somewhere for the sprinters, I could wait for reviews from
> the sprinters, then commit the code early next week. This would make
> for a really good review for the guidelines.
> 
> The work is available from:
> https://github.com/akaariai/django/tree/django_git_guidelines
> or for compare view:
> https://github.com/akaariai/django/compare/django_git_guidelines

Hello Anssi,

As discussed on IRC, I reviewed your patch, copy-edited it a bit slightly and 
committed it.

This can definitely still be polished, but we had to start somewhere, and it's 
done. We'll adjust it as necessary.

Many thanks for your work!

-- 
Aymeric.

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



Re: startproject template context

2012-06-07 Thread Luke Plant
On 01/05/12 22:58, Ramiro Morales wrote:
> On Tue, May 1, 2012 at 11:04 AM, Sam Simmons  wrote:
>> For app/project templates I found the docs a little misleading when they say
>> 'Any option passed to the startapp command' will be added to the context.
> 
> You' ve understood the documentation correctly. This is a feature that is
> currently (and AFAICT was DOA) not fully implemented because there is code to
> process and add the custom command line switches to the context but the
> validation that the management commands framework performs on the command line
> options isn't allowing them to pass.

This sounds like it would be a misfeature.

It is a golden rule of command line apps that typos in options passed to
the command are not silently ignored.

If I type:

django-admin.py startproject --tempate=/home/me/foo

I should get an exception, not the default template rendered with
"tempate=/home/me/foo".

The feature given in the patch on ticket #18277, is however, much more
like it - you have to pass the context using --add-context

Regards,

Luke

-- 
OSBORN'S LAW
Variables won't, constants aren't.

Luke Plant || http://lukeplant.me.uk/

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



Re: Will django escaping ever consider context of javascript and CSS?

2012-06-07 Thread Luke Plant
On 03/05/12 17:49, Voulnet wrote:
> The document you linked says it doesn't make it safe to use, but rather
> helps in fixing syntax errors.
> 
> " escapejs
> 
> Escapes characters for use in JavaScript strings. This does not make the
> string safe for use in HTML, but does protect you from syntax errors
> when using templates to generate JavaScript/JSON."

This means that it is not safe for use *in HTML*. It does guarantee that
all the data ends up as a single javascript string literal, but that
javascript string will still need HTML escaping if you are planning on
inserting it in the DOM. This needs to be done using a javascript escape
function (not provided).

We've looked at custom escape mechanisms in the past. There are big
difficulties due to the fact that builtin filters only work correctly
with the context of HTML escaping.

Some relevant previous discussions:

http://goo.gl/XZ7Pt

http://goo.gl/T8tkx

Luke

-- 
OSBORN'S LAW
Variables won't, constants aren't.

Luke Plant || http://lukeplant.me.uk/

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



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

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

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

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

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

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

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

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

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

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

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

 - Anssi

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



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

2012-06-07 Thread Florian Apolloner
Hi,

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

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

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

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

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

Cheers,
Florian

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