From 2415c2e564ae02584a05f7068f165bb551967339 Mon Sep 17 00:00:00 2001 From: Jay Pipes Date: Fri, 29 Sep 2017 12:59:47 -0400 Subject: [PATCH] rp: de-ORM ResourceProvider.get_by_uuid() This patch series aims to remove the use of SQLAlchemy ORM querying from the nova/objects/resource_provider.py module. Currently, there is a mix of non-ORM (SQLAlchemy core expression API) and ORM (SQLAlchemy's orm module and associated query generation from model reflection). While implementing the database schema for the nested resource providers table structure, which uses both an adjacency list model as well as a cached root tree ID, it became obvious that using the SQLAlchemy ORM modeling to generate the required queries for various methods related to resource providers was resulting in awkward and hard-to-reason-about code. Even using the recommended handling of self-referential tables [1] for adjacency list modeling, the way the session and querying handling was done in the resource_provider.py module led to a number of lazy-loading problems and inactive session errors. In this starter patch, we tackle the ResourceProvider.get_by_uuid() method, converting it to use the SQLAlchemy core expression API instead of an ORM query. [1] http://docs.sqlalchemy.org/en/latest/orm/self_referential.html blueprint: de-orm-resource-providers Change-Id: I2f14afa8fc01b0ec1b7ea3eaa0bf6c459a8681d2 --- nova/objects/resource_provider.py | 44 +++++++++++++------ .../unit/objects/test_resource_provider.py | 6 +-- 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/nova/objects/resource_provider.py b/nova/objects/resource_provider.py index 2f23ce1e23..08839fcdf7 100644 --- a/nova/objects/resource_provider.py +++ b/nova/objects/resource_provider.py @@ -388,6 +388,30 @@ def _set_inventory(context, rp, inv_list): return exceeded +@db_api.api_context_manager.reader +def _get_provider_by_uuid(context, uuid): + """Given a UUID, return a dict of information about the resource provider + from the database. + + :raises: NotFound if no such provider was found + :param uuid: The UUID to look up + """ + conn = conn = context.session.connection() + rpt = sa.alias(_RP_TBL, name="rp") + cols = [ + rpt.c.id, + rpt.c.uuid, + rpt.c.name, + rpt.c.generation, + ] + sel = sa.select(cols).where(rpt.c.uuid == uuid) + res = conn.execute(sel).fetchone() + if not res: + raise exception.NotFound( + 'No resource provider with uuid %s found' % uuid) + return dict(res) + + @base.NovaObjectRegistry.register_if(False) class ResourceProvider(base.NovaObject): @@ -425,8 +449,13 @@ class ResourceProvider(base.NovaObject): @classmethod def get_by_uuid(cls, context, uuid): - db_resource_provider = cls._get_by_uuid_from_db(context, uuid) - return cls._from_db_object(context, cls(), db_resource_provider) + """Returns a new ResourceProvider object with the supplied UUID. + + :raises NotFound if no such provider could be found + :param uuid: UUID of the provider to search for + """ + rp_rec = _get_provider_by_uuid(context, uuid) + return cls._from_db_object(context, cls(), rp_rec) def add_inventory(self, inventory): """Add one new Inventory to the resource provider. @@ -521,17 +550,6 @@ class ResourceProvider(base.NovaObject): resource_provider.obj_reset_changes() return resource_provider - @staticmethod - @db_api.api_context_manager.reader - def _get_by_uuid_from_db(context, uuid): - result = context.session.query(models.ResourceProvider).filter_by( - uuid=uuid).first() - if not result: - raise exception.NotFound( - 'No resource provider with uuid %s found' - % uuid) - return result - @staticmethod @db_api.api_context_manager.reader def _get_aggregates(context, rp_id): diff --git a/nova/tests/unit/objects/test_resource_provider.py b/nova/tests/unit/objects/test_resource_provider.py index a49ba310f6..3f7f95b0d7 100644 --- a/nova/tests/unit/objects/test_resource_provider.py +++ b/nova/tests/unit/objects/test_resource_provider.py @@ -71,8 +71,8 @@ def _fake_ensure_cache(ctxt): class TestResourceProviderNoDB(test_objects._LocalTest): USES_DB = False - @mock.patch('nova.objects.resource_provider.ResourceProvider.' - '_get_by_uuid_from_db', return_value=_RESOURCE_PROVIDER_DB) + @mock.patch('nova.objects.resource_provider._get_provider_by_uuid', + return_value=_RESOURCE_PROVIDER_DB) def test_object_get_by_uuid(self, mock_db_get): resource_provider_object = resource_provider.ResourceProvider.\ get_by_uuid(mock.sentinel.ctx, _RESOURCE_PROVIDER_UUID) @@ -130,7 +130,7 @@ class TestResourceProvider(test_objects._LocalTest): uuid=_RESOURCE_PROVIDER_UUID, name=_RESOURCE_PROVIDER_NAME) rp.create() - retrieved_rp = resource_provider.ResourceProvider._get_by_uuid_from_db( + retrieved_rp = resource_provider.ResourceProvider.get_by_uuid( self.context, _RESOURCE_PROVIDER_UUID) self.assertEqual(rp.uuid, retrieved_rp.uuid) self.assertEqual(rp.name, retrieved_rp.name)