openstack
/
openstacksdk
Code Issues Proposed changes

Add computed attribute type and location to base resource 

The shade objects all have a 'location' attribute which includes
information about the cloud, domain and project an object is from.

Add this to the base Resource object so that Resource objects start to
match shade objects. Also, add a new type of attribute, 'computed' to
set location to since location is not necessarily information from the
remote payload.

Also add an attribute parameter "coerce_to_default" which can be set to
tell the SDK to force the use of the default value if the attribute
would have been None or missing.

Change-Id: I315bc863bc87a17fb6f6a3e265a46b9e0acd41ed
This commit is contained in:
Monty Taylor 2018-08-06 08:47:12 -05:00
parent 2d472aea49
commit e3e9a9d39c
No known key found for this signature in database
GPG Key ID: 7BAE94BC7141A594
3 changed files with 165 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

171
openstack/resource.py
View File

@ -86,31 +86,41 @@ class _BaseComponent(object):
_map_cls = dict
def __init__(self, name, type=None, default=None, alias=None,
alternate_id=False, list_type=None, **kwargs):
alternate_id=False, list_type=None, coerce_to_default=False,
**kwargs):
"""A typed descriptor for a component that makes up a Resource
:param name: The name this component exists as on the server
:param type: The type this component is expected to be by the server.
By default this is None, meaning any value you specify
will work. If you specify type=dict and then set a
component to a string, __set__ will fail, for example.
:param type:
The type this component is expected to be by the server.
By default this is None, meaning any value you specify
will work. If you specify type=dict and then set a
component to a string, __set__ will fail, for example.
:param default: Typically None, but any other default can be set.
:param alias: If set, alternative attribute on object to return.
:param alternate_id: When `True`, this property is known
internally as a value that can be sent
with requests that require an ID but
when `id` is not a name the Resource has.
This is a relatively uncommon case, and this
setting should only be used once per Resource.
:param list_type: If type is `list`, list_type designates what the
type of the elements of the list should be.
:param alternate_id:
When `True`, this property is known internally as a value that
can be sent with requests that require an ID but when `id` is
not a name the Resource has. This is a relatively uncommon case,
and this setting should only be used once per Resource.
:param list_type:
If type is `list`, list_type designates what the type of the
elements of the list should be.
:param coerce_to_default:
If the Component is None or not present, force the given default
to be used. If a default is not given but a type is given,
construct an empty version of the type in question.
"""
self.name = name
self.type = type
self.default = default
if type is not None and coerce_to_default and not default:
self.default = type()
else:
self.default = default
self.alias = alias
self.alternate_id = alternate_id
self.list_type = list_type
self.coerce_to_default = coerce_to_default
def __get__(self, instance, owner):
if instance is None:
@ -150,6 +160,8 @@ class _BaseComponent(object):
return _convert_type(value, self.type, self.list_type)
def __set__(self, instance, value):
if self.coerce_to_default and value is None:
value = self.default
if value != self.default:
value = _convert_type(value, self.type, self.list_type)
@ -183,6 +195,12 @@ class URI(_BaseComponent):
key = "_uri"
class Computed(_BaseComponent):
"""Computed attributes"""
key = "_computed"
class _ComponentManager(collections.MutableMapping):
"""Storage of a component type"""
@ -313,8 +331,8 @@ class Resource(object):
id = Body("id")
#: The name of this resource.
name = Body("name")
#: The location of this resource.
location = Header("Location")
#: The OpenStack location of this resource.
location = Computed('location')
#: Mapping of accepted query parameter names.
_query_mapping = QueryParameters()
@ -359,35 +377,57 @@ class Resource(object):
#: API microversion (string or None) this Resource was loaded with
microversion = None
def __init__(self, _synchronized=False, **attrs):
_connection = None
_body = None
_header = None
_uri = None
_computed = None
def __init__(self, _synchronized=False, connection=None, **attrs):
"""The base resource
:param bool _synchronized: This is not intended to be used directly.
See :meth:`~openstack.resource.Resource.new` and
:meth:`~openstack.resource.Resource.existing`.
:param bool _synchronized:
This is not intended to be used directly. See
:meth:`~openstack.resource.Resource.new` and
:meth:`~openstack.resource.Resource.existing`.
:param openstack.connection.Connection connection:
Reference to the Connection being used. Defaults to None to allow
Resource objects to be used without an active Connection, such as
in unit tests. Use of ``self._connection`` in Resource code should
protect itself with a check for None.
"""
self._connection = connection
self.microversion = attrs.pop('microversion', None)
# NOTE: _collect_attrs modifies **attrs in place, removing
# items as they match up with any of the body, header,
# or uri mappings.
body, header, uri = self._collect_attrs(attrs)
body, header, uri, computed = self._collect_attrs(attrs)
# TODO(briancurtin): at this point if attrs has anything left
# they're not being set anywhere. Log this? Raise exception?
# How strict should we be here? Should strict be an option?
self._body = _ComponentManager(attributes=body,
synchronized=_synchronized)
self._header = _ComponentManager(attributes=header,
synchronized=_synchronized)
self._uri = _ComponentManager(attributes=uri,
synchronized=_synchronized)
self._body = _ComponentManager(
attributes=body,
synchronized=_synchronized)
self._header = _ComponentManager(
attributes=header,
synchronized=_synchronized)
self._uri = _ComponentManager(
attributes=uri,
synchronized=_synchronized)
self._computed = _ComponentManager(
attributes=computed,
synchronized=_synchronized)
def __repr__(self):
pairs = ["%s=%s" % (k, v) for k, v in dict(itertools.chain(
self._body.attributes.items(),
self._header.attributes.items(),
self._uri.attributes.items())).items()]
pairs = [
"%s=%s" % (k, v if v is not None else 'None')
for k, v in dict(itertools.chain(
self._body.attributes.items(),
self._header.attributes.items(),
self._uri.attributes.items(),
self._computed.attributes.items())).items()
]
args = ", ".join(pairs)
return "%s.%s(%s)" % (
@ -397,9 +437,12 @@ class Resource(object):
"""Return True if another resource has the same contents"""
if not isinstance(comparand, Resource):
return False
return all([self._body.attributes == comparand._body.attributes,
self._header.attributes == comparand._header.attributes,
self._uri.attributes == comparand._uri.attributes])
return all([
self._body.attributes == comparand._body.attributes,
self._header.attributes == comparand._header.attributes,
self._uri.attributes == comparand._uri.attributes,
self._computed.attributes == comparand._computed.attributes,
])
def __getattribute__(self, name):
"""Return an attribute on this instance
@ -427,11 +470,12 @@ class Resource(object):
been created.
"""
self.microversion = attrs.pop('microversion', None)
body, header, uri = self._collect_attrs(attrs)
body, header, uri, computed = self._collect_attrs(attrs)
self._body.update(body)
self._header.update(header)
self._uri.update(uri)
self._computed.update(computed)
def _collect_attrs(self, attrs):
"""Given attributes, return a dict per type of attribute
@ -444,7 +488,25 @@ class Resource(object):
header = self._consume_header_attrs(attrs)
uri = self._consume_uri_attrs(attrs)
return body, header, uri
if any([body, header, uri]):
attrs = self._compute_attributes(body, header, uri)
body.update(self._consume_attrs(self._body_mapping(), attrs))
header.update(self._consume_attrs(self._header_mapping(), attrs))
uri.update(self._consume_attrs(self._uri_mapping(), attrs))
computed = self._consume_attrs(self._computed_mapping(), attrs)
# TODO(mordred) We should make a Location Resource and add it here
# instead of just the dict.
if self._connection:
computed['location'] = munch.unmunchify(
self._connection._openstackcloud.current_location)
return body, header, uri, computed
def _compute_attributes(self, body, header, uri):
"""Compute additional attributes from the remote resource."""
return {}
def _consume_body_attrs(self, attrs):
return self._consume_mapped_attrs(Body, attrs)
@ -539,6 +601,11 @@ class Resource(object):
"""Return all URI members of this class"""
return cls._get_mapping(URI)
@classmethod
def _computed_mapping(cls):
"""Return all URI members of this class"""
return cls._get_mapping(Computed)
@classmethod
def _alternate_id(cls):
"""Return the name of any value known as an alternate_id
@ -585,7 +652,7 @@ class Resource(object):
return cls(_synchronized=False, **kwargs)
@classmethod
def existing(cls, **kwargs):
def existing(cls, connection=None, **kwargs):
"""Create an instance of an existing remote resource.
When creating the instance set the ``_synchronized`` parameter
@ -598,10 +665,14 @@ class Resource(object):
:param dict kwargs: Each of the named arguments will be set as
attributes on the resulting Resource object.
"""
return cls(_synchronized=True, **kwargs)
res = cls(_synchronized=True, **kwargs)
# TODO(shade) Done as a second call rather than a constructor param
# because otherwise the mocking in the tests goes nuts.
res._connection = connection
return res
@classmethod
def _from_munch(cls, obj, synchronized=True):
def _from_munch(cls, obj, synchronized=True, connection=None):
"""Create an instance from a ``munch.Munch`` object.
This is intended as a temporary measure to convert between shade-style
@ -611,16 +682,18 @@ class Resource(object):
:param bool synchronized: whether this object already exists on server
Must be set to ``False`` for newly created objects.
"""
return cls(_synchronized=synchronized, **obj)
return cls(_synchronized=synchronized, connection=connection, **obj)
def to_dict(self, body=True, headers=True, ignore_none=False,
original_names=False):
def to_dict(self, body=True, headers=True, computed=True,
ignore_none=False, original_names=False):
"""Return a dictionary of this resource's contents
:param bool body: Include the :class:`~openstack.resource.Body`
attributes in the returned dictionary.
:param bool headers: Include the :class:`~openstack.resource.Header`
attributes in the returned dictionary.
:param bool computed: Include the :class:`~openstack.resource.Computed`
attributes in the returned dictionary.
:param bool ignore_none: When True, exclude key/value pairs where
the value is None. This will exclude
attributes that the server hasn't returned.
@ -637,9 +710,11 @@ class Resource(object):
components.append(Body)
if headers:
components.append(Header)
if computed:
components.append(Computed)
if not components:
raise ValueError(
"At least one of `body` or `headers` must be True")
"At least one of `body`, `headers` or `computed` must be True")
# isinstance stricly requires this to be a tuple
components = tuple(components)
@ -1082,6 +1157,10 @@ class Resource(object):
raw_resource.pop("self", None)
value = cls.existing(microversion=microversion, **raw_resource)
# TODO(shade) Done as a second call rather than a constructor
# param because otherwise the mocking in the tests goes nuts.
if hasattr(session, '_sdk_connection'):
value._connection = session._sdk_connection
marker = value.id
yield value
total_yielded += 1
@ -1189,6 +1268,10 @@ class Resource(object):
# Try to short-circuit by looking directly for a matching ID.
try:
match = cls.existing(id=name_or_id, **params)
# TODO(shade) Done as a second call rather than a constructor
# param because otherwise the mocking in the tests goes nuts.
if hasattr(session, '_sdk_connection'):
match._connection = session._sdk_connection
return match.fetch(session)
except exceptions.NotFoundException:
pass

54
openstack/tests/unit/test_resource.py
View File

@ -401,17 +401,22 @@ class TestResource(base.TestCase):
body = {"body": 1}
header = {"header": 2, "Location": "somewhere"}
uri = {"uri": 3}
everything = dict(itertools.chain(body.items(), header.items(),
uri.items()))
computed = {"computed": 4}
everything = dict(itertools.chain(
body.items(),
header.items(),
uri.items(),
computed.items(),
))
mock_collect = mock.Mock()
mock_collect.return_value = body, header, uri
mock_collect.return_value = body, header, uri, computed
with mock.patch.object(resource.Resource,
"_collect_attrs", mock_collect):
sot = resource.Resource(_synchronized=False, **everything)
mock_collect.assert_called_once_with(everything)
self.assertEqual("somewhere", sot.location)
self.assertIsNone(sot.location)
self.assertIsInstance(sot._body, resource._ComponentManager)
self.assertEqual(body, sot._body.dirty)
@ -433,6 +438,7 @@ class TestResource(base.TestCase):
a = {"a": 1}
b = {"b": 2}
c = {"c": 3}
d = {"d": 4}
class Test(resource.Resource):
def __init__(self):
@ -448,6 +454,10 @@ class TestResource(base.TestCase):
self._uri.attributes.items = mock.Mock(
return_value=c.items())
self._computed = mock.Mock()
self._computed.attributes.items = mock.Mock(
return_value=d.items())
the_repr = repr(Test())
# Don't test the arguments all together since the dictionary order
@ -456,6 +466,7 @@ class TestResource(base.TestCase):
self.assertIn("a=1", the_repr)
self.assertIn("b=2", the_repr)
self.assertIn("c=3", the_repr)
self.assertIn("d=4", the_repr)
def test_equality(self):
class Example(resource.Resource):
@ -477,11 +488,14 @@ class TestResource(base.TestCase):
body = "body"
header = "header"
uri = "uri"
computed = "computed"
sot._collect_attrs = mock.Mock(return_value=(body, header, uri))
sot._collect_attrs = mock.Mock(
return_value=(body, header, uri, computed))
sot._body.update = mock.Mock()
sot._header.update = mock.Mock()
sot._uri.update = mock.Mock()
sot._computed.update = mock.Mock()
args = {"arg": 1}
sot._update(**args)
@ -490,19 +504,7 @@ class TestResource(base.TestCase):
sot._body.update.assert_called_once_with(body)
sot._header.update.assert_called_once_with(header)
sot._uri.update.assert_called_once_with(uri)
def test__collect_attrs(self):
sot = resource.Resource()
expected_attrs = ["body", "header", "uri"]
sot._consume_attrs = mock.Mock()
sot._consume_attrs.side_effect = expected_attrs
# It'll get passed an empty dict at the least.
actual_attrs = sot._collect_attrs(dict())
self.assertItemsEqual(expected_attrs, actual_attrs)
sot._computed.update.assert_called_with(computed)
def test__consume_attrs(self):
serverside_key1 = "someKey1"
@ -538,7 +540,7 @@ class TestResource(base.TestCase):
# Check that even on an empty class, we get the expected
# built-in attributes.
self.assertIn("location", resource.Resource._header_mapping())
self.assertIn("location", resource.Resource._computed_mapping())
self.assertIn("name", resource.Resource._body_mapping())
self.assertIn("id", resource.Resource._body_mapping())
@ -695,7 +697,8 @@ class TestResource(base.TestCase):
expected = {
'id': 'FAKE_ID',
'name': None,
'bar': None
'bar': None,
'location': None,
}
self.assertEqual(expected, res.to_dict(headers=False))
@ -744,10 +747,13 @@ class TestResource(base.TestCase):
res = Test(id='FAKE_ID')
err = self.assertRaises(ValueError,
res.to_dict, body=False, headers=False)
self.assertEqual('At least one of `body` or `headers` must be True',
six.text_type(err))
err = self.assertRaises(
ValueError,
res.to_dict,
body=False, headers=False, computed=False)
self.assertEqual(
'At least one of `body`, `headers` or `computed` must be True',
six.text_type(err))
def test_to_dict_with_mro_no_override(self):

8
releasenotes/notes/shade-location-b0d2e5cae743b738.yaml Normal file
View File

@ -0,0 +1,8 @@
---
upgrade:
- |
The base ``Resource`` field ``location`` is no longer drawn from the
``Location`` HTTP header, but is instead a dict containing information
about cloud, domain and project. The location dict is a feature of shade
objects and is being added to all objects as part of the alignment of
shade and sdk.