From 5ec38f6d3b76619fac442a85a9b5c73dc606b83a Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Mon, 5 Sep 2022 11:33:11 +0200 Subject: [PATCH] Make us compatible with oslo.db 12.1.0 With oslo.db 12.1.0 the following sqlalchemy warning become an error: sqlalchemy.exc.RemovedIn20Warning: Retrieving row members using strings or other non-integers is deprecated; use row._mapping for a dictionary interface to the row (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9) We tried to fix this before but missed a test_case that still used dict access to get the fields of a Row instead of attribute access. We fixed that test here. Also while fixed it I noticed that the generic _AttributeCache object states that it stores dicts but actually it sometimes stores dict but sometimes it stores Row objects. So the doc is updated and the dict path converted to store namedtuple objects in the cache instead. Note that Row is also acts like a namedtuple except that ._mapping does not exists in namedtuple but exists in Row. The trait object assumed it gets a Row object with ._mapping from the cache so that is adjusted to only assume a namedtuple and use _asdict() to covert it to dict which is available both in Row and namedtuple. Change-Id: I23ac1d85290a2dec307f8e76aafb02096259b605 --- placement/attribute_cache.py | 29 +++++++++++-------- placement/objects/trait.py | 4 +-- .../functional/db/test_attribute_cache.py | 10 +++---- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/placement/attribute_cache.py b/placement/attribute_cache.py index 60545d3ed..d654d9651 100644 --- a/placement/attribute_cache.py +++ b/placement/attribute_cache.py @@ -9,6 +9,7 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +import collections import sqlalchemy as sa @@ -41,6 +42,12 @@ class _AttributeCache(object): _table = None _not_found = None + # The cache internally stores either sqlalchemy Row objects or + # Attrs namedtuples but Row is compatible with namedtuple interface too. + Attrs = collections.namedtuple( + "Attrs", ["id", "name", "updated_at", "created_at"] + ) + def __init__(self, ctx): """Initialize the cache of resource class identifiers. @@ -87,14 +94,14 @@ class _AttributeCache(object): :param attr_str: The string representation of the attribute for which to look up the object. - :returns: dict representing the attribute fields, if the attribute was - found in the appropriate database table. + :returns: namedtuple representing the attribute fields, if the + attribute was found in the appropriate database table. :raises An instance of the subclass' _not_found exception if attr_str cannot be found in the DB. """ - attr_id_str = self._all_cache.get(attr_str) - if attr_id_str is not None: - return attr_id_str + attrs = self._all_cache.get(attr_str) + if attrs is not None: + return attrs # Otherwise, check the database table self._refresh_from_db(self._ctx) @@ -126,7 +133,7 @@ class _AttributeCache(object): def get_all(self): """Return an iterator of all the resources in the cache with all their - attributes. + attributes as a namedtuple. In Python3 the return value is a generator. """ @@ -152,6 +159,8 @@ class _AttributeCache(object): res = ctx.session.execute(sel).fetchall() self._id_cache = {r[1]: r[0] for r in res} self._str_cache = {r[0]: r[1] for r in res} + # Note that r is Row object that is compatible with the namedtuple + # interface of the cache self._all_cache = {r[1]: r for r in res} def _add_attribute(self, attr_id, name, created_at, updated_at): @@ -160,12 +169,8 @@ class _AttributeCache(object): """ self._id_cache[name] = attr_id self._str_cache[attr_id] = name - self._all_cache[name] = { - 'id': attr_id, - 'name': name, - 'created_at': created_at, - 'updated_at': updated_at, - } + attrs = self.Attrs(attr_id, name, updated_at, created_at) + self._all_cache[name] = attrs class ConsumerTypeCache(_AttributeCache): diff --git a/placement/objects/trait.py b/placement/objects/trait.py index e6d1bb388..cfb8e58c5 100644 --- a/placement/objects/trait.py +++ b/placement/objects/trait.py @@ -90,8 +90,8 @@ class Trait(object): @classmethod def get_by_name(cls, context, name): - db_trait = context.trait_cache.all_from_string(name)._mapping - return cls._from_db_object(context, cls(context), db_trait) + trait = context.trait_cache.all_from_string(name) + return cls._from_db_object(context, cls(context), trait._asdict()) @staticmethod @db_api.placement_context_manager.writer diff --git a/placement/tests/functional/db/test_attribute_cache.py b/placement/tests/functional/db/test_attribute_cache.py index d13133f6b..2754d4d97 100644 --- a/placement/tests/functional/db/test_attribute_cache.py +++ b/placement/tests/functional/db/test_attribute_cache.py @@ -93,11 +93,11 @@ class TestResourceClassCache(base.TestCase): # Verify all fields available from all_from_string iron_nfv_class = cache.all_from_string('IRON_NFV') - self.assertEqual(1001, iron_nfv_class['id']) - self.assertEqual('IRON_NFV', iron_nfv_class['name']) + self.assertEqual(1001, iron_nfv_class.id) + self.assertEqual('IRON_NFV', iron_nfv_class.name) # updated_at not set on insert - self.assertIsNone(iron_nfv_class['updated_at']) - self.assertIsInstance(iron_nfv_class['created_at'], datetime.datetime) + self.assertIsNone(iron_nfv_class.updated_at) + self.assertIsInstance(iron_nfv_class.created_at, datetime.datetime) # Update IRON_NFV (this is a no-op but will set updated_at) with self.placement_db.get_engine().connect() as conn: @@ -116,7 +116,7 @@ class TestResourceClassCache(base.TestCase): iron_nfv_class = cache.all_from_string('IRON_NFV') # updated_at set on update - self.assertIsInstance(iron_nfv_class['updated_at'], datetime.datetime) + self.assertIsInstance(iron_nfv_class.updated_at, datetime.datetime) def test_rc_cache_miss(self): """Test that we raise ResourceClassNotFound if an unknown resource