Merge "resource: Merge unnecessary separation of logic"

This commit is contained in:
Zuul 2022-08-17 18:01:41 +00:00 committed by Gerrit Code Review
commit 433815dde5
18 changed files with 151 additions and 109 deletions

View File

@ -74,7 +74,7 @@ class AcceleratorRequest(resource.Resource):
request = self._prepare_request(prepend_key=prepend_key,
base_path=base_path, patch=True)
microversion = self._get_microversion_for(session, 'patch')
microversion = self._get_microversion(session, action='patch')
if patch:
request.body = self._convert_patch(patch)

View File

@ -768,7 +768,7 @@ class Node(_common.ListMixin, resource.Resource):
fails for a required interface.
"""
session = self._get_session(session)
version = self._get_microversion_for(session, 'fetch')
version = self._get_microversion(session, action='fetch')
request = self._prepare_request(requires_id=True)
request.url = utils.urljoin(request.url, 'validate')
@ -818,7 +818,7 @@ class Node(_common.ListMixin, resource.Resource):
def _do_maintenance_action(self, session, verb, body=None):
session = self._get_session(session)
version = self._get_microversion_for(session, 'commit')
version = self._get_microversion(session, action='commit')
request = self._prepare_request(requires_id=True)
request.url = utils.urljoin(request.url, 'maintenance')
response = getattr(session, verb)(
@ -837,7 +837,7 @@ class Node(_common.ListMixin, resource.Resource):
reboot
"""
session = self._get_session(session)
version = self._get_microversion_for(session, 'commit')
version = self._get_microversion(session, action='commit')
request = self._prepare_request(requires_id=True)
request.url = utils.urljoin(request.url, 'management', 'boot_device')
@ -1007,7 +1007,7 @@ class Node(_common.ListMixin, resource.Resource):
:returns: The HTTP response.
"""
session = self._get_session(session)
version = self._get_microversion_for(session, 'commit')
version = self._get_microversion(session, action='commit')
request = self._prepare_request(requires_id=True)
request.url = utils.urljoin(request.url, 'vendor_passthru?method={}'
.format(method))
@ -1032,7 +1032,7 @@ class Node(_common.ListMixin, resource.Resource):
:returns: The HTTP response.
"""
session = self._get_session(session)
version = self._get_microversion_for(session, 'fetch')
version = self._get_microversion(session, action='fetch')
request = self._prepare_request(requires_id=True)
request.url = utils.urljoin(request.url, 'vendor_passthru/methods')

View File

