Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#23123 closed Cleanup/optimization (fixed)

Misleading Code in Admin Example

Reported by: wkschwartz@… Owned by: Baptiste Mispelon <bmispelon@…>
Component: Documentation Version: 1.7-rc-2
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

In https://docs.djangoproject.com/en/1.7/ref/contrib/admin/#django.contrib.admin.ModelAdmin.get_search_results there is a code example:

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:

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

Attachments (1)

23123.diff (854 bytes ) - added by Tim Graham 10 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 by Rainer Koirikivi, 10 years ago

Having the queryset |= ... line after the except clause, as in the suggested example, causes an exception in case int(search_term) fails.

by Tim Graham, 10 years ago

Attachment: 23123.diff added

comment:2 by Tim Graham, 10 years ago

Triage Stage: UnreviewedAccepted

How about putting queryset != ... in the else clause of the try/except.

comment:3 by Gabriel Muñumel, 10 years ago

Owner: changed from nobody to Gabriel Muñumel
Status: newassigned

comment:4 by Gabriel Muñumel, 10 years ago

Owner: Gabriel Muñumel removed
Status: assignednew

comment:5 by Baptiste Mispelon <bmispelon@…>, 10 years ago

Owner: set to Baptiste Mispelon <bmispelon@…>
Resolution: fixed
Status: newclosed

In e5619330e2d3bf901155e98ef3fa7d224b6a260a:

Fixed #23123 -- Don't use a bare except in ModelAdmin documentation

Thanks to wkschwartz for the report and to Tim for the patch.

comment:6 by Baptiste Mispelon <bmispelon@…>, 10 years ago

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

comment:7 by Tim Graham <timograham@…>, 10 years ago

In 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

Note: See TracTickets for help on using tickets.
Back to Top