openstack/openstacksdk
Code Issues Proposed changes

resource: Merge unnecessary separation of logic

Browse Source 
We had '_get_microversion_for_list' and '_get_microversion_for', the
latter of which simply called the former. There's no reason for this
distinction and I suspect it's simply there because of legacy reasons.
Merge them.

Change-Id: I5124fa0005e809891a1fc970c6ccdc5f4a766d33
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
This commit is contained in:
Stephen Finucane 2022-06-09 15:54:02 +02:00 committed by Artem Goncharov
parent 6469429498
commit 7dd6aa22f5
18 changed files with 151 additions and 109 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
openstack/accelerator/v2/accelerator_request.py
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)

10
openstack/baremetal/v1/node.py
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')

4
openstack/baremetal_introspection/v1/introspection.py
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')

2
openstack/block_storage/v2/backup.py
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')

2
openstack/block_storage/v3/backup.py
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')

4
openstack/block_storage/v3/group.py
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': {

10
openstack/block_storage/v3/group_type.py
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)

10
openstack/compute/v2/flavor.py
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)

2
openstack/compute/v2/hypervisor.py
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)

110
openstack/compute/v2/server_group.py
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

6
openstack/compute/v2/server_migration.py
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},

2
openstack/dns/v2/zone_export.py
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(

2
openstack/dns/v2/zone_import.py
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(

2
openstack/object_store/v1/info.py
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:

2
openstack/object_store/v1/obj.py
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

4
openstack/orchestration/v1/stack.py
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

76
openstack/resource.py
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

10
openstack/tests/unit/test_resource.py
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')