[Django] #21619: SingleObjectMixin raises ObjectDoesNotExist, rather than a more specific subclass

Django

unread,

Dec 14, 2013, 12:40:05 PM12/14/13

to django-...@googlegroups.com

#21619: SingleObjectMixin raises ObjectDoesNotExist, rather than a more specific
subclass
-----------------------------------------+--------------------
Reporter: Keryn Knight <django@…> | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Generic views | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------------+--------------------
When calling `get_object` for something which inherits from
SingleObjectMixin, the exception caught is
[https://github.com/django/django/blob/36ded01527b690b5df0574492af9cfcc2ea3d1dc/django/views/generic/detail.py#L53
ObjectDoesNotExist], when as far as I can tell, based on the `Http404`
[https://github.com/django/django/blob/36ded01527b690b5df0574492af9cfcc2ea3d1dc/django/views/generic/detail.py#L55
raised], the more specific form of `queryset.model.DoesNotExist` could be
used instead.

I can't see any docs or comments as to a historic (or current) reason it
may be catching the generic version (perhaps there are scenarios where the
Model's `DoesNotExist` is not available yet?), and there are probably no
implications by casting a wider net than necessary, but I'm opening the
ticket anyway in case as a possible cleanup worth doing.

--
Ticket URL: <https://code.djangoproject.com/ticket/21619>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Dec 14, 2013, 1:06:07 PM12/14/13

to django-...@googlegroups.com

#21619: SingleObjectMixin raises ObjectDoesNotExist, rather than a more specific
subclass

-------------------------------------+-------------------------------------
Reporter: Keryn Knight | Owner: nobody
<django@…> | Status: new
Type: | Version: master
Cleanup/optimization | Resolution:
Component: Generic views | Triage Stage:
Severity: Normal | Unreviewed
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by mjtamlyn):

* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0

Comment:

I don't see any reason why we would need the less specific exception.

--
Ticket URL: <https://code.djangoproject.com/ticket/21619#comment:1>

Django

unread,

Dec 14, 2013, 1:06:22 PM12/14/13

to django-...@googlegroups.com

#21619: SingleObjectMixin raises ObjectDoesNotExist, rather than a more specific
subclass

-------------------------------------+-------------------------------------
Reporter: Keryn Knight | Owner: nobody
<django@…> | Status: new
Type: | Version: master
Cleanup/optimization | Resolution:

Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by mjtamlyn):

* stage: Unreviewed => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/21619#comment:2>

Django

unread,

Dec 14, 2013, 6:30:43 PM12/14/13

to django-...@googlegroups.com

#21619: SingleObjectMixin raises ObjectDoesNotExist, rather than a more specific
subclass

-------------------------------------+-------------------------------------
Reporter: Keryn Knight | Owner: pjrharley
<django@…> | Status: assigned

Has patch: 1 | UI/UX: 0

Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Changes (by pjrharley):

* owner: nobody => pjrharley
* cc: pjrharley (added)
* has_patch: 0 => 1
* status: new => assigned

Comment:

Submitted PR here:

https://github.com/django/django/pull/2077

--
Ticket URL: <https://code.djangoproject.com/ticket/21619#comment:3>

Django

unread,

Dec 14, 2013, 7:43:19 PM12/14/13

to django-...@googlegroups.com

#21619: SingleObjectMixin raises ObjectDoesNotExist, rather than a more specific
subclass

Keywords: | Patch needs improvement: 1

Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Changes (by bmispelon):

* needs_better_patch: 0 => 1

Comment:

Hi,

I added some comments on the PR.
Once those are addressed, you can remove the `patch needs improvement`
flag on the ticket.

Thanks.

--
Ticket URL: <https://code.djangoproject.com/ticket/21619#comment:4>

Django

unread,

Dec 15, 2013, 3:25:55 PM12/15/13

to django-...@googlegroups.com

#21619: SingleObjectMixin raises ObjectDoesNotExist, rather than a more specific
subclass

Keywords: | Patch needs improvement: 0

Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Changes (by pjrharley):

* needs_better_patch: 1 => 0

Comment:

Thanks, new commit added to the PR and some comments over there.

--
Ticket URL: <https://code.djangoproject.com/ticket/21619#comment:5>

Django

unread,

Dec 15, 2013, 3:56:47 PM12/15/13

to django-...@googlegroups.com

#21619: SingleObjectMixin raises ObjectDoesNotExist, rather than a more specific
subclass

-------------------------------------+-------------------------------------
Reporter: Keryn Knight | Owner: pjrharley

<django@…> | Status: closed
Type: | Version: master
Cleanup/optimization | Resolution: fixed

Changes (by Baptiste Mispelon <bmispelon@…>):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"cdd6617da679e19059907979b03c4ef7b15691e6"]:
{{{
#!CommitTicketReference repository=""
revision="cdd6617da679e19059907979b03c4ef7b15691e6"
Fixed #21619 -- Made SingleObjectMixin.get_object catch a more precise
exception.