Make objects behave more like our old dictionaries

Prior to Cinder Objects we passed a dictionary representation
of a Cinder resource around. There are a few places where we
abuse this a bit and do things like "ref_dict.get('some-key', None)"
to reuse some code and determine if for example the object is
a snapshot or a volume (based on it's keys). This is probably
a pretty lazy/bad idea, but we have cases where we do it,
such as in the SolidFire driver and it looks like maybe some
calls in the profiler.

For now, we just make the compatibility shim in Object base truly
compatible and return None for non-existent attributes rather
than raising. Follow up work would be to clean up all of our
usage to strictly use objects and get rid of the compatibility
shim altogether.

Change-Id: Ia640c912700d9569c6b522b77fc357fb026269b1
Closes-Bug: #1427851

diff --git a/cinder/objects/base.py b/cinder/objects/base.py
index 5bf74ded50ab73c54d3f7407dfa18ab3c30c4331..61a47c623ab29182da89cca348d032e8922160eb 100644 (file)
--- a/cinder/objects/base.py
+++ b/cinder/objects/base.py
@@ -28,7 +28,7 @@ import six
 
 from cinder import context
 from cinder import exception
-from cinder.i18n import _, _LE
+from cinder.i18n import _, _LE, _LW
 from cinder import objects
 from cinder.objects import fields
 from cinder.openstack.common import log as logging
@@ -611,8 +611,16 @@ class CinderObjectDictCompat(object):
         NOTE(danms): May be removed in the future.
         """
         if key not in self.obj_fields:
-            raise AttributeError("'%s' object has no attribute '%s'" % (
-                self.__class__, key))
+            # NOTE(jdg): There are a number of places where we rely on the
+            # old dictionary version and do a get(xxx, None).
+            # The following preserves that compatability but in
+            # the future we'll remove this shim altogether so don't
+            # rely on it.
+            LOG.warning(_LW('Cinder object %(object_name)s has no '
+                            'attribute named: %(attribute_name)s'),
+                        {'object_name': self.__class__.__name__,
+                         'attribute_name': key})
+            return None
         if value != NotSpecifiedSentinel and not self.obj_attr_is_set(key):
             return value
         else:

diff --git a/cinder/tests/objects/test_objects.py b/cinder/tests/objects/test_objects.py
index 1eacc4c881871b9cd16e7d5bee73872c7204c8db..2758dba342cce4e9a67eafe0af71324b561b895e 100644 (file)
--- a/cinder/tests/objects/test_objects.py
+++ b/cinder/tests/objects/test_objects.py
@@ -613,10 +613,8 @@ class _TestObject(object):
         self.assertEqual('loaded!', obj.get('bar'))
         # Bar now has a default, but loaded value should be returned
         self.assertEqual('loaded!', obj.get('bar', 'not-loaded'))
-        # Invalid attribute should raise AttributeError
-        self.assertRaises(AttributeError, obj.get, 'nothing')
-        # ...even with a default
-        self.assertRaises(AttributeError, obj.get, 'nothing', 3)
+        # Invalid attribute should return None
+        self.assertEqual(None, obj.get('nothing'))
 
     def test_object_inheritance(self):
         base_fields = base.CinderPersistentObject.fields.keys()