From 5d1a50018510b2b281ad33895ae2d9555f5d5b05 Mon Sep 17 00:00:00 2001 From: Eric Fried Date: Mon, 5 Nov 2018 14:02:45 -0600 Subject: [PATCH] Remove LazyLoad of Scheduler Clients Things have changed here on Walton's Mountain since LazyLoad was introduced [1]. It seems to have been created to avoid a circular import, but as this patch should attest, that's no longer an issue. Why change this now, besides removing weird and complicated code? Because a subsequent patch needs to access a *property* of the report client from the compute manager. As written, LazyLoad only lets you get at *methods* (as functools.partial). There are other ways to solve that while preserving the deferred import, but if this works, it's the better solution. [1] Ie5732baf9709cd0cb951eae4638910372c79e5f1 Change-Id: I1f97d00fb7633f173370ed6787c9a71ecd8106d5 --- nova/scheduler/client/__init__.py | 29 +++---------------- .../unit/conductor/tasks/test_live_migrate.py | 10 +++---- nova/tests/unit/scheduler/test_client.py | 5 ---- 3 files changed, 9 insertions(+), 35 deletions(-) diff --git a/nova/scheduler/client/__init__.py b/nova/scheduler/client/__init__.py index 78808378d3e3..2602879a3a71 100644 --- a/nova/scheduler/client/__init__.py +++ b/nova/scheduler/client/__init__.py @@ -13,38 +13,17 @@ # License for the specific language governing permissions and limitations # under the License. -import functools - -from oslo_utils import importutils - +from nova.scheduler.client import query +from nova.scheduler.client import report from nova.scheduler import utils -class LazyLoader(object): - - def __init__(self, klass, *args, **kwargs): - self.klass = klass - self.args = args - self.kwargs = kwargs - self.instance = None - - def __getattr__(self, name): - return functools.partial(self.__run_method, name) - - def __run_method(self, __name, *args, **kwargs): - if self.instance is None: - self.instance = self.klass(*self.args, **self.kwargs) - return getattr(self.instance, __name)(*args, **kwargs) - - class SchedulerClient(object): """Client library for placing calls to the scheduler.""" def __init__(self): - self.queryclient = LazyLoader(importutils.import_class( - 'nova.scheduler.client.query.SchedulerQueryClient')) - self.reportclient = LazyLoader(importutils.import_class( - 'nova.scheduler.client.report.SchedulerReportClient')) + self.queryclient = query.SchedulerQueryClient() + self.reportclient = report.SchedulerReportClient() @utils.retry_select_destinations def select_destinations(self, context, spec_obj, instance_uuids, diff --git a/nova/tests/unit/conductor/tasks/test_live_migrate.py b/nova/tests/unit/conductor/tasks/test_live_migrate.py index 9d67225ac086..b50845696ba7 100644 --- a/nova/tests/unit/conductor/tasks/test_live_migrate.py +++ b/nova/tests/unit/conductor/tasks/test_live_migrate.py @@ -629,12 +629,12 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase): @mock.patch.object(objects.ComputeNode, 'get_by_host_and_nodename', side_effect=exception.ComputeHostNotFound(host='host')) - def test_remove_host_allocations_compute_host_not_found(self, get_cn): + @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' + 'remove_provider_tree_from_instance_allocation') + def test_remove_host_allocations_compute_host_not_found( + self, remove_provider, get_cn): """Tests that failing to find a ComputeNode will not blow up the _remove_host_allocations method. """ - with mock.patch.object( - self.task.scheduler_client.reportclient, - 'remove_provider_from_instance_allocation') as remove_provider: - self.task._remove_host_allocations('host', 'node') + self.task._remove_host_allocations('host', 'node') remove_provider.assert_not_called() diff --git a/nova/tests/unit/scheduler/test_client.py b/nova/tests/unit/scheduler/test_client.py index a720a0bfbcd1..5abe014c5516 100644 --- a/nova/tests/unit/scheduler/test_client.py +++ b/nova/tests/unit/scheduler/test_client.py @@ -40,12 +40,10 @@ class SchedulerClientTestCase(test.NoDBTestCase): def test_select_destinations(self, mock_select_destinations): fake_spec = objects.RequestSpec() fake_spec.instance_uuid = uuids.instance - self.assertIsNone(self.client.queryclient.instance) self.client.select_destinations('ctxt', fake_spec, [fake_spec.instance_uuid]) - self.assertIsNotNone(self.client.queryclient.instance) mock_select_destinations.assert_called_once_with('ctxt', fake_spec, [fake_spec.instance_uuid], False, False) @@ -97,11 +95,8 @@ class SchedulerClientTestCase(test.NoDBTestCase): @mock.patch.object(scheduler_report_client.SchedulerReportClient, 'update_compute_node') def test_update_compute_node(self, mock_update_compute_node): - self.assertIsNone(self.client.reportclient.instance) - self.client.update_compute_node(mock.sentinel.ctx, mock.sentinel.cn) - self.assertIsNotNone(self.client.reportclient.instance) mock_update_compute_node.assert_called_once_with( mock.sentinel.ctx, mock.sentinel.cn)