openstack/openstacksdk
Code Issues Proposed changes

Remove the need for OpenStackConfig in CloudRegion

Browse Source 
There are a few things in CloudRegion that require calling back out to
the creating OpenStackConfig object. However, now that creating a
CloudRegion directly is a thing, those calls have become more awkward.

Shift the cache settings and methods to CloudRegion along with the
extra_config logic. Move two of the utility methods in loader into a
_util file so that both loader and cloud_region can import it. This
renames get_cache_expiration to get_cache_expirations, but I'm fairly
confident that the only consumer of get_cache_expirations here is
openstack.cloud.

Leave passing in the OpenStackConfig for now, because osc uses it.
However, if we can get osc shifted to using get_password_callback, we
can get rid of it.

Change-Id: Ia6ed0f00bfc2483bd09169811198cdf1a0ab2f15
This commit is contained in:
Monty Taylor
2018-02-23 08:50:23 -06:00
parent dc564548e1
commit 35edf460bc
5 changed files with 85 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
openstack/cloud/openstackcloud.py
View File

@@ -196,7 +196,7 @@ class OpenStackCloud(_normalize.Normalizer):
self.cache_enabled = True
self._cache = self._make_cache(
cache_class, cache_expiration_time, cache_arguments)
expirations = self.config.get_cache_expiration()
expirations = self.config.get_cache_expirations()
for expire_key in expirations.keys():
# Only build caches for things we have list operations for
if getattr(

14
openstack/config/_util.py
View File

@@ -29,3 +29,17 @@ def normalize_keys(config):
else:
new_config[key] = value
return new_config
def merge_clouds(old_dict, new_dict):
"""Like dict.update, except handling nested dicts."""
ret = old_dict.copy()
for (k, v) in new_dict.items():
if isinstance(v, dict):
if k in ret:
ret[k] = merge_clouds(ret[k], v)
else:
ret[k] = v.copy()
else:
ret[k] = v
return ret

64
openstack/config/cloud_region.py
View File

@@ -12,6 +12,7 @@
# License for the specific language governing permissions and limitations
# under the License.
import copy
import math
import warnings
@@ -79,10 +80,14 @@ class CloudRegion(object):
force_ipv4=False, auth_plugin=None,
openstack_config=None, session_constructor=None,
app_name=None, app_version=None, session=None,
discovery_cache=None):
discovery_cache=None, extra_config=None,
cache_expiration_time=0, cache_expirations=None,
cache_path=None, cache_class='dogpile.cache.null',
cache_arguments=None, password_callback=None):
self._name = name
self.region_name = region_name
self.config = _util.normalize_keys(config)
self._extra_config = extra_config or {}
self.log = _log.setup_logging('openstack.config')
self._force_ipv4 = force_ipv4
self._auth = auth_plugin
@@ -92,6 +97,12 @@ class CloudRegion(object):
self._app_name = app_name
self._app_version = app_version
self._discovery_cache = discovery_cache or None
self._cache_expiration_time = cache_expiration_time
self._cache_expirations = cache_expirations or {}
self._cache_path = cache_path
self._cache_class = cache_class
self._cache_arguments = cache_arguments
self._password_callback = password_callback
def __getattr__(self, key):
"""Return arbitrary attributes."""
@@ -398,26 +409,20 @@ class CloudRegion(object):
return endpoint
def get_cache_expiration_time(self):
if self._openstack_config:
return self._openstack_config.get_cache_expiration_time()
return 0
# TODO(mordred) We should be validating/transforming this on input
return int(self._cache_expiration_time)
def get_cache_path(self):
if self._openstack_config:
return self._openstack_config.get_cache_path()
return self._cache_path
def get_cache_class(self):
if self._openstack_config:
return self._openstack_config.get_cache_class()
return 'dogpile.cache.null'
return self._cache_class
def get_cache_arguments(self):
if self._openstack_config:
return self._openstack_config.get_cache_arguments()
return copy.deepcopy(self._cache_arguments)
def get_cache_expiration(self):
if self._openstack_config:
return self._openstack_config.get_cache_expiration()
def get_cache_expirations(self):
return copy.deepcopy(self._cache_expirations)
def get_cache_resource_expiration(self, resource, default=None):
"""Get expiration time for a resource
@@ -428,11 +433,9 @@ class CloudRegion(object):
:returns: Expiration time for the resource type as float or default
"""
if self._openstack_config:
expiration = self._openstack_config.get_cache_expiration()
if resource not in expiration:
return default
return float(expiration[resource])
if resource not in self._cache_expirations:
return default
return float(self._cache_expirations[resource])
def requires_floating_ip(self):
"""Return whether or not this cloud requires floating ips.
@@ -503,6 +506,20 @@ class CloudRegion(object):
return net['name']
return None
def _get_extra_config(self, key, defaults=None):
"""Fetch an arbitrary extra chunk of config, laying in defaults.
:param string key: name of the config section to fetch
:param dict defaults: (optional) default values to merge under the
found config
"""
defaults = _util.normalize_keys(defaults or {})
if not key:
return defaults
return _util.merge_clouds(
defaults,
_util.normalize_keys(self._extra_config.get(key, {})))
def get_client_config(self, name=None, defaults=None):
"""Get config settings for a named client.
@@ -520,7 +537,8 @@ class CloudRegion(object):
A dict containing merged settings from the named section, the
client section and the defaults.
"""
if not self._openstack_config:
return defaults or {}
return self._openstack_config.get_extra_config(
name, self._openstack_config.get_extra_config('client', defaults))
return self._get_extra_config(
name, self._get_extra_config('client', defaults))
def get_password_callback(self):
return self._password_callback

63
openstack/config/loader.py
View File

@@ -83,20 +83,6 @@ def get_boolean(value):
return False
def _merge_clouds(old_dict, new_dict):
"""Like dict.update, except handling nested dicts."""
ret = old_dict.copy()
for (k, v) in new_dict.items():
if isinstance(v, dict):
if k in ret:
ret[k] = _merge_clouds(ret[k], v)
else:
ret[k] = v.copy()
else:
ret[k] = v
return ret
def _auth_update(old_dict, new_dict_source):
"""Like dict.update, except handling the nested dict called auth."""
new_dict = copy.deepcopy(new_dict_source)
@@ -186,7 +172,7 @@ class OpenStackConfig(object):
self.config_filename, self.cloud_config = self._load_config_file()
_, secure_config = self._load_secure_file()
if secure_config:
self.cloud_config = _merge_clouds(
self.cloud_config = _util.merge_clouds(
self.cloud_config, secure_config)
if not self.cloud_config:
@@ -194,6 +180,10 @@ class OpenStackConfig(object):
if 'clouds' not in self.cloud_config:
self.cloud_config['clouds'] = {}
# Save the other config
self.extra_config = copy.deepcopy(self.cloud_config)
self.extra_config.pop('clouds', None)
# Grab ipv6 preference settings from env
client_config = self.cloud_config.get('client', {})
@@ -261,7 +251,7 @@ class OpenStackConfig(object):
self._cache_path = CACHE_PATH
self._cache_class = 'dogpile.cache.null'
self._cache_arguments = {}
self._cache_expiration = {}
self._cache_expirations = {}
if 'cache' in self.cloud_config:
cache_settings = _util.normalize_keys(self.cloud_config['cache'])
@@ -283,8 +273,8 @@ class OpenStackConfig(object):
cache_settings.get('path', self._cache_path))
self._cache_arguments = cache_settings.get(
'arguments', self._cache_arguments)
self._cache_expiration = cache_settings.get(
'expiration', self._cache_expiration)
self._cache_expirations = cache_settings.get(
'expiration', self._cache_expirations)
# Flag location to hold the peeked value of an argparse timeout value
self._argv_timeout = False
@@ -331,7 +321,7 @@ class OpenStackConfig(object):
defaults = _util.normalize_keys(defaults or {})
if not key:
return defaults
return _merge_clouds(
return _util.merge_clouds(
defaults,
_util.normalize_keys(self.cloud_config.get(key, {})))
@@ -354,27 +344,6 @@ class OpenStackConfig(object):
return path, yaml.safe_load(f)
return (None, {})
def get_cache_expiration_time(self):
return int(self._cache_expiration_time)
def get_cache_interval(self):
return self.get_cache_expiration_time()
def get_cache_max_age(self):
return self.get_cache_expiration_time()
def get_cache_path(self):
return self._cache_path
def get_cache_class(self):
return self._cache_class
def get_cache_arguments(self):
return copy.deepcopy(self._cache_arguments)
def get_cache_expiration(self):
return copy.deepcopy(self._cache_expiration)
def _expand_region_name(self, region_name):
return {'name': region_name, 'values': {}}
@@ -1092,12 +1061,19 @@ class OpenStackConfig(object):
name=cloud_name,
region_name=config['region_name'],
config=config,
extra_config=self.extra_config,
force_ipv4=force_ipv4,
auth_plugin=auth_plugin,
openstack_config=self,
session_constructor=self._session_constructor,
app_name=self._app_name,
app_version=self._app_version,
cache_expiration_time=self._cache_expiration_time,
cache_expirations=self._cache_expirations,
cache_path=self._cache_path,
cache_class=self._cache_class,
cache_arguments=self._cache_arguments,
password_callback=self._pw_callback,
)
# TODO(mordred) Backwards compat for OSC transition
get_one_cloud = get_one
@@ -1189,9 +1165,16 @@ class OpenStackConfig(object):
name=cloud_name,
region_name=config['region_name'],
config=config,
extra_config=self.extra_config,
force_ipv4=force_ipv4,
auth_plugin=auth_plugin,
openstack_config=self,
cache_expiration_time=self._cache_expiration_time,
cache_expirations=self._cache_expirations,
cache_path=self._cache_path,
cache_class=self._cache_class,
cache_arguments=self._cache_arguments,
password_callback=self._pw_callback,
)
@staticmethod

6
releasenotes/notes/make-cloud-region-standalone-848a2c4b5f3ebc29.yaml Normal file
View File

@@ -0,0 +1,6 @@
---
features:
- |
Updated the ``openstack.config.cloud_config.CloudRegion`` object to be
able to store and retreive cache settings and the password callback object
without needing an ``openstack.config.loader.OpenStackConfig`` object.