diff options
author | Victor Stinner <vstinner@redhat.com> | 2015-06-25 11:13:16 +0200 |
---|---|---|
committer | Roman Podoliaka <rpodolyaka@mirantis.com> | 2015-06-26 19:02:53 +0300 |
commit | 2e79681670f7b8e6843abf29d6e95ffda0c3147c (patch) | |
tree | e074cb25aaee499742e3c5dc8255c498714fdd30 | |
parent | eeacae192e69f56b38e5cfa0654e3a653bc9258a (diff) | |
download | oslo-db-2e79681670f7b8e6843abf29d6e95ffda0c3147c.tar.gz |
Fix sqlalchemy.ModelBase.__contains__() behaviour
Currently, sqlalchemy.ModelBase.__contains__() catches any exception,
and by doing that, it hides real bugs. For example, a Nova unit test
raises the following error, but __contains__() simply returns False:
sqlalchemy.orm.exc.DetachedInstanceError: Parent instance
<InstanceExtra at 0x7fea635bc5f8> is not bound to a Session;
deferred load operation of attribute 'pci_requests' cannot proceed
On Python 3, hasattr() calls getattr(): it returns True if getattr()
succeeds, False if getattr() raises an AttributeError, or passes
through the exception, if getattr() failed for a different reason.
On Python 2, hasattr() also calls getattr(), but it catches *any*
exception.
This change replaces hasattr() with getattr(), and it only catches
AttributeError as Python 3, so passes through sqlalchemy exceptions.
Add an unit test to test the new behaviour.
Closes-Bug: #1469225
Change-Id: If9c3ccc03f1dc9746936b0b83ea132508491e577
-rw-r--r-- | oslo_db/sqlalchemy/models.py | 10 | ||||
-rw-r--r-- | oslo_db/tests/sqlalchemy/test_models.py | 12 |
2 files changed, 21 insertions, 1 deletions
diff --git a/oslo_db/sqlalchemy/models.py b/oslo_db/sqlalchemy/models.py index 4293c9d..0c4e0de 100644 --- a/oslo_db/sqlalchemy/models.py +++ b/oslo_db/sqlalchemy/models.py @@ -54,7 +54,15 @@ class ModelBase(six.Iterator): return getattr(self, key) def __contains__(self, key): - return hasattr(self, key) + # Don't use hasattr() because hasattr() catches any exception, not only + # AttributeError. We want to passthrough SQLAlchemy exceptions + # (ex: sqlalchemy.orm.exc.DetachedInstanceError). + try: + getattr(self, key) + except AttributeError: + return False + else: + return True def get(self, key, default=None): return getattr(self, key, default) diff --git a/oslo_db/tests/sqlalchemy/test_models.py b/oslo_db/tests/sqlalchemy/test_models.py index 525573a..279c5c3 100644 --- a/oslo_db/tests/sqlalchemy/test_models.py +++ b/oslo_db/tests/sqlalchemy/test_models.py @@ -70,6 +70,18 @@ class ModelBaseTest(test_base.DbTestCase): self.assertFalse('non-existent-key' in mb) + def test_modelbase_contains_exc(self): + class ErrorModel(models.ModelBase): + @property + def bug(self): + raise ValueError + + model = ErrorModel() + model.update({'attr': 5}) + + self.assertTrue('attr' in model) + self.assertRaises(ValueError, lambda: 'bug' in model) + def test_modelbase_items_iteritems(self): h = {'a': '1', 'b': '2'} expected = { |