Re: ORA-01882 [was Re: looking up date as str (fails under Oracle, Ticket #20015)]

2013-05-18 Thread Shai Berger
Oopsie:

On Sunday 19 May 2013 08:12:12 Shai Berger wrote:
>  ...They do pass on our CI [0],...

http://ci.djangoproject.com/job/Django%20Oracle/lastCompletedBuild/database=oracle,python=python2.7/

Thanks,
Shai.

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




ORA-01882 [was Re: looking up date as str (fails under Oracle, Ticket #20015)]

2013-05-18 Thread Shai Berger
On Saturday 18 May 2013 19:49:58 Shai Berger wrote:
> 
> I'm toying with a different solution now -- removing the date casting on
> Oracle too. It passed the lookup tests, now I'm running the whole suite
> (that takes a little time). After seeing that no other backend in core
> does it, and as we do set the timestamp format, I wonder if it is really
> necessary. If that works, it's a pretty solution that affects Oracle only.
> 
Running over the entire Django test-suite, the only difference that is made by 
removing the cast-to-date on Oracle is that now test_lookup_date_as_str 
passes; so I'm very much inclined to do that.

However, I have a little problem in my testing environment: I keep getting 
"ora-01882 timezone region not found" for 5 TZ-related tests. As these are 
all, of course, datetime-related tests, I'd like to get them to pass. They do 
pass on our CI [0], and the mailing list includes mentions of running into 
related problems. What I see when I investigate is: The test specifies the 
timezone as "Africa/Nairobi", which would have been fine, exceot that by the 
time it gets into the SQL to be executed, it is transformed to "EAT", which 
(rightfully) triggers the error.

Florian, Aymeric, or any Django/Oracle user in Kenya: How did you overcome 
this?

Thanks,
Shai.


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




Ticket #9321 -- Modelform widgets help text for ManyToMany fields

2013-05-18 Thread Ramiro Morales
Hi all,

This is a proposal for fixing this small and old issue
(https://code.djangoproject.com/ticket/9321):

Model forms for models that include a ManyToMany field get a hard-coded

"*Hold down "Control", or "Command" on a Mac, to select more than one."

sentence in the respective help text

This sentence can be either:
 a) Forced as the HTML field help text if the user didn't provide a
help_text or
 b) Appended to the user's one.

Emphasis should be put on the fact that this only affects:

* Model forms
* ...for models with at least one m2m field
* ...for which custom form fields/widgets are being used where the user
  interaction doesn't actually involve using a visual selection
  expandable by mouse click and key presses combinations. Or designs
  where the developer simply doesn't want that string appearing on their
  form at all.

The proposed fix is somewhat simple (I'd say trivial) compared
to the ones proposed so far and has the following implementation
timeline:

Django 1.6:
Stop hard-coding this at the model level. Still do that at the form
field level (ModelMultipleChoiceField) but only for field instances that
are paired to a (subclass of) SelecMultiple widget.

This has the net effect that we stop forcing the hard-coded string on
every kind of form field for m2ms (the main complaint and the actual
fix of the issue at hand).

This could impact custom m2m field form fields/widgets that don't
inherit from SelectMultiple with a previously unannounced
backward-incompatible change if they rely on the help_text munging
performed by the ORM field, but OTOH would keep the greater audience
(those who use the default widgets or use a subclass of SelectMultiple
for which the hard-coded help text is relevant/useful e.g. the admin)
safe by not changing anything at all for them.

Django 1.7:
Simply stop forcing the hard-coded sentence. If users want to have it
set or added to their help text they should provide it by themselves
just like it happens with every other [model] form field.
These users are in a better position to know if it is relevant to the form
widget in use.

This deprecation cycle won't be implemented using
[Pending]DeprecationWarning's, but rather by only documentation. The
release notes and backward incompatible timeline entry for 1.6 would
warn about the changes coming in 1.7 so they can act in advance.

Reasoning for this is that we would be forced to detect field/widget
subclassing to be able to catch the cases where warnings should be
triggered and these warnings could get noisy fast. IMHO this isn't worth
the trouble.

What do you think?

Also, WIP PR: 
https://github.com/ramiro/django/compare/master...9321-m2m-help_text
(only piece missing is the warnings described in the last two paragraphs.)

-- 
Ramiro Morales
@ramiromorales

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




Re: Model field validation (of the field itself, not values)

2013-05-18 Thread Jacob Kaplan-Moss
Hey Luke -

Yup, this is indeed a problem. It's actually the subject of a Summer
of Code proposal [1], so if you can wait there's a good chance that
you won't have to do any work yourself :)

Jacob

[1] 
https://groups.google.com/forum/#!msg/django-developers/e0-rOIkrXaQ/w5aiW_R6aFYJ

On Sat, May 18, 2013 at 2:26 PM, Luke Sneeringer  wrote:
> Good afternoon,
> I am currently working on writing a small number of subclasses to
> `models.Field`. As part of doing this, I wanted to add validation to how
> those fields are instantiated, and discovered that I think it can't be done.
> Assuming I'm right, I'd like to fix it.
>
> Here's the use case. Say you want to create a Field subclass, MyField, which
> requires one or more arguments to be set at initialization. This is
> equivalent to the requirement of having max_length set on models.CharField.
> The way models.CharField works is that this validation happens when
> manage.py syncdb (or sql, or whatnot) is run.
>
> It ultimately runs through this file:
> https://github.com/django/django/blob/master/django/core/management/validation.py
>
> ...and if there's an issue, you get an error (e.g. "CharFields require a
> max_length argument that is a positive integer.")
>
> Now, it would seem to me that the appropriate way to write a well-behaved
> Field subclass would be to have my logic work the same way that the build-in
> fields do. However, I can't. Sadness. The reason I can't is because that
> get_validation_errors function is basically a black box. It checks for
> errors in the field classes included in core through a nested if, e.g.
>
> if isinstance(f, models.CharField):
> [ logic for validating CharField ]
> if isinstance(f, models.DecimalField):
> [ logic for validating DecimalField ]
>
> ...and so on and so forth. This means that any field subclasses that aren't
> created by the Django core team can't have their logic validated similarly.
>
> It seems like a better way to do this would be to have said logic be part of
> the field class itself. Why does models.CharField not have the code for how
> to determine whether a CharField instance has been instantiated properly?
> Same for DecimalField, FileField, and so on and so forth? This would then
> allow non-core fields to define that method and get the same validation.
>
> I've searched for this on Trac, but don't see a ticket about this. I also
> combed the documentation, but the issue isn't discussed. It's not clear to
> me whether or not I should be filing a ticket on Trac or sending a
> discussion to this list. I'd like to make this improvement if there'd be
> support for doing so, but I don't want to do the work only to be given an
> "in principle" no.
>
> Would this be something we would be willing to change, to allow non-core
> Field subclasses to be able to be validated during syncdb?
>
> Best Regards,
> Luke Sneeringer
>
> --
> 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?hl=en.
> 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?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Model field validation (of the field itself, not values)

2013-05-18 Thread Luke Sneeringer
Good afternoon,
I am currently working on writing a small number of subclasses to 
`models.Field`. As part of doing this, I wanted to add validation to how those 
fields are instantiated, and discovered that I think it can't be done. Assuming 
I'm right, I'd like to fix it.

Here's the use case. Say you want to create a Field subclass, MyField, which 
requires one or more arguments to be set at initialization. This is equivalent 
to the requirement of having max_length set on models.CharField. The way 
models.CharField works is that this validation happens when manage.py syncdb 
(or sql, or whatnot) is run.

It ultimately runs through this file: 
https://github.com/django/django/blob/master/django/core/management/validation.py

...and if there's an issue, you get an error (e.g. "CharFields require a 
max_length argument that is a positive integer.")

Now, it would seem to me that the appropriate way to write a well-behaved Field 
subclass would be to have my logic work the same way that the build-in fields 
do. However, I can't. Sadness. The reason I can't is because that 
get_validation_errors function is basically a black box. It checks for errors 
in the field classes included in core through a nested if, e.g.

if isinstance(f, models.CharField):
[ logic for validating CharField ]
if isinstance(f, models.DecimalField):
[ logic for validating DecimalField ]

...and so on and so forth. This means that any field subclasses that aren't 
created by the Django core team can't have their logic validated similarly.

It seems like a better way to do this would be to have said logic be part of 
the field class itself. Why does models.CharField not have the code for how to 
determine whether a CharField instance has been instantiated properly? Same for 
DecimalField, FileField, and so on and so forth? This would then allow non-core 
fields to define that method and get the same validation.

I've searched for this on Trac, but don't see a ticket about this. I also 
combed the documentation, but the issue isn't discussed. It's not clear to me 
whether or not I should be filing a ticket on Trac or sending a discussion to 
this list. I'd like to make this improvement if there'd be support for doing 
so, but I don't want to do the work only to be given an "in principle" no.

Would this be something we would be willing to change, to allow non-core Field 
subclasses to be able to be validated during syncdb?

Best Regards,
Luke Sneeringer

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




Re: Anyone have ideas on #16550 - custom SQL before/after syncdb?

2013-05-18 Thread Donald Stufft
There's already a patch on the ticket tracker for a pre_syncdb signal, and 
yesterday I started updating it and modifying it a bit as I needed this signal.

https://github.com/dstufft/django/compare/pre-syncdb-signal

On May 18, 2013, at 1:06 PM, Karol Sikora  wrote:

> I can try to implement approach with pre_syncdb signal tomorrow. I think that 
> is quite enough solution before deeper diggling into new migrations framework.
> 
> Karol
> 
> 18 maj 2013 19:03, "Anssi Kääriäinen"  napisał(a):
> On 16 touko, 11:20, Danilo Bargen  wrote:
> > As a sidenote, there was a discussion about this on this mailing list a few
> > months ago:
> >
> > https://groups.google.com/forum/#!searchin/django-developers/16550/dj...
> 
> I think a pre_sync signal is best approach. The signal should be
> called either just after connecting to the (test) DB in syncdb
> command, or maybe just after existing tables have been introspected so
> that you know what tables are already there. Latter might be better as
> syncdb can be ran multiple times, and you only need to CREATE
> EXTENSION on initial sync. OTOH if you add one more model with
> JSONField it seems you would need to run another CREATE EXTENSION, or
> to investigate if some existing model has already ran CREATE
> EXTENSION. So, defensive coding (that is, CREATE EXTENSION IF NOT
> EXISTS) would be needed.
> 
> Another problem is that post_syncdb is called from flush command, too.
> This seems wrong. Could we just add post_flush signal and send that
> instead? Another option is to add a "for_flush" kwarg to the signal
> parameters, but it feels just so wrong to have post_syncdb signal with
> an argument that tells syncdb didn't happen after all. The reason for
> post_syncdb from flush is creation of ContentTypes and Permissions
> after truncation of those tables.
> 
> While the pre_syncdb signal approach has many shortcomings (how to
> include the output in sqlall?), I think it is enough to solve this
> problem for now. You can run CREATE EXTENSION IF NOT EXISTS and that
> seems already a big step forward. Also, distinguishing post_flush from
> post_syncdb seems like a good idea, but should be done in separate
> ticket.
> 
> Later on migrations framework could offer a much better solution to
> this. But pre_syncdb signal seems small enough to include already in
> 1.6. Maybe this could be done in the sprints?
> 
>  - Anssi
> 
> --
> 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?hl=en.
> 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?hl=en.
> For more options, visit https://groups.google.com/groups/opt_out.
>  
>  


-
Donald Stufft
PGP: 0x6E3CBCE93372DCFA // 7C6B 7C5D 5E2B 6356 A926 F04F 6E3C BCE9 3372 DCFA



signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: Anyone have ideas on #16550 - custom SQL before/after syncdb?

2013-05-18 Thread Karol Sikora
I can try to implement approach with pre_syncdb signal tomorrow. I think
that is quite enough solution before deeper diggling into new migrations
framework.

Karol
18 maj 2013 19:03, "Anssi Kääriäinen"  napisał(a):

> On 16 touko, 11:20, Danilo Bargen  wrote:
> > As a sidenote, there was a discussion about this on this mailing list a
> few
> > months ago:
> >
> > https://groups.google.com/forum/#!searchin/django-developers/16550/dj...
>
> I think a pre_sync signal is best approach. The signal should be
> called either just after connecting to the (test) DB in syncdb
> command, or maybe just after existing tables have been introspected so
> that you know what tables are already there. Latter might be better as
> syncdb can be ran multiple times, and you only need to CREATE
> EXTENSION on initial sync. OTOH if you add one more model with
> JSONField it seems you would need to run another CREATE EXTENSION, or
> to investigate if some existing model has already ran CREATE
> EXTENSION. So, defensive coding (that is, CREATE EXTENSION IF NOT
> EXISTS) would be needed.
>
> Another problem is that post_syncdb is called from flush command, too.
> This seems wrong. Could we just add post_flush signal and send that
> instead? Another option is to add a "for_flush" kwarg to the signal
> parameters, but it feels just so wrong to have post_syncdb signal with
> an argument that tells syncdb didn't happen after all. The reason for
> post_syncdb from flush is creation of ContentTypes and Permissions
> after truncation of those tables.
>
> While the pre_syncdb signal approach has many shortcomings (how to
> include the output in sqlall?), I think it is enough to solve this
> problem for now. You can run CREATE EXTENSION IF NOT EXISTS and that
> seems already a big step forward. Also, distinguishing post_flush from
> post_syncdb seems like a good idea, but should be done in separate
> ticket.
>
> Later on migrations framework could offer a much better solution to
> this. But pre_syncdb signal seems small enough to include already in
> 1.6. Maybe this could be done in the sprints?
>
>  - Anssi
>
> --
> 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?hl=en
> .
> 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?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: Anyone have ideas on #16550 - custom SQL before/after syncdb?

2013-05-18 Thread Anssi Kääriäinen
On 16 touko, 11:20, Danilo Bargen  wrote:
> As a sidenote, there was a discussion about this on this mailing list a few
> months ago:
>
> https://groups.google.com/forum/#!searchin/django-developers/16550/dj...

I think a pre_sync signal is best approach. The signal should be
called either just after connecting to the (test) DB in syncdb
command, or maybe just after existing tables have been introspected so
that you know what tables are already there. Latter might be better as
syncdb can be ran multiple times, and you only need to CREATE
EXTENSION on initial sync. OTOH if you add one more model with
JSONField it seems you would need to run another CREATE EXTENSION, or
to investigate if some existing model has already ran CREATE
EXTENSION. So, defensive coding (that is, CREATE EXTENSION IF NOT
EXISTS) would be needed.

Another problem is that post_syncdb is called from flush command, too.
This seems wrong. Could we just add post_flush signal and send that
instead? Another option is to add a "for_flush" kwarg to the signal
parameters, but it feels just so wrong to have post_syncdb signal with
an argument that tells syncdb didn't happen after all. The reason for
post_syncdb from flush is creation of ContentTypes and Permissions
after truncation of those tables.

While the pre_syncdb signal approach has many shortcomings (how to
include the output in sqlall?), I think it is enough to solve this
problem for now. You can run CREATE EXTENSION IF NOT EXISTS and that
seems already a big step forward. Also, distinguishing post_flush from
post_syncdb seems like a good idea, but should be done in separate
ticket.

Later on migrations framework could offer a much better solution to
this. But pre_syncdb signal seems small enough to include already in
1.6. Maybe this could be done in the sprints?

 - Anssi

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




Re: looking up date as str (fails under Oracle, Ticket #20015)

2013-05-18 Thread Shai Berger
On Saturday 18 May 2013 19:35:31 Anssi Kääriäinen wrote:
> On 18 touko, 17:46, Shai Berger  wrote:
> > 
> > 1) The fixes I see would affect all backends, but AFAIK only Oracle is
> > reported as failing the test; anyone wants to comment on this? Do all
> > other backends just not need a cast_to_date operation?
> > 
> > 2) Should I fix it in the ugly ways, or look for a more general solution,
> > which will involve deeper changes?
> 
> I don't think it is necessary to do deeper changes. My interpretation
> of the current lookup system is that it has been stretched to its
> limits and needs to be refactored. I intended to do that already for
> the 1.6 release but ran out of steam.
> 
> Of course, if you want to do improvements to current code that isn't
> forbidden... But don't feel obligated to do so.
> 
> I am not sure about question 1 except that changing the backend API to
> make Oracle work is OK.
> 
I'm toying with a different solution now -- removing the date casting on Oracle 
too. It passed the lookup tests, now I'm running the whole suite (that takes a 
little time). After seeing that no other backend in core does it, and as we do 
set the timestamp format, I wonder if it is really necessary. If that works, 
it's a pretty solution that affects Oracle only.

Shai.

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




Re: Making __repr__ safe by default

2013-05-18 Thread Anssi Kääriäinen
On 18 touko, 19:04, Patryk Zawadzki  wrote:
> As suggested by Marc Tamlyn I am posting this here for discussion:
>
> https://code.djangoproject.com/ticket/20448
>
> tl;dr:
>
> Currently __repr__() calls __unicode__() which can be a very bad
> thing. You really don't want things like an exception being raised or
> debugger being used to cause additional side effects (like executing a
> database query inside __unicode__).

There was very similar discussion recently (maybe in some ticket, I
don't remember where) about QuerySet.__repr__(). IIRC the conclusion
was that executing SELECT queries as part of __repr__() is OK. The
reasoning had four points:
  1. This is the way it has always been
  2. Due to #1, changes in __repr__() will cascade to a lot of
tutorials etc which use the current repr format.
  3. SELECT queries should be safe (for QuerySet SELECT FOR UPDATE not
so safe...)
  4. __repr__() is more informative the current way

So, changing the way Model.__repr__() works should consider those four
points. To me it seems that points #1 - #3 apply directly, maybe also
#4.

I think we can keep things as is. I do see that the current way can be
problematic in some cases, but at least for Model.__unicode__(), if
you know it is unsafe, then write your own safe __repr__(). And if it
is decided that this change is after all needed, then doing changes to
both QuerySet.__repr__() and Model.__repr__() should be done in one
go.

 - Anssi

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




Re: looking up date as str (fails under Oracle, Ticket #20015)

2013-05-18 Thread Anssi Kääriäinen
On 18 touko, 17:46, Shai Berger  wrote:
> Hi django devs,
>
> While going through Oracle bugs, I ran into the ticket in the subject[0]. The
> problem there is that current code assumes that whenever we want to compare
> anything (pretty much) against a datetime column, in any way, we'd like to
> compare the column value and the given value as datetimes. Of course, in the
> case of lookup_date_as_str -- lookup types like 'startswith' -- this is not
> the case.
>
> I want to fix things so that for the relevant lookup types, the comparison 
> will
> not try to force anything into a date. I see at least two ways to do that, but
> they share ugliness: They take a place in general code which special-cases
> datetimes, and add an extra special-casing for the lookup types. The fix (in
> each of the ways I see) is less than 5 lines of code, but, euh.
>
> So, two questions arise:
>
> 1) The fixes I see would affect all backends, but AFAIK only Oracle is 
> reported
> as failing the test; anyone wants to comment on this? Do all other backends
> just not need a cast_to_date operation?
>
> 2) Should I fix it in the ugly ways, or look for a more general solution, 
> which
> will involve deeper changes?

I don't think it is necessary to do deeper changes. My interpretation
of the current lookup system is that it has been stretched to its
limits and needs to be refactored. I intended to do that already for
the 1.6 release but ran out of steam.

Of course, if you want to do improvements to current code that isn't
forbidden... But don't feel obligated to do so.

I am not sure about question 1 except that changing the backend API to
make Oracle work is OK.

 - Anssi

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




Re: test discovery

2013-05-18 Thread Chris Wilson

Hi Carl,

On Sat, 18 May 2013, Carl Meyer wrote:

I don't think this should be fixed in the test runner itself; in 
general, file-path test labels _should_ be interpreted as relative to 
wherever you are running the tests from.


But it should be fixed in the 
test_runner.test_discover_runner.DiscoverRunnerTest.test_file_path test 
- that test apparently needs to isolate itself better by setting the CWD 
for the duration of the test, or something similar. Mind filing a bug? I 
should be able to take a look soon.


Done, thanks: https://code.djangoproject.com/ticket/20449

Cheers, Chris.
--
Aptivate | http://www.aptivate.org | Phone: +44 1223 967 838
Future Business, Cam City FC, Milton Rd, Cambridge, CB4 1UY, UK

Aptivate is a not-for-profit company registered in England and Wales
with company number 04980791.

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




Making __repr__ safe by default

2013-05-18 Thread Patryk Zawadzki
As suggested by Marc Tamlyn I am posting this here for discussion:

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

tl;dr:

Currently __repr__() calls __unicode__() which can be a very bad
thing. You really don't want things like an exception being raised or
debugger being used to cause additional side effects (like executing a
database query inside __unicode__).

-- 
Patryk Zawadzki
I solve problems.

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




Re: test discovery

2013-05-18 Thread Carl Meyer
Hi Chris,

On May 18, 2013, at 8:34 AM, Chris Wilson  wrote:

> On Sat, 18 May 2013, Chris Wilson wrote:
> 
>> I think Travis is unhappy about something in this commit. Any ideas?
>> 
>> ==
>> ERROR: test_file_path (test_runner.test_discover_runner.DiscoverRunnerTest)
>> --
>> Traceback (most recent call last):
>> File 
>> "/home/travis/build/aptivate/django/tests/test_runner/test_discover_runner.py",
>>  line 65, in test_file_path
>>   ["test_discovery_sample/"],
>> File "/home/travis/build/aptivate/django/django/test/runner.py", line 63, in 
>> build_suite
>>   tests = self.test_loader.loadTestsFromName(label)
>> File "/usr/lib/python2.7/unittest/loader.py", line 91, in loadTestsFromName
>>   module = __import__('.'.join(parts_copy))
>> ImportError: Import by filename is not supported.
>> 
>> 
> 
> It seems to be checking if a file exists, using a relative path, in 
> django/test/runner.py:65:
> 
>if not os.path.exists(label_as_path):
>tests = self.test_loader.loadTestsFromName(label)
> 
> And that will behave differently depending if you run your tests from inside 
> the tests directory like this:
> 
>./runtests.py -v2 --settings=test_postgres_nogis test_runner
> 
> Or from the parent directory, as Travis currently does, like this:
> 
>python -Wall tests/runtests.py --selenium --verbosity 2 \
>--settings=django_settings
> 
> I can work around it in Travis by changing working directory before running 
> the tests, but is it worth fixing this in the test runner, perhaps using an 
> absolute path based on __file__?

I don't think this should be fixed in the test runner itself; in general, 
file-path test labels _should_ be interpreted as relative to wherever you are 
running the tests from. 

But it should be fixed in the 
test_runner.test_discover_runner.DiscoverRunnerTest.test_file_path test - that 
test apparently needs to isolate itself better by setting the CWD for the 
duration of the test, or something similar. Mind filing a bug? I should be able 
to take a look soon.

Carl

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




Re: test discovery

2013-05-18 Thread Carl Meyer
Hi Chris,

On May 18, 2013, at 9:42 AM, Chris Wilson  wrote:
> Another odd behaviour of the new test runner. This runs the tests, but fails 
> to add the test app to INSTALLED_APPS, so they all fail because their tables 
> are not created:
> 
>PYTHONPATH=.. python -Wall runtests.py --selenium --verbosity 2 \
>--settings=tests.travis_configs.test_postgres_nogis \
>tests.transactions.tests.AtomicInsideTransactionTests
> 
> Changing the spec to remove the initial "tests." results in the app being 
> loaded and the tests run:
> 
>PYTHONPATH=.. python -Wall runtests.py --selenium --verbosity 2 \
>--settings=tests.travis_configs.test_postgres_nogis \
>transactions.tests.AtomicInsideTransactionTests
> 
> Is this expected behaviour? It's rather counter-intuitive to me.

The initial "tests." prefix shouldn't be used, and wasn't intended to be 
supported. There is no __init__.py in the tests/ directory, and when running 
runtests.py the tests/ directory itself is placed on sys.path, so every module 
in there is a top-level module when running tests. I'm actually surprised 
unittest itself even runs the tests when specified that way, given the lack of 
__init__.py.

That said, if this turns out to to be something that trips a lot of people up, 
it would be trivial to add a special-case in runtests.py to look for and remove 
the initial "tests." in provided test labels.

Carl

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




looking up date as str (fails under Oracle, Ticket #20015)

2013-05-18 Thread Shai Berger
Hi django devs,

While going through Oracle bugs, I ran into the ticket in the subject[0]. The 
problem there is that current code assumes that whenever we want to compare 
anything (pretty much) against a datetime column, in any way, we'd like to 
compare the column value and the given value as datetimes. Of course, in the 
case of lookup_date_as_str -- lookup types like 'startswith' -- this is not 
the case.

I want to fix things so that for the relevant lookup types, the comparison will 
not try to force anything into a date. I see at least two ways to do that, but 
they share ugliness: They take a place in general code which special-cases 
datetimes, and add an extra special-casing for the lookup types. The fix (in 
each of the ways I see) is less than 5 lines of code, but, euh.

So, two questions arise:

1) The fixes I see would affect all backends, but AFAIK only Oracle is reported 
as failing the test; anyone wants to comment on this? Do all other backends 
just not need a cast_to_date operation?

2) Should I fix it in the ugly ways, or look for a more general solution, which 
will involve deeper changes?

Thanks,
Shai.

[0] https://code.djangoproject.com/ticket/20015

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




Re: RFC: "universal" view decorators

2013-05-18 Thread Donald Stufft

On May 18, 2013, at 8:57 AM, Marc Tamlyn  wrote:

> I'm going to resurrect this old thread as I'd like to get it resolved in some 
> fashion.
> 
> I used to be very in favour of the class decorators approach. I've been using 
> an implementation of `@class_view_decorator(func)` for some time and it works 
> pretty well. That said my implementation at least was subject to a notable 
> flaw which is that if multiple parts of the hierarchy have the same decorator 
> applied to them then the check gets done twice. Mixins are much cleverer than 
> this because of MRO, so we avoid that problem.
> 
> Mixins however have their own issues - it's "harder" (for some definition) to 
> ensure that all of your top-level permission checking happening in the 
> correct order. That said, I am certainly veering towards implementing this 
> using mixins (for each piece of functionality).
> 
> I'd really like to have a look at Donald's implementation, sadly it seems to 
> no longer be on github. Do you still have then code somewhere, or can you 
> explain the implementation idea?

I'm sure I have it somewhere, but knowing where is another thing all together ;)

If I recall I made a mixin superclass that added an ``as_decorator()`` class 
function which functioned similarly to the ``as_view()`` function. It took a 
common entry point and generated a non class function based on that and 
returned it.

As far as the actual mixins themselves I'd take a look at django-braces which 
has implementations of a lot of the decorators as mixins already they'd just 
need maybe some tweaking and then code added to allow turning them into actual 
decorators.

> 
> Marc
> 
> 
> On Thursday, 15 September 2011 22:44:39 UTC+2, Jacob Kaplan-Moss wrote:
> Hi folks --
> I'd like to convert all the view decorators built into Django to be
> "universal" -- so they'll work to decorate *any* view, whether a
> function, method, or class. I believe I've figured out a technique for
> this, but I'd like some feedback on the approach before I dive in too
> deep.
> 
> Right now view decorators (e.g. @login_required) only work on
> functions. If you want to use a decorator on a method then you need to
> "convert" the decorator using method_decorator(original_decorator).
> You can't use view decorators on class-based views at all. This means
> making a class-based view require login requires this awesomeness::
> 
> class MyView(View):
> @method_decorator(login_required)
> def dispatch(self, *args, **kwargs):
> return super(MyView, self.dispatch(*args, **kwargs)
> 
> This makes me sad. It's really counter-intuitive and relies on a
> recognizing that functions and methods are different to even know to
> look for method_decorator.
> 
> #14512 proposes a adding another view-decorator-factory for decorating
> class-based views, which would turn the above into::
> 
> @class_view_decorator(login_required)
> class MyView(View):
> ...
> 
> This makes me less sad, but still sad. Factory functions. Ugh.
> 
> I want @login_required to work for anything::
> 
> @login_required
> class MyView(View):
> ...
> 
> class Something(object):
> @login_required
> def my_view(self, request):
> ...
> 
> @login_required
> def my_view(request):
> ...
> 
> 
> Now, back in the day we somewhat had this: decorators tried to work
> with both functions and methods. The technique turned out not to work
> (see #12804) and was removed in [12399]. See
> http://groups.google.com/group/django-developers/browse_thread/thread/3024b14a47f5404d
> for the discussion.
> 
> I believe, however, I've figured out a different technique to make
> this work: don't try to detect bound versus unbound methods, but
> instead look for the HttpRequest object. It'll either be args[0] if
> the view's a function, or args[1] if the view's a method. This
> technique won't work for any old decorator, but it *will* work (I
> think) for any decorator *designed to be applied only to views*.
> 
> I've written a proof-of-concept patch to @login_required (well,
> @user_passes_test, actually):
> 
> https://gist.github.com/1220375
> 
> The test suite passes with this, with one exception:
> https://code.djangoproject.com/browser/django/trunk/tests/regressiontests/decorators/tests.py#L87.
> I maintain that this test is broken and should be using RequestFactory
> instead.
> 
> Can I get some thoughts on this technique and some feedback on whether
> it's OK to apply to every decorator built into Django?
> 
> Jacob
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> -- 
> 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 

Re: BCrypt + Python3

2013-05-18 Thread Donald Stufft

On May 18, 2013, at 5:15 AM, Aymeric Augustin 
 wrote:

> Apologies for answering so late. I see the change discussed here was already 
> committed. The change itself is fine — essentially because it's limited to 
> the bcrypt password hasher — but I'd like to bring some perspective to parts 
> of this discussion.
> 
> Overall, I strongly advocate consistency in the Python ecosystem, and the 
> standard library sets the, err, standard. Here's how it deals with this 
> situation in Python 3.
> 
 import hashlib
> 
> 1) Hash functions must reject str objects because the encoding isn't 
> guaranteed:
> 
 hashlib.md5('foo')