@ -63,7 +63,7 @@ class Introspection(resource.Resource):
session = self._get_session(session)
version = self._get_microversion_for(session, 'delete')
version = self._get_microversion(session, action='delete')
request = self._prepare_request(requires_id=True)
request.url = utils.urljoin(request.url, 'abort')
response = session.post(
@ -89,7 +89,7 @@ class Introspection(resource.Resource):
"""
session = self._get_session(session)
version = (self._get_microversion_for(session, 'fetch')
version = (self._get_microversion(session, action='fetch')
if processed else '1.17')
request = self._prepare_request(requires_id=True)
request.url = utils.urljoin(request.url, 'data')

View File

@ -96,7 +96,7 @@ class Backup(resource.Resource):
raise exceptions.MethodNotSupported(self, "create")
session = self._get_session(session)
microversion = self._get_microversion_for(session, 'create')
microversion = self._get_microversion(session, action='create')
requires_id = (self.create_requires_id
if self.create_requires_id is not None
else self.create_method == 'PUT')

View File

@ -106,7 +106,7 @@ class Backup(resource.Resource):
raise exceptions.MethodNotSupported(self, "create")
session = self._get_session(session)
microversion = self._get_microversion_for(session, 'create')
microversion = self._get_microversion(session, action='create')
requires_id = (self.create_requires_id
if self.create_requires_id is not None
else self.create_method == 'PUT')

View File

@ -44,7 +44,7 @@ class Group(resource.Resource):
def _action(self, session, body):
"""Preform group actions given the message body."""
session = self._get_session(session)
microversion = self._get_microversion_for(session, 'create')
microversion = self._get_microversion(session, action='create')
url = utils.urljoin(self.base_path, self.id, 'action')
response = session.post(url, json=body, microversion=microversion)
exceptions.raise_from_response(response)
@ -71,7 +71,7 @@ class Group(resource.Resource):
):
"""Creates a new group from source."""
session = cls._get_session(session)
microversion = cls._get_microversion_for(cls, session, 'create')
microversion = cls._get_microversion(session, action='create')
url = utils.urljoin(cls.base_path, 'action')
body = {
'create-from-src': {

View File

@ -51,7 +51,7 @@ class GroupType(resource.Resource):
:returns: An updated version of this object.
"""
url = utils.urljoin(GroupType.base_path, self.id, 'group_specs')
microversion = self._get_microversion_for(session, 'fetch')
microversion = self._get_microversion(session, action='fetch')
response = session.get(url, microversion=microversion)
exceptions.raise_from_response(response)
specs = response.json().get('group_specs', {})
@ -69,7 +69,7 @@ class GroupType(resource.Resource):
:returns: An updated version of this object.
"""
url = utils.urljoin(GroupType.base_path, self.id, 'group_specs')
microversion = self._get_microversion_for(session, 'create')
microversion = self._get_microversion(session, action='create')
response = session.post(
url, json={'group_specs': specs}, microversion=microversion,
)
@ -86,7 +86,7 @@ class GroupType(resource.Resource):
:returns: The value of the group spec property.
"""
url = utils.urljoin(GroupType.base_path, self.id, 'group_specs', prop)
microversion = self._get_microversion_for(session, 'fetch')
microversion = self._get_microversion(session, action='fetch')
response = session.get(url, microversion=microversion)
exceptions.raise_from_response(response)
val = response.json().get(prop)
@ -101,7 +101,7 @@ class GroupType(resource.Resource):
:returns: The updated value of the group spec property.
"""
url = utils.urljoin(GroupType.base_path, self.id, 'group_specs', prop)
microversion = self._get_microversion_for(session, 'commit')
microversion = self._get_microversion(session, action='commit')
response = session.put(
url, json={prop: val}, microversion=microversion
)
@ -117,6 +117,6 @@ class GroupType(resource.Resource):
:returns: None
"""
url = utils.urljoin(GroupType.base_path, self.id, 'group_specs', prop)
microversion = self._get_microversion_for(session, 'delete')
microversion = self._get_microversion(session, action='delete')
response = session.delete(url, microversion=microversion)
exceptions.raise_from_response(response)

View File

@ -145,7 +145,7 @@ class Flavor(resource.Resource):
before that a separate call is required.
"""
url = utils.urljoin(Flavor.base_path, self.id, 'os-extra_specs')
microversion = self._get_microversion_for(session, 'fetch')
microversion = self._get_microversion(session, action='fetch')
response = session.get(url, microversion=microversion)
exceptions.raise_from_response(response)
specs = response.json().get('extra_specs', {})
@ -155,7 +155,7 @@ class Flavor(resource.Resource):
def create_extra_specs(self, session, specs):
"""Creates extra specs for a flavor"""
url = utils.urljoin(Flavor.base_path, self.id, 'os-extra_specs')
microversion = self._get_microversion_for(session, 'create')
microversion = self._get_microversion(session, action='create')
response = session.post(
url,
json={'extra_specs': specs},
@ -169,7 +169,7 @@ class Flavor(resource.Resource):
"""Get individual extra_spec property"""
url = utils.urljoin(Flavor.base_path, self.id,
'os-extra_specs', prop)
microversion = self._get_microversion_for(session, 'fetch')
microversion = self._get_microversion(session, action='fetch')
response = session.get(url, microversion=microversion)
exceptions.raise_from_response(response)
val = response.json().get(prop)
@ -179,7 +179,7 @@ class Flavor(resource.Resource):
"""Update An Extra Spec For A Flavor"""
url = utils.urljoin(Flavor.base_path, self.id,
'os-extra_specs', prop)
microversion = self._get_microversion_for(session, 'commit')
microversion = self._get_microversion(session, action='commit')
response = session.put(
url,
json={prop: val},
@ -192,7 +192,7 @@ class Flavor(resource.Resource):
"""Delete An Extra Spec For A Flavor"""
url = utils.urljoin(Flavor.base_path, self.id,
'os-extra_specs', prop)
microversion = self._get_microversion_for(session, 'delete')
microversion = self._get_microversion(session, action='delete')
response = session.delete(
url,
microversion=microversion)

View File

@ -91,7 +91,7 @@ class Hypervisor(resource.Resource):
raise exceptions.SDKException(
'Hypervisor.get_uptime is not supported anymore')
url = utils.urljoin(self.base_path, self.id, 'uptime')
microversion = self._get_microversion_for(session, 'fetch')
microversion = self._get_microversion(session, action='fetch')
response = session.get(
url, microversion=microversion)
self._translate_response(response)

View File

@ -54,38 +54,86 @@ class ServerGroup(resource.Resource):
#: The user ID who owns the server group
user_id = resource.Body('user_id')
def _get_microversion_for(self, session, action):
"""Get microversion to use for the given action.
# TODO(stephenfin): It would be nice to have a hookpoint to do this
# microversion-based request manipulation, but we don't have anything like
# that right now
def create(self, session, prepend_key=True, base_path=None, **params):
"""Create a remote resource based on this instance.
The base version uses :meth:`_get_microversion_for_list`.
Subclasses can override this method if more complex logic is needed.
:param session: :class`keystoneauth1.adapter.Adapter`
:param action: One of "fetch", "commit", "create", "delete", "patch".
Unused in the base implementation.
:return: microversion as string or ``None``
:param session: The session to use for making this request.
:type session: :class:`~keystoneauth1.adapter.Adapter`
:param prepend_key: A boolean indicating whether the resource_key
should be prepended in a resource creation request. Default to
True.
:param str base_path: Base part of the URI for creating resources, if
different from :data:`~openstack.resource.Resource.base_path`.
:param dict params: Additional params to pass.
:return: This :class:`Resource` instance.
:raises: :exc:`~openstack.exceptions.MethodNotSupported` if
:data:`Resource.allow_create` is not set to ``True``.
"""
if action not in ('fetch', 'commit', 'create', 'delete', 'patch'):
raise ValueError('Invalid action: %s' % action)
if not self.allow_create:
raise exceptions.MethodNotSupported(self, 'create')
microversion = self._get_microversion_for_list(session)
if action == 'create':
# `policy` and `rules` are added with mv=2.64. In it also
# `policies` are removed.
if utils.supports_microversion(session, '2.64'):
if self.policies:
if not self.policy and isinstance(self.policies, list):
self.policy = self.policies[0]
self._body.clean(only={'policies'})
microversion = self._max_microversion
else:
if self.rules:
message = ("API version %s is required to set rules, but "
"it is not available.") % 2.64
raise exceptions.NotSupported(message)
if self.policy:
if not self.policies:
self.policies = [self.policy]
self._body.clean(only={'policy'})
session = self._get_session(session)
microversion = self._get_microversion(session, action='create')
requires_id = (
self.create_requires_id
if self.create_requires_id is not None
else self.create_method == 'PUT'
)
return microversion
if self.create_exclude_id_from_body:
self._body._dirty.discard("id")
# `policy` and `rules` are added with mv=2.64. In it also
# `policies` are removed.
if utils.supports_microversion(session, '2.64'):
if self.policies:
if not self.policy and isinstance(self.policies, list):
self.policy = self.policies[0]
self._body.clean(only={'policies'})
microversion = self._max_microversion
else: # microversion < 2.64
if self.rules:
msg = (
"API version 2.64 is required to set rules, but "
"it is not available."
)
raise exceptions.NotSupported(msg)
if self.policy:
if not self.policies:
self.policies = [self.policy]
self._body.clean(only={'policy'})
if self.create_method == 'POST':
request = self._prepare_request(
requires_id=requires_id,
prepend_key=prepend_key,
base_path=base_path,
)
response = session.post(
request.url,
json=request.body,
headers=request.headers,
microversion=microversion,
params=params,
)
else:
raise exceptions.ResourceFailure(
"Invalid create method: %s" % self.create_method
)
has_body = (
self.has_body
if self.create_returns_body is None
else self.create_returns_body
)
self.microversion = microversion
self._translate_response(response, has_body=has_body)
# direct comparision to False since we need to rule out None
if self.has_body and self.create_returns_body is False:
# fetch the body if it's required but not returned by create
return self.fetch(session)
return self

View File

@ -74,14 +74,10 @@ class ServerMigration(resource.Resource):
_max_microversion = '2.80'
@classmethod
def _get_microversion_for_action(cls, session):
return cls._get_microversion_for_list(session)
def _action(self, session, body):
"""Preform server migration actions given the message body."""
session = self._get_session(session)
microversion = self._get_microversion_for_list(session)
microversion = self._get_microversion(session, action='list')
url = utils.urljoin(
self.base_path % {'server_uuid': self.server_id},

View File

@ -71,7 +71,7 @@ class ZoneExport(_base.Resource):
raise exceptions.MethodNotSupported(self, "create")
session = self._get_session(session)
microversion = self._get_microversion_for(session, 'create')
microversion = self._get_microversion(session, action='create')
# Create ZoneExport requires empty body
# skip _prepare_request completely, since we need just empty body
request = resource._Request(

View File

@ -71,7 +71,7 @@ class ZoneImport(_base.Resource):
raise exceptions.MethodNotSupported(self, "create")
session = self._get_session(session)
microversion = self._get_microversion_for(session, 'create')
microversion = self._get_microversion(session, action='create')
# Create ZoneImport requires empty body and 'text/dns' as content-type
# skip _prepare_request completely, since we need just empty body
request = resource._Request(

View File

@ -66,7 +66,7 @@ class Info(resource.Resource):
url = "{scheme}://{netloc}/info".format(
scheme=endpoint.scheme, netloc=endpoint.netloc)
microversion = self._get_microversion_for(session, 'fetch')
microversion = self._get_microversion(session, action='fetch')
response = session.get(url, microversion=microversion)
kwargs = {}
if error_message:

View File

@ -323,7 +323,7 @@ class Object(_base.BaseResource):
request = self._prepare_request()
session = self._get_session(session)
microversion = self._get_microversion_for(session, 'delete')
microversion = self._get_microversion(session, action='delete')
if self.is_static_large_object is None:
# Fetch metadata to determine SLO flag

View File

@ -138,7 +138,7 @@ class Stack(resource.Resource):
requires_id=False,
base_path=base_path)
microversion = self._get_microversion_for(session, 'commit')
microversion = self._get_microversion(session, action='commit')
request_url = request.url
if preview:
@ -177,7 +177,7 @@ class Stack(resource.Resource):
request = self._prepare_request(requires_id=requires_id,
base_path=base_path)
# session = self._get_session(session)
microversion = self._get_microversion_for(session, 'fetch')
microversion = self._get_microversion(session, action='fetch')
# NOTE(gtema): would be nice to simply use QueryParameters, however
# Heat return 302 with parameters being set into URL and requests

View File

@ -1300,8 +1300,8 @@ class Resource(dict):
)
@classmethod
def _get_microversion_for_list(cls, session):
"""Get microversion to use when listing resources.
def _get_microversion(cls, session, *, action):
"""Get microversion to use for the given action.
The base version uses the following logic:
@ -1313,9 +1313,22 @@ class Resource(dict):
Subclasses can override this method if more complex logic is needed.
:param session: :class`keystoneauth1.adapter.Adapter`
:return: microversion as string or ``None``
:param session: The session to use for making the request.
:type session: :class:`~keystoneauth1.adapter.Adapter`
:param action: One of "fetch", "commit", "create", "delete", "patch".
:type action: str
:return: Microversion as string or ``None``
"""
if action not in {
'list',
'fetch',
'commit',
'create',
'delete',
'patch',
}:
raise ValueError('Invalid action: %s' % action)
if session.default_microversion:
return session.default_microversion
@ -1323,22 +1336,6 @@ class Resource(dict):
session, cls._max_microversion
)
def _get_microversion_for(self, session, action):
"""Get microversion to use for the given action.
The base version uses :meth:`_get_microversion_for_list`.
Subclasses can override this method if more complex logic is needed.
:param session: :class`keystoneauth1.adapter.Adapter`
:param action: One of "fetch", "commit", "create", "delete", "patch".
Unused in the base implementation.
:return: microversion as string or ``None``
"""
if action not in ('fetch', 'commit', 'create', 'delete', 'patch'):
raise ValueError('Invalid action: %s' % action)
return self._get_microversion_for_list(session)
def _assert_microversion_for(
self,
session,
@ -1365,7 +1362,7 @@ class Resource(dict):
raise exceptions.NotSupported(message)
actual = self._get_microversion_for(session, action)
actual = self._get_microversion(session, action=action)
if expected is None:
return actual
@ -1403,10 +1400,10 @@ class Resource(dict):
:data:`Resource.allow_create` is not set to ``True``.
"""
if not self.allow_create:
raise exceptions.MethodNotSupported(self, "create")
raise exceptions.MethodNotSupported(self, 'create')
session = self._get_session(session)
microversion = self._get_microversion_for(session, 'create')
microversion = self._get_microversion(session, action='create')
requires_id = (
self.create_requires_id
if self.create_requires_id is not None
@ -1486,7 +1483,7 @@ class Resource(dict):
:data:`Resource.allow_create` is not set to ``True``.
"""
if not cls.allow_create:
raise exceptions.MethodNotSupported(cls, "create")
raise exceptions.MethodNotSupported(cls, 'create')
if not (
data
@ -1496,7 +1493,7 @@ class Resource(dict):
raise ValueError('Invalid data passed: %s' % data)
session = cls._get_session(session)
microversion = cls._get_microversion_for(cls, session, 'create')
microversion = cls._get_microversion(session, action='create')
requires_id = (
cls.create_requires_id
if cls.create_requires_id is not None
@ -1591,13 +1588,13 @@ class Resource(dict):
the resource was not found.
"""
if not self.allow_fetch:
raise exceptions.MethodNotSupported(self, "fetch")
raise exceptions.MethodNotSupported(self, 'fetch')
request = self._prepare_request(
requires_id=requires_id, base_path=base_path
)
session = self._get_session(session)
microversion = self._get_microversion_for(session, 'fetch')
microversion = self._get_microversion(session, action='fetch')
response = session.get(
request.url,
microversion=microversion,
@ -1627,12 +1624,12 @@ class Resource(dict):
was not found.
"""
if not self.allow_head:
raise exceptions.MethodNotSupported(self, "head")
request = self._prepare_request(base_path=base_path)
raise exceptions.MethodNotSupported(self, 'head')
session = self._get_session(session)
microversion = self._get_microversion_for(session, 'fetch')
microversion = self._get_microversion(session, action='fetch')
request = self._prepare_request(base_path=base_path)
response = session.head(request.url, microversion=microversion)
self.microversion = microversion
@ -1681,7 +1678,7 @@ class Resource(dict):
return self
if not self.allow_commit:
raise exceptions.MethodNotSupported(self, "commit")
raise exceptions.MethodNotSupported(self, 'commit')
# Avoid providing patch unconditionally to avoid breaking subclasses
# without it.
@ -1691,7 +1688,7 @@ class Resource(dict):
request = self._prepare_request(
prepend_key=prepend_key, base_path=base_path, **kwargs
)
microversion = self._get_microversion_for(session, 'commit')
microversion = self._get_microversion(session, action='commit')
return self._commit(
session,
@ -1808,12 +1805,12 @@ class Resource(dict):
return self
if not self.allow_patch:
raise exceptions.MethodNotSupported(self, "patch")
raise exceptions.MethodNotSupported(self, 'patch')
request = self._prepare_request(
prepend_key=prepend_key, base_path=base_path, patch=True
)
microversion = self._get_microversion_for(session, 'patch')
microversion = self._get_microversion(session, action='patch')
if patch:
request.body += self._convert_patch(patch)
@ -1851,11 +1848,11 @@ class Resource(dict):
def _raw_delete(self, session, **kwargs):
if not self.allow_delete:
raise exceptions.MethodNotSupported(self, "delete")
raise exceptions.MethodNotSupported(self, 'delete')
request = self._prepare_request(**kwargs)
session = self._get_session(session)
microversion = self._get_microversion_for(session, 'delete')
microversion = self._get_microversion(session, action='delete')
return session.delete(
request.url, headers=request.headers, microversion=microversion
@ -1903,9 +1900,10 @@ class Resource(dict):
contains invalid params.
"""
if not cls.allow_list:
raise exceptions.MethodNotSupported(cls, "list")
raise exceptions.MethodNotSupported(cls, 'list')
session = cls._get_session(session)
microversion = cls._get_microversion_for_list(session)
microversion = cls._get_microversion(session, action='list')
if base_path is None:
base_path = cls.base_path

View File

@ -3086,7 +3086,7 @@ class TestWaitForDelete(base.TestCase):
self.cloud.compute, res, 0.1, 0.3)
@mock.patch.object(resource.Resource, '_get_microversion_for', autospec=True)
@mock.patch.object(resource.Resource, '_get_microversion', autospec=True)
class TestAssertMicroversionFor(base.TestCase):
session = mock.Mock()
res = resource.Resource()
@ -3097,7 +3097,7 @@ class TestAssertMicroversionFor(base.TestCase):
self.assertEqual(
'1.42',
self.res._assert_microversion_for(self.session, 'fetch', '1.6'))
mock_get_ver.assert_called_once_with(self.res, self.session, 'fetch')
mock_get_ver.assert_called_once_with(self.session, action='fetch')
def test_incompatible(self, mock_get_ver):
mock_get_ver.return_value = '1.1'
@ -3106,7 +3106,7 @@ class TestAssertMicroversionFor(base.TestCase):
'1.6 is required, but 1.1 will be used',
self.res._assert_microversion_for,
self.session, 'fetch', '1.6')
mock_get_ver.assert_called_once_with(self.res, self.session, 'fetch')
mock_get_ver.assert_called_once_with(self.session, action='fetch')
def test_custom_message(self, mock_get_ver):
mock_get_ver.return_value = '1.1'
@ -3116,7 +3116,7 @@ class TestAssertMicroversionFor(base.TestCase):
self.res._assert_microversion_for,
self.session, 'fetch', '1.6',
error_message='boom')
mock_get_ver.assert_called_once_with(self.res, self.session, 'fetch')
mock_get_ver.assert_called_once_with(self.session, action='fetch')
def test_none(self, mock_get_ver):
mock_get_ver.return_value = None
@ -3125,4 +3125,4 @@ class TestAssertMicroversionFor(base.TestCase):
'1.6 is required, but the default version',
self.res._assert_microversion_for,
self.session, 'fetch', '1.6')
mock_get_ver.assert_called_once_with(self.res, self.session, 'fetch')
mock_get_ver.assert_called_once_with(self.session, action='fetch')