[Django] #30309: Remove hasattr reference in One-to-One documentation example
Django
-----------------------------------------+------------------------
Reporter: davidjb | Owner: nobody
Type: Uncategorized | Status: new
Component: Documentation | Version: 2.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
The One-to-One example of using `hasattr` to avoid exception handling is
based on the side effect of `hasattr` swallowing all exceptions. This was
pre-Python 3.2 behaviour and has been changed to only suppress
AttributeErrors as explained at https://bugs.python.org/issue9666.
PR: https://github.com/django/django/pull/11159
--
Ticket URL: <https://code.djangoproject.com/ticket/30309>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: davidjb | Owner: nobody
Type: Uncategorized | Status: closed
Component: Documentation | Version: 2.2
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* status: new => closed
* resolution: => invalid
Comment:
IMO this doc is still valid and it is not related with pre-Python 3.2
behavior. It's quite straightforward, so if you want to check that `p2`
doesn’t have an associated restaurant you can catch `ObjectDoesNotExist`
exception or you can use `hasattr(p2, 'restaurant')` (if you don't want to
catch exceptions).
--
Ticket URL: <https://code.djangoproject.com/ticket/30309#comment:1>
Django
Reporter: David Beitey | Owner: nobody
Type: Uncategorized | Status: closed
Component: Documentation | Version: 2.2
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by David Beitey):
In my experience today, using `hasattr(p2, 'restaurant')` is throwing a
`ObjectDoesNotExist` exception with Django 2.1.7 / Python 3.7 and the
exception is not being swallowed. This was the reason for me making the
contribution. For note, I'm not the only one to making this observation:
https://stackoverflow.com/a/22240021/1048705 -- the last paragraph of that
answer.
If it's Django-specific behaviour to add `hasattr` functionality to
swallow errors, then perhaps a bug report is necessary instead of a
documentation fix. If Django is doing this, I'd suggest changing the
behaviour so Django follows Python itself; why Python made the change
explained on the mailing list: https://mail.python.org/pipermail/python-
dev/2010-August/103178.html
Are we able to re-open this? Happy to do further diagnosis if you can't
replicate the exception being raised.
Fwiw, I understand the use case for avoiding exception handling with a
`OneToOneField` - I'd love a clean solution for this myself.
--
Ticket URL: <https://code.djangoproject.com/ticket/30309#comment:2>
Django
Reporter: David Beitey | Owner: nobody
Type: Uncategorized | Status: closed
Component: Documentation | Version: 2.2
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by felixxm):
`hasattr()` is not throwing an exceptions, just in case I checked with
Python 3.6/3.7 and Django 2.1/2.2.
Did you check this?
FYI: `RelatedObjectDoesNotExist` is a subclass of `AttributeError`.
--
Ticket URL: <https://code.djangoproject.com/ticket/30309#comment:3>
Django
Reporter: David Beitey | Owner: nobody
Type: Uncategorized | Status: closed
Component: Documentation | Version: 2.2
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by David Beitey):
Replying to [comment:3 felixxm]:
> `hasattr()` is not throwing an exceptions, just in case I checked with
Python 3.6/3.7 and Django 2.1/2.2.
>
> Did you check this?
>
> FYI: `RelatedObjectDoesNotExist` is a subclass of `AttributeError`.
Okay, I dug into the situation and what's happening isn't related to
Python version.
`RelatedObjectDoesNotExist` gets raised when the value of a OneToOneField
is None/null. However, if there is data in the field but it isn't present
in the related table, `[model_identifier].DoesNotExist` gets raised. The
latter exception doesn't inherit from `AttributeError` and isn't swallowed
with `hasattr()`.
Since the behaviour of `hasattr()` is to return False when there's no
related object, that seems to be what should happen in both situations --
with None as a value or a field value pointing at a non-existant related
object. In other words, it feels as though `RelatedObjectDoesNotExist`
should be being raised when accessing the attribute on the model in this
way, rather than at the query level (see traceback below).
Here's my example:
{{{
from django.db import models
class Person(models.Model):
login = models.CharField(db_column='login', max_length=100)
# and more...
class Meta:
managed = False
db_table = 'SYS_PEOPLE'
class Author(models.Model):
person = models.OneToOneField(Person,
db_column='login',
primary_key=True,
on_delete=models.CASCADE)
# and more...
class Meta:
managed = False
db_table = 'SYS_AUTHORS'
# manage.py shell
from app.models import Author
author = Author(person_id='fake')
author.person # raises DoesNotExist
hasattr(author, 'person') # raises DoesNotExist
author = Author(person_id=None)
author.person # raises RelatedObjectDoesNotExist
hasattr(author, 'person') # Swallows
}}}
Tracebacks:
{{{
Traceback (most recent call last):
File "/home/user/local/share/virtualenvs/app-1933afW/lib/python3.7/site-
packages/django/db/models/fields/related_descriptors.py", line 163, in
__get__
rel_obj = self.field.get_cached_value(instance)
File "/home/user/local/share/virtualenvs/app-1933afW/lib/python3.7/site-
packages/django/db/models/fields/mixins.py", line 13, in get_cached_value
return instance._state.fields_cache[cache_name]
KeyError: 'person'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "./manage.py", line 17, in <module>
execute_from_command_line(sys.argv)
File "/home/user/local/share/virtualenvs/app-1933afW/lib/python3.7/site-
packages/django/core/management/__init__.py", line 381, in
execute_from_command_line
utility.execute()
File "/home/user/local/share/virtualenvs/app-1933afW/lib/python3.7/site-
packages/django/core/management/__init__.py", line 375, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/home/user/local/share/virtualenvs/app-1933afW/lib/python3.7/site-
packages/django/core/management/base.py", line 316, in run_from_argv
self.execute(*args, **cmd_options)
File "/home/user/local/share/virtualenvs/app-1933afW/lib/python3.7/site-
packages/django/core/management/base.py", line 353, in execute
output = self.handle(*args, **options)
File "/home/user/local/share/virtualenvs/app-1933afW/lib/python3.7/site-
packages/django/core/management/commands/shell.py", line 92, in handle
exec(sys.stdin.read())
File "<string>", line 4, in <module>
File "/home/user/local/share/virtualenvs/app-1933afW/lib/python3.7/site-
packages/django/db/models/fields/related_descriptors.py", line 177, in
__get__
rel_obj = self.get_object(instance)
File "/home/user/local/share/virtualenvs/app-1933afW/lib/python3.7/site-
packages/django/db/models/fields/related_descriptors.py", line 297, in
get_object
return super().get_object(instance)
File "/home/user/local/share/virtualenvs/app-1933afW/lib/python3.7/site-
packages/django/db/models/fields/related_descriptors.py", line 144, in
get_object
return qs.get(self.field.get_reverse_related_filter(instance))
File "/home/user/local/share/virtualenvs/app-1933afW/lib/python3.7/site-
packages/django/db/models/query.py", line 399, in get
self.model._meta.object_name
app.models.person.DoesNotExist: Person matching query does not exist.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30309#comment:4>
Django
Reporter: David Beitey | Owner: nobody
Type: Uncategorized | Status: closed
Component: Documentation | Version: 2.2
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by felixxm):
First of all described situation has nothing to do with examples in docs.
Secondly I'm sorry but I strongly disagree with you, this is a completely
different situation. IMO if related field is assigned but objects does not
exists then `hasattr()` should raise `DoesNotExist` exception, because
attribute is assigned to an object it is simply incorrect, `existence !=
correctness`.
You can always start a new thread in the
[https://groups.google.com/forum/?utm_source=digest&utm_medium=email#!forum
/django-developers django-developers] if you think that this should be
changed.
--
Ticket URL: <https://code.djangoproject.com/ticket/30309#comment:5>
Django
Reporter: David Beitey | Owner: nobody
Type: Uncategorized | Status: closed
Component: Documentation | Version: 2.2
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette):
FWIW this scenario can also happen when using
`on_delete=models.DO_NOTHING, db_constraint=False` and an attempt is made
to access a remote object that has been deleted.
I'd be in favor of making `ForwardManyToOneDescriptor.get_object` raise
`self.RelatedObjectDoesNotExist` as the current code clearly doesn't take
`db_constraint=False` into account based on
[https://github.com/django/django/blob/755673e1bca7edb6bee7a958f40d9ae54d85d44c/django/db/models/fields/related_descriptors.py#L144
the heuristic comment there].
--
Ticket URL: <https://code.djangoproject.com/ticket/30309#comment:6>
Django
Reporter: David Beitey | Owner: nobody
Type: Uncategorized | Status: closed
Component: Documentation | Version: 2.2
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by David Beitey):
Discussion around raising `self.RelatedObjectDoesNotExist` continues at
https://groups.google.com/d/msgid/django-
developers/cdcf9eda-c691-4354-b806-51de36ec6d24%40googlegroups.com
--
Ticket URL: <https://code.djangoproject.com/ticket/30309#comment:7>