[Django] #23123: Misleading Code in Admin Example
Django
--------------------------------------+----------------------
Reporter: wkschwartz@… | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.7-rc-2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 1 | UI/UX: 0
--------------------------------------+----------------------
In
https://docs.djangoproject.com/en/1.7/ref/contrib/admin/#django.contrib.admin.ModelAdmin.get_search_results
there is a code example:
{{{#!python
def get_search_results(self, request, queryset, search_term):
queryset, use_distinct = super(PersonAdmin,
self).get_search_results(request, queryset, search_term)
try:
search_term_as_int = int(search_term)
queryset |= self.model.objects.filter(age=search_term_as_int)
except:
pass
return queryset, use_distinct
}}}
This code catches a `SystemExit` when all it meant to do was deal with the
case when `search_term` is not an integer. See
https://docs.python.org/2/howto/doanddont.html#except
Better would be:
{{{#!python
def get_search_results(self, request, queryset, search_term):
queryset, use_distinct = super(PersonAdmin,
self).get_search_results(request, queryset, search_term)
try:
search_term_as_int = int(search_term)
except ValueError:
pass
queryset |= self.model.objects.filter(age=search_term_as_int)
return queryset, use_distinct
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23123>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: wkschwartz@… | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.7-rc-2
Component: Documentation | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by koirikivi):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Having the `queryset |= ...` line after the except clause, as in the
suggested example, causes an exception in case `int(search_term)` fails.
--
Ticket URL: <https://code.djangoproject.com/ticket/23123#comment:1>
Django
Reporter: wkschwartz@… | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.7-rc-2
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by timo):
* stage: Unreviewed => Accepted
Comment:
How about putting `queryset != ...` in the else clause of the try/except.
--
Ticket URL: <https://code.djangoproject.com/ticket/23123#comment:2>
Django
Reporter: wkschwartz@… | Owner: gmunumel
Type: Cleanup/optimization | Status: assigned
Component: Documentation | Version: 1.7-rc-2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by gmunumel):
* owner: nobody => gmunumel
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/23123#comment:3>
Django
Reporter: wkschwartz@… | Owner:
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.7-rc-2
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by gmunumel):
* owner: gmunumel =>
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/23123#comment:4>
Django
Reporter: wkschwartz@… | Owner: Baptiste
Type: | Mispelon <bmispelon@…>
Cleanup/optimization | Status: closed
Component: Documentation | Version: 1.7-rc-2
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by Baptiste Mispelon <bmispelon@…>):
* owner: => Baptiste Mispelon <bmispelon@…>
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"e5619330e2d3bf901155e98ef3fa7d224b6a260a"]:
{{{
#!CommitTicketReference repository=""
revision="e5619330e2d3bf901155e98ef3fa7d224b6a260a"
Fixed #23123 -- Don't use a bare except in ModelAdmin documentation
Thanks to wkschwartz for the report and to Tim for the patch.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23123#comment:5>
Django
Reporter: wkschwartz@… | Owner: Baptiste
Type: | Mispelon <bmispelon@…>
Cleanup/optimization | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by Baptiste Mispelon <bmispelon@…>):
In [changeset:"9fb1652ac33f3ea7ba2f05e089d73691c9d41b8e"]:
{{{
#!CommitTicketReference repository=""
revision="9fb1652ac33f3ea7ba2f05e089d73691c9d41b8e"
[1.7.x] Fixed #23123 -- Don't use a bare except in ModelAdmin
documentation
Thanks to wkschwartz for the report and to Tim for the patch.
Backport of e5619330e2d3bf901155e98ef3fa7d224b6a260a from master.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23123#comment:6>
Django
Reporter: wkschwartz@… | Owner: Baptiste
Type: | Mispelon <bmispelon@…>
Cleanup/optimization | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"8e25b696ba896f7f9b07565e7559aa08e743288a"]:
{{{
#!CommitTicketReference repository=""
revision="8e25b696ba896f7f9b07565e7559aa08e743288a"
[1.6.x] Fixed #23123 -- Don't use a bare except in ModelAdmin
documentation
Thanks to wkschwartz for the report and to Tim for the patch.
Backport of e5619330e2 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23123#comment:7>