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
This commit is contained in:
parent
820790225c
commit
879f7d4f12
|
@ -166,9 +166,8 @@ def from_conf(conf, session=None, **kwargs):
|
||||||
"for project '{project}' (service type "
|
"for project '{project}' (service type "
|
||||||
"'{service_type}'): {exception}".format(
|
"'{service_type}'): {exception}".format(
|
||||||
project=project_name, service_type=st, exception=e))
|
project=project_name, service_type=st, exception=e))
|
||||||
_logger.warn("Disabling service '{service_type}'.".format(
|
_logger.warn("Disabling service '{service_type}': {reason}".format(
|
||||||
service_type=st))
|
service_type=st, reason=reason))
|
||||||
_logger.warn(reason)
|
|
||||||
_disable_service(config_dict, st, reason=reason)
|
_disable_service(config_dict, st, reason=reason)
|
||||||
continue
|
continue
|
||||||
# Load them into config_dict under keys prefixed by ${service_type}_
|
# 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):
|
class CloudRegion(object):
|
||||||
|
# TODO(efried): Doc the rest of the kwargs
|
||||||
"""The configuration for a Region of an OpenStack Cloud.
|
"""The configuration for a Region of an OpenStack Cloud.
|
||||||
|
|
||||||
A CloudRegion encapsulates the config information needed for connections
|
A CloudRegion encapsulates the config information needed for connections
|
||||||
to all of the services in a Region of a Cloud.
|
to all of the services in a Region of a Cloud.
|
||||||
|
|
||||||
TODO(efried): Doc the rest of the kwargs
|
|
||||||
|
|
||||||
:param str region_name:
|
:param str region_name:
|
||||||
The default region name for all services in this CloudRegion. If
|
The default region name for all services in this CloudRegion. If
|
||||||
both ``region_name`` and ``config['region_name'] are specified, the
|
both ``region_name`` and ``config['region_name'] are specified, the
|
||||||
|
@ -265,8 +263,6 @@ class CloudRegion(object):
|
||||||
def __eq__(self, other):
|
def __eq__(self, other):
|
||||||
return (
|
return (
|
||||||
self.name == other.name
|
self.name == other.name
|
||||||
# Ew
|
|
||||||
and self.get_region_name() == other.get_region_name()
|
|
||||||
and self.config == other.config)
|
and self.config == other.config)
|
||||||
|
|
||||||
def __ne__(self, other):
|
def __ne__(self, other):
|
||||||
|
|
|
@ -182,25 +182,25 @@ class TestCloudRegion(base.TestCase):
|
||||||
self.assertEqual(3, cc.get_connect_retries('baremetal'))
|
self.assertEqual(3, cc.get_connect_retries('baremetal'))
|
||||||
|
|
||||||
def test_get_region_name(self):
|
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
|
# No region_name kwarg, no regions specified in services dict
|
||||||
# (including the default).
|
# (including the default).
|
||||||
cc = cloud_region.CloudRegion(config=fake_services_dict)
|
cc = cloud_region.CloudRegion(config=fake_services_dict)
|
||||||
self.assertIsNone(cc.region_name)
|
assert_region_name(None, None)
|
||||||
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'))
|
|
||||||
|
|
||||||
# Only region_name kwarg; it's returned for everything
|
# Only region_name kwarg; it's returned for everything
|
||||||
cc = cloud_region.CloudRegion(
|
cc = cloud_region.CloudRegion(
|
||||||
region_name='foo', config=fake_services_dict)
|
region_name='foo', config=fake_services_dict)
|
||||||
self.assertEqual('foo', cc.region_name)
|
assert_region_name('foo', 'foo')
|
||||||
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'))
|
|
||||||
|
|
||||||
# No region_name kwarg; values (including default) show through from
|
# No region_name kwarg; values (including default) show through from
|
||||||
# config dict
|
# config dict
|
||||||
|
@ -208,13 +208,7 @@ class TestCloudRegion(base.TestCase):
|
||||||
fake_services_dict,
|
fake_services_dict,
|
||||||
region_name='the-default', compute_region_name='compute-region')
|
region_name='the-default', compute_region_name='compute-region')
|
||||||
cc = cloud_region.CloudRegion(config=services_dict)
|
cc = cloud_region.CloudRegion(config=services_dict)
|
||||||
self.assertEqual('the-default', cc.region_name)
|
assert_region_name('the-default', 'compute-region')
|
||||||
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'))
|
|
||||||
|
|
||||||
# region_name kwarg overrides config dict default (for backward
|
# region_name kwarg overrides config dict default (for backward
|
||||||
# compatibility), but service-specific region_name takes precedence.
|
# compatibility), but service-specific region_name takes precedence.
|
||||||
|
@ -223,12 +217,7 @@ class TestCloudRegion(base.TestCase):
|
||||||
region_name='dict', compute_region_name='compute-region')
|
region_name='dict', compute_region_name='compute-region')
|
||||||
cc = cloud_region.CloudRegion(
|
cc = cloud_region.CloudRegion(
|
||||||
region_name='kwarg', config=services_dict)
|
region_name='kwarg', config=services_dict)
|
||||||
self.assertEqual('kwarg', cc.region_name)
|
assert_region_name('kwarg', 'compute-region')
|
||||||
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'))
|
|
||||||
|
|
||||||
def test_aliases(self):
|
def test_aliases(self):
|
||||||
services_dict = fake_services_dict.copy()
|
services_dict = fake_services_dict.copy()
|
||||||
|
|
Loading…
Reference in New Issue