From 879f7d4f12216710defc4cb5ffe0e9f2c00cda64 Mon Sep 17 00:00:00 2001 From: Eric Fried Date: Thu, 6 Jun 2019 09:28:48 -0500 Subject: [PATCH] Minor fixups from from_conf changes Addresses the following nonblocking nits from previous patches: - Consolidate warning logs when disabling service due to exception when processing oslo.config ksa settings [1]. - Move a TODO out of a docstring [2]. - Remove a now-redundant region_name comparison in CloudRegion.__eq__ [3]. - DRY the test_get_region_name unit test [4]. [1] https://review.opendev.org/#/c/663439/3/openstack/config/cloud_region.py@171 [2] https://review.opendev.org/#/c/662865/4/openstack/config/cloud_region.py@156 [3] https://review.opendev.org/#/c/662865/4/openstack/config/cloud_region.py@237 [4] https://review.opendev.org/#/c/662865/4/openstack/tests/unit/config/test_cloud_config.py@185 Change-Id: I1c140677347cf40db11e75f6a868356300d85071 --- openstack/config/cloud_region.py | 10 ++--- .../tests/unit/config/test_cloud_config.py | 37 +++++++------------ 2 files changed, 16 insertions(+), 31 deletions(-) diff --git a/openstack/config/cloud_region.py b/openstack/config/cloud_region.py index 99a85033a..abce4e97b 100644 --- a/openstack/config/cloud_region.py +++ b/openstack/config/cloud_region.py @@ -166,9 +166,8 @@ def from_conf(conf, session=None, **kwargs): "for project '{project}' (service type " "'{service_type}'): {exception}".format( project=project_name, service_type=st, exception=e)) - _logger.warn("Disabling service '{service_type}'.".format( - service_type=st)) - _logger.warn(reason) + _logger.warn("Disabling service '{service_type}': {reason}".format( + service_type=st, reason=reason)) _disable_service(config_dict, st, reason=reason) continue # Load them into config_dict under keys prefixed by ${service_type}_ @@ -180,13 +179,12 @@ def from_conf(conf, session=None, **kwargs): class CloudRegion(object): + # TODO(efried): Doc the rest of the kwargs """The configuration for a Region of an OpenStack Cloud. A CloudRegion encapsulates the config information needed for connections to all of the services in a Region of a Cloud. - TODO(efried): Doc the rest of the kwargs - :param str region_name: The default region name for all services in this CloudRegion. If both ``region_name`` and ``config['region_name'] are specified, the @@ -265,8 +263,6 @@ class CloudRegion(object): def __eq__(self, other): return ( self.name == other.name - # Ew - and self.get_region_name() == other.get_region_name() and self.config == other.config) def __ne__(self, other): diff --git a/openstack/tests/unit/config/test_cloud_config.py b/openstack/tests/unit/config/test_cloud_config.py index 2bfcbcefa..718450217 100644 --- a/openstack/tests/unit/config/test_cloud_config.py +++ b/openstack/tests/unit/config/test_cloud_config.py @@ -182,25 +182,25 @@ class TestCloudRegion(base.TestCase): self.assertEqual(3, cc.get_connect_retries('baremetal')) def test_get_region_name(self): - # TODO(efried): ddt this + + def assert_region_name(default, compute): + self.assertEqual(default, cc.region_name) + self.assertEqual(default, cc.get_region_name()) + self.assertEqual(default, cc.get_region_name(service_type=None)) + self.assertEqual( + compute, cc.get_region_name(service_type='compute')) + self.assertEqual( + default, cc.get_region_name(service_type='placement')) # No region_name kwarg, no regions specified in services dict # (including the default). cc = cloud_region.CloudRegion(config=fake_services_dict) - self.assertIsNone(cc.region_name) - self.assertIsNone(cc.get_region_name()) - self.assertIsNone(cc.get_region_name(service_type=None)) - self.assertIsNone(cc.get_region_name(service_type='compute')) - self.assertIsNone(cc.get_region_name(service_type='placement')) + assert_region_name(None, None) # Only region_name kwarg; it's returned for everything cc = cloud_region.CloudRegion( region_name='foo', config=fake_services_dict) - self.assertEqual('foo', cc.region_name) - self.assertEqual('foo', cc.get_region_name()) - self.assertEqual('foo', cc.get_region_name(service_type=None)) - self.assertEqual('foo', cc.get_region_name(service_type='compute')) - self.assertEqual('foo', cc.get_region_name(service_type='placement')) + assert_region_name('foo', 'foo') # No region_name kwarg; values (including default) show through from # config dict @@ -208,13 +208,7 @@ class TestCloudRegion(base.TestCase): fake_services_dict, region_name='the-default', compute_region_name='compute-region') cc = cloud_region.CloudRegion(config=services_dict) - self.assertEqual('the-default', cc.region_name) - self.assertEqual('the-default', cc.get_region_name()) - self.assertEqual('the-default', cc.get_region_name(service_type=None)) - self.assertEqual( - 'compute-region', cc.get_region_name(service_type='compute')) - self.assertEqual( - 'the-default', cc.get_region_name(service_type='placement')) + assert_region_name('the-default', 'compute-region') # region_name kwarg overrides config dict default (for backward # compatibility), but service-specific region_name takes precedence. @@ -223,12 +217,7 @@ class TestCloudRegion(base.TestCase): region_name='dict', compute_region_name='compute-region') cc = cloud_region.CloudRegion( region_name='kwarg', config=services_dict) - self.assertEqual('kwarg', cc.region_name) - self.assertEqual('kwarg', cc.get_region_name()) - self.assertEqual('kwarg', cc.get_region_name(service_type=None)) - self.assertEqual( - 'compute-region', cc.get_region_name(service_type='compute')) - self.assertEqual('kwarg', cc.get_region_name(service_type='placement')) + assert_region_name('kwarg', 'compute-region') def test_aliases(self): services_dict = fake_services_dict.copy()