Re: Custom FilterSpecs #5833 planned for Django 1.1?
On Feb 7, 12:35 pm, Alex Gaynor wrote: > On Sat, Feb 7, 2009 at 8:27 AM, Ben Gerdemann wrote: > > A couple things, first the patch still has a pair of TODO comments, so > either those comments are no longer applicable, or what they refer to should > be fixed. Secondly, it needs docs and tests. I'm not a core dev so I can't > speak to whether the approach taken is the right one(I notice it changed > radically from Honza's original patches), however those items are going to > be a baseline for whatever patch is accepted. I updated the patch to the latest HEAD and fixed the TODO comments which were both validation items. I was going to start writing some tests by basing them off of tests for the existing admin filterspecs, but I couldn't find any. The closest I could find was in admin_views, which checks some basic error conditions. Is there somewhere else I should look? In order to pass the existing tests, I had to disable the use of custom GET params. In the original patch, the filterspec could consume a custom param and then return an arbitrary queryset based on this parameter, but to do this it silently removed the parameters that don't match a field name from the request. The problem is that there is a test to insure that any parameters that do not match field names are handled by forwarding to ?e=1 (testIncorrectLookupParameters) which failed when the previous patch silently removed them. Additionally, the code in the previous patch didn't handle filters on M2M relationships through an intermediate table because it assumed the first part of the search param would be a field name. Anyway, before we got too far with this specific implementation, perhaps we need some developer input if this is a good way to proceed. Being able to use custom GET parameters to filter would be a very powerful feature and it would disappointing if we could figure out a way to do it. Cheers, Ben --~--~-~--~~~---~--~~ 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: Custom FilterSpecs #5833 planned for Django 1.1?
On Sun, Feb 8, 2009 at 10:28 AM, Ben Gerdemann wrote: > > On Feb 7, 12:35 pm, Alex Gaynor wrote: > > On Sat, Feb 7, 2009 at 8:27 AM, Ben Gerdemann wrote: > > > > A couple things, first the patch still has a pair of TODO comments, so > > either those comments are no longer applicable, or what they refer to > should > > be fixed. Secondly, it needs docs and tests. I'm not a core dev so I > can't > > speak to whether the approach taken is the right one(I notice it changed > > radically from Honza's original patches), however those items are going > to > > be a baseline for whatever patch is accepted. > > I updated the patch to the latest HEAD and fixed the TODO comments > which were both validation items. I was going to start writing some > tests by basing them off of tests for the existing admin filterspecs, > but I couldn't find any. The closest I could find was in admin_views, > which checks some basic error conditions. Is there somewhere else I > should look? > > In order to pass the existing tests, I had to disable the use of > custom GET params. In the original patch, the filterspec could consume > a custom param and then return an arbitrary queryset based on this > parameter, but to do this it silently removed the parameters that > don't match a field name from the request. The problem is that there > is a test to insure that any parameters that do not match field names > are handled by forwarding to ?e=1 (testIncorrectLookupParameters) > which failed when the previous patch silently removed them. > Additionally, the code in the previous patch didn't handle filters on > M2M relationships through an intermediate table because it assumed the > first part of the search param would be a field name. > > Anyway, before we got too far with this specific implementation, > perhaps we need some developer input if this is a good way to proceed. > Being able to use custom GET parameters to filter would be a very > powerful feature and it would disappointing if we could figure out a > way to do it. > I don't have time to devote to this right now, but I'd suggest taking a look at the svn history of the tests that check for the ?e=1 redirect. I have a vague recollection that it/they may be there as a result of a problem where incorrect lookup parameters caused an exception, and when that was fixed the test was added to ensure invalid lookups didn't cause the admin to generate a Server Error. I think the main goal of the test is to ensure no exception was raised, not necessarily to lock in place the use of the ?e=1 redirect as a way of handling the situation. If you've got something more useful to do with GET parameters that don't match field names than do a cryptic (and often huh?-inducing, since first it is easy to not notice what's happened and second it's not exactly crystal clear even if you notice the ?e=1 in the url in the address bar) redirect to ?e=1, I wouldn't necessarily think a test that checks for that ?e=1 redirect should stand in the way of that. Karen --~--~-~--~~~---~--~~ 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: #9344 and policy for small bug reports
I just want to register a similar issue. Had opened a ticket, #10057, with patch on a trivial matter as http://code.djangoproject.com/ticket/10057. Haven't found even any comments regarding its feasibility etc. Can't figure out whether people are engaged for some milestone coming but this seems to be a bad time. Lot of tickets are there remaining to be un-reviewed. Feel like It'll be better If we can at least review the problem as soon as possible. Assigning it to some one may take more time. (Writing in Sanskrit and at odd times as per UTC seems not that difficult for me. ;) ) Or else, the ticket owners may lose their interest in the topic and later when someone takes charge, no one will be there to give more feedback, when needed.. What can we do for that? --~--~-~--~~~---~--~~ 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: Custom FilterSpecs #5833 planned for Django 1.1?
On Feb 8, 2:15 pm, Karen Tracey wrote: > I don't have time to devote to this right now, but I'd suggest taking a look > at the svn history of the tests that check for the ?e=1 redirect. I have a > vague recollection that it/they may be there as a result of a problem where > incorrect lookup parameters caused an exception, and when that was fixed the > test was added to ensure invalid lookups didn't cause the admin to generate > a Server Error. I think the main goal of the test is to ensure no exception > was raised, not necessarily to lock in place the use of the ?e=1 redirect as > a way of handling the situation. > > If you've got something more useful to do with GET parameters that don't > match field names than do a cryptic (and often huh?-inducing, since first it > is easy to not notice what's happened and second it's not exactly crystal > clear even if you notice the ?e=1 in the url in the address bar) redirect to > ?e=1, I wouldn't necessarily think a test that checks for that ?e=1 redirect > should stand in the way of that. I found the check-in (#9245) and ticket (http://code.djangoproject.com/ ticket/9252) you were referring to. If we want to flag when an unknown GET parameter is passed to the change_list, we're going to have to create a mechanism where each filterspec "consumes" the parameters that it uses and passes the filtered parameter list to the next filterspec until we get to the code in get_query_set() that handles the generic cases. This won't be difficult, except that filterspecs are designed to inherited and this will be require every filterspec to "consume" it's own parameters. The other option is to just ignore unknown parameters. This seems kind of ugly, but I'll bet there are many frameworks out there that simply ignore unknown parameters. Thoughts? Cheers, Ben --~--~-~--~~~---~--~~ 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: Option to disable messages in auth context processor
On Thu, Feb 5, 2009 at 9:07 AM, Jacob Kaplan-Moss wrote: > On Thu, Feb 5, 2009 at 9:01 AM, Vinicius Mendes wrote: >> So I decided to write a new messages app and it works very well, the only >> problem is the django.core.context_processors.auth. > > Yeah, this processor has a bunch of bugs in it. I think Malcolm and I > figured out that there's something like five different bugs -- not bad > for three lines of code! Are these documented anywhere, by chance? I know the get_and_delete_messages call is problematic because it causes evaluation of the LazyUser object created by the AuthenticationMiddleware. This causes both an access to the session (causing #6552 - Vary: Cookie [1]) and an extra query on every request to grab the user. What are the other issues? Gary [1] http://code.djangoproject.com/ticket/6552 --~--~-~--~~~---~--~~ 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: Interaction of annotate() and values()
On Sun, Feb 8, 2009 at 1:56 AM, Russell Keith-Magee
wrote:
> So - some options:
>
> 1) Leave things as-is. Annotated columns always appear in the result
> set. This is inconsistent with extra(), and means you can't use
> annotated queries in __in clauses.
>
> 2) Modify things slightly - annotate() calls before a values() call
> aren't automatically added to the results, but annotate() calls after
> values() are included automatically. This would allow annotations to
> always be returned, optionally under those circumstances that allow
> (i.e., annotate() before values()).
>
> 3) Rely on multiple calls to values() to get annotate columns into the
> result set. i.e., the previous example would need to become something
> like:
>
> >>>
> Book.values('title','isbn').annotate(n_authors=Count('authors')).values('title','isbn','n_authors')
>
> This has the potential to be a bit fragile - we would need to put in
> all sorts of checks for changes in column lists between successive
> calls to values() - but it would allow all combinations of annotation.
>
> Opinions? Any other options I may have missed?
>
I don't like the third option -- if I'm understanding it right it seems like
it requires a lot of duplication in calling values() twice for some pretty
simple and common uses? 2 sounds like the best option to me, assuming it's
not too terribly convoluted to implement.
Karen
--~--~-~--~~~---~--~~
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: Interaction of annotate() and values()
On Mon, Feb 9, 2009 at 3:12 PM, Karen Tracey wrote:
> On Sun, Feb 8, 2009 at 1:56 AM, Russell Keith-Magee
> wrote:
>>
>> So - some options:
>>
>> 1) Leave things as-is. Annotated columns always appear in the result
>> set. This is inconsistent with extra(), and means you can't use
>> annotated queries in __in clauses.
>>
>> 2) Modify things slightly - annotate() calls before a values() call
>> aren't automatically added to the results, but annotate() calls after
>> values() are included automatically. This would allow annotations to
>> always be returned, optionally under those circumstances that allow
>> (i.e., annotate() before values()).
>>
>> 3) Rely on multiple calls to values() to get annotate columns into the
>> result set. i.e., the previous example would need to become something
>> like:
>>
>> >>>
>> >>> Book.values('title','isbn').annotate(n_authors=Count('authors')).values('title','isbn','n_authors')
>>
>> This has the potential to be a bit fragile - we would need to put in
>> all sorts of checks for changes in column lists between successive
>> calls to values() - but it would allow all combinations of annotation.
>>
>> Opinions? Any other options I may have missed?
>
> I don't like the third option -- if I'm understanding it right it seems like
> it requires a lot of duplication in calling values() twice for some pretty
> simple and common uses? 2 sounds like the best option to me, assuming it's
> not too terribly convoluted to implement.
Option 2 was my preferred option. The implementation borders on
trivial, and there's only 1 or two test cases from the test suite that
are affected. I just wanted a sanity check to see if I was missing an
obvious option from being to close to the code, or if anyone was
particularly attached to the compulsory inclusion behavior of
annotate().
Yours,
Russ Magee %-)
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---