> Traceback (most recent call last):
>  File "", line 1, in 
> TypeError: Unicode-objects must be encoded before hashing
> 
> 2) Digests must be returned as bytes (quite obviously):
> 
 hashlib.md5(b'foo').digest()
> b'\xac\xbd\x18\xdbL\xc2\xf8\\\xed\xefeO\xcc\xc4\xa4\xd8'
> 
> 3) Hex digests must be returned as str:
> 
 hashlib.md5(b'foo').hexdigest()
> 'acbd18db4cc2f85cedef654fccc4a4d8'
> 
> Adapting this example to Python 2 is left as an exercise :)
> 
> As a consequence, I agree with Claude's recommendation to use unicode strings 
> whenever possible (eg. for hex digests). However, I believe that a simple 
> hash function mustn't accept unicode strings. Wrappers — say, an 
> make_password_hash function — must encode unicode strings to bytes before 
> passing them to hash functions.
> 
> Regarding Donald's pull request, `data = force_bytes(data)` makes sense, 
> because the hasher must be fed bytes. There's already a `password = 
> force_bytes(password)` just above.
> 
> I'm less enthusiastic about the change adding `force_text(data)`. It actually 
> works around bcrypt.hashpw returning an unexpected type in these 
> circumstance. But, if that's how bcrypt.hashpw works, that's fine.

Well the python library returns bytes (and accepts bytes for the salt) because 
fundamentally bcrypt operates on bytes, and the C library reflects that. The 
force_text would need to happen either in Django or in the Python library and I 
believe it's more appropriate for it to happen in Django.

> 
> Donald, we've discussed this before and I know you have strong feelings 
> against the design of the standard library in this regard. Still, Python is 
> the environment we're living in, and we shouldn't fight it.
> 
> -- 
> 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?hl=en.
> For more options, visit https://groups.google.com/groups/opt_out.
> 
> 


-
Donald Stufft
PGP: 0x6E3CBCE93372DCFA // 7C6B 7C5D 5E2B 6356 A926 F04F 6E3C BCE9 3372 DCFA



signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: test discovery

2013-05-18 Thread Chris Wilson

Hi all,

On Fri, 10 May 2013, Carl Meyer wrote:

I merged this patch tonight. Thanks to everyone who contributed! Now 
let's see how the CI servers feel about it...


I think Travis is unhappy about something in this commit. Any ideas?

==
ERROR: test_file_path 
(test_runner.test_discover_runner.DiscoverRunnerTest)

--
Traceback (most recent call last):
  File 
"/home/travis/build/aptivate/django/tests/test_runner/test_discover_runner.py", 
line 65, in test_file_path

["test_discovery_sample/"],
  File "/home/travis/build/aptivate/django/django/test/runner.py", line 
63, in build_suite

tests = self.test_loader.loadTestsFromName(label)
  File "/usr/lib/python2.7/unittest/loader.py", line 91, in 
loadTestsFromName

module = __import__('.'.join(parts_copy))
ImportError: Import by filename is not supported.



Cheers, Chris.
--
Aptivate | http://www.aptivate.org | Phone: +44 1223 967 838
Future Business, Cam City FC, Milton Rd, Cambridge, CB4 1UY, UK

Aptivate is a not-for-profit company registered in England and Wales
with company number 04980791.

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




deprecating ipaddressfield (at some time)

2013-05-18 Thread Erik Romijn
Hello all,

Since Django 1.4, we've added GenericIPAddressField, next to IPAddressField. 
The new GenericIPAddressField supports IPv4 as well as IPv6 addresses, and does 
normalisation of IPv6 addresses. It can also be configured to only accept IPv4 
or IPv6 addresses.

As far as I know, IPAddressField has no current features that are not also 
available in a GenericIPAddressField. Therefore, I suggest that we, some time 
from now, deprecate IPAddressField, in favour of GenericIPAddressField.

For users, it is database-dependent whether IPAddressFields can just be 
replaced with GenericIPAddressFields: on PostgreSQL and SQLite, no changes are 
needed; schema changes are needed on MySQL and Oracle. Examples are listed in 
the 1.6 release notes[1], as we just made the same change for comments.

Is there anyone who sees any issues with this? I'm also not sure what timeline 
would be appropriate.

I have created ticket #20439[2] to keep track of this issue.

cheers,
Erik

[1] 
https://docs.djangoproject.com/en/dev/releases/1.6/#storage-of-ip-addresses-in-the-comments-app
[2] https://code.djangoproject.com/ticket/20439

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




Proposal: better support for generating rest apis in django core

2013-05-18 Thread Karol Sikora
Hi,

We was talked with Russell on djangocon eu about integrating more rich 
support for working django as rest api provider, focused on dealing with 
one-page web applications.
The motivations that currently without third party modules like 
django-rest-framework or tastypie its quite impossible to create rich web 
applications working as one-page.
As django aims to follow "batteries included" philosophy, so maybe it will 
be a good idea to integrate support for dealing with modern web 
applications.

Better rest apis support conists from two parts: models(and other data 
sources) serialization and handlers.
Django has currently limited support for simple serialization in 
django.core.serializers, but its quite useless in creating rest apis.
Ideally serialization support should have:
* possibility to serialize data to any format by plugabble serializers(in 
core i thing xml and json should be included),
* great integration with django models through eg, adding mixin class to 
model class definition
* possibility to integrate with other data sources(approach with appending 
mixin to class with data fields seems to be good for dealing with this case)
* possibility to provide data mutators as method in api class
* permissions mangement using user object

I propose approach to create class which can work either as mixin to model 
classes or as standalone base for other data sources. In this object we can 
configure serializer class, set up permissions and mutator methods.
This class should have endpoint method like rest_get(self, user, *args, 
**kwargs), which will be called from view/other to get serialized data, 
rest_get, rest_post, rest_put and so on(getting request as first parameter 
i think).
Validations should be integrated through forms I think, its quite mature 
and stable soultion, I dont see any argument to build custom solution.
Serializers should take care on privileges checking, as its coupled with 
user models. As we provide deep integrations with models it seems to be a 
good solution that follows "fat models" approach.

Next thing is dealing with http request. We propose to use CBV and based on 
them create light wrappers to define handled http verbs, provide serializer 
class and errors handling.

The question is, what You're thinking about whole concpet(maybe we won't 
integrate rest api supports in django core and redirect peoples to use 
django-rest-framerwork and similar projects), maybe my ideas are good but 
You find some holes on it?

We're read to implement this solution and maintain them in the future.

Waiting for Your opinion!

Karol




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




Re: BCrypt + Python3

2013-05-18 Thread Aymeric Augustin
Apologies for answering so late. I see the change discussed here was already 
committed. The change itself is fine — essentially because it's limited to the 
bcrypt password hasher — but I'd like to bring some perspective to parts of 
this discussion.

Overall, I strongly advocate consistency in the Python ecosystem, and the 
standard library sets the, err, standard. Here's how it deals with this 
situation in Python 3.

>>> import hashlib

1) Hash functions must reject str objects because the encoding isn't guaranteed:

>>> hashlib.md5('foo')
Traceback (most recent call last):
  File "", line 1, in 
TypeError: Unicode-objects must be encoded before hashing

2) Digests must be returned as bytes (quite obviously):

>>> hashlib.md5(b'foo').digest()
b'\xac\xbd\x18\xdbL\xc2\xf8\\\xed\xefeO\xcc\xc4\xa4\xd8'

3) Hex digests must be returned as str:

>>> hashlib.md5(b'foo').hexdigest()
'acbd18db4cc2f85cedef654fccc4a4d8'

Adapting this example to Python 2 is left as an exercise :)

As a consequence, I agree with Claude's recommendation to use unicode strings 
whenever possible (eg. for hex digests). However, I believe that a simple hash 
function mustn't accept unicode strings. Wrappers — say, an make_password_hash 
function — must encode unicode strings to bytes before passing them to hash 
functions.

Regarding Donald's pull request, `data = force_bytes(data)` makes sense, 
because the hasher must be fed bytes. There's already a `password = 
force_bytes(password)` just above.

I'm less enthusiastic about the change adding `force_text(data)`. It actually 
works around bcrypt.hashpw returning an unexpected type in these circumstance. 
But, if that's how bcrypt.hashpw works, that's fine.

Donald, we've discussed this before and I know you have strong feelings against 
the design of the standard library in this regard. Still, Python is the 
environment we're living in, and we shouldn't fight it.

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