Skip to content

Commit

Permalink
Merge "proxy: Remove '_check_resource' decorator"
Browse files Browse the repository at this point in the history
  • Loading branch information
@openstack-gerrit
Zuul authored and openstack-gerrit committed Feb 26, 2025
2 parents 7cc12cd + 6822ab9 commit cdfff8f
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 119 deletions.
9 changes: 6 additions & 3 deletions openstack/block_storage/v2/_proxy.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -758,10 +758,13 @@ def get_limits(self, project=None):
:class:`~openstack.block_storage.v2.limits.RateLimit`
:rtype: :class:`~openstack.block_storage.v2.limits.Limits`
"""
params = {}
if project:
params['project_id'] = resource.Resource._get_id(project)
return self._get(_limits.Limits, requires_id=False, **params)
return self._get(
_limits.Limits,
requires_id=False,
project_id=resource.Resource._get_id(project),
)
return self._get(_limits.Limits, requires_id=False)

# ========== Capabilities ==========

Expand Down
14 changes: 6 additions & 8 deletions openstack/network/v2/_proxy.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,11 +193,10 @@ class Proxy(proxy.Proxy):
"vpn_service": _vpn_service.VpnService,
}

@proxy._check_resource(strict=False)
def _update(
self,
resource_type: type[resource.ResourceT],
value: ty.Union[str, resource.ResourceT],
value: ty.Union[str, resource.ResourceT, None],
base_path: ty.Optional[str] = None,
if_revision: ty.Optional[int] = None,
**attrs: ty.Any,
Expand All @@ -212,11 +211,10 @@ def _update(

return res.commit(self, base_path=base_path)

@proxy._check_resource(strict=False)
def _delete(
self,
resource_type: type[resource.ResourceT],
value: ty.Union[str, resource.ResourceT],
value: ty.Union[str, resource.ResourceT, None],
ignore_missing: bool = True,
if_revision: ty.Optional[int] = None,
**attrs: ty.Any,
Expand Down Expand Up @@ -3594,10 +3592,10 @@ def delete_qos_minimum_packet_rate_rule(
rule belongs or a
:class:`~openstack.network.v2.qos_policy.QoSPolicy` instance.
:param bool ignore_missing: When set to ``False``
:class:`~openstack.exceptions.NotFoundException` will be raised when
the resource does not exist. When set to ``True``, no exception
will be set when attempting to delete a nonexistent minimum packet
rate rule.
:class:`~openstack.exceptions.NotFoundException` will be raised
when the resource does not exist. When set to ``True``, no
exception will be set when attempting to delete a nonexistent
minimum packet rate rule.
:returns: ``None``
"""
Expand Down
108 changes: 42 additions & 66 deletions openstack/proxy.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,37 +36,6 @@
from openstack import connection


# The _check_resource decorator is used on Proxy methods to ensure that
# the `actual` argument is in fact the type of the `expected` argument.
# It does so under two cases:
# 1. When strict=False, if and only if `actual` is a Resource instance,
# it is checked to see that it's an instance of the `expected` class.
# This allows `actual` to be other types, such as strings, when it makes
# sense to accept a raw id value.
# 2. When strict=True, `actual` must be an instance of the `expected` class.
def _check_resource(strict=False):
def wrap(method):
def check(self, expected, actual=None, *args, **kwargs):
if (
strict
and actual is not None
and not isinstance(actual, resource.Resource)
):
raise ValueError(f"A {expected.__name__} must be passed")
elif isinstance(actual, resource.Resource) and not isinstance(
actual, expected
):
raise ValueError(
f"Expected {expected.__name__} but received {actual.__class__.__name__}"
)

return method(self, expected, actual, *args, **kwargs)

return check

return wrap


def normalize_metric_name(name):
name = name.replace('.', '_')
name = name.replace(':', '_')
Expand Down Expand Up @@ -471,6 +440,11 @@ class if using an existing instance, or ``utils.Munch``,
res = resource_type.new(id=value, connection=conn, **attrs)
else:
# An existing resource instance
if not isinstance(value, resource_type):
raise ValueError(
f'Expected {resource_type.__name__} but received '
f'{value.__class__.__name__}'
)
res = value
res._update(**attrs)

Expand Down Expand Up @@ -529,6 +503,8 @@ def _find(
) -> ty.Optional[resource.ResourceT]:
"""Find a resource
:param resource_type: The type of resource to find. This should be a
:class:`~openstack.resource.Resource` subclass.
:param name_or_id: The name or ID of a resource to find.
:param bool ignore_missing: When set to ``False``
:class:`~openstack.exceptions.NotFoundException` will be
Expand All @@ -545,22 +521,22 @@ def _find(
self, name_or_id, ignore_missing=ignore_missing, **attrs
)

@_check_resource(strict=False)
def _delete(
self,
resource_type: type[resource.ResourceT],
value: ty.Union[str, resource.ResourceT],
value: ty.Union[str, resource.ResourceT, None],
ignore_missing: bool = True,
**attrs: ty.Any,
) -> ty.Optional[resource.ResourceT]:
"""Delete a resource
:param resource_type: The type of resource to delete. This should
be a :class:`~openstack.resource.Resource`
subclass with a ``from_id`` method.
:param value: The value to delete. Can be either the ID of a
resource or a :class:`~openstack.resource.Resource`
subclass.
:param resource_type: The type of resource to delete. This should be a
:class:`~openstack.resource.Resource` subclass.
:param value: The resource to delete. This can be the ID of a resource,
a :class:`~openstack.resource.Resource` subclass instance, or None
for resources that don't have their own identifier or have
identifiers with multiple parts. If None, you must pass these other
identifiers as kwargs.
:param bool ignore_missing: When set to ``False``
:class:`~openstack.exceptions.NotFoundException` will be
raised when the resource does not exist.
Expand All @@ -587,21 +563,22 @@ def _delete(

return rv

@_check_resource(strict=False)
def _update(
self,
resource_type: type[resource.ResourceT],
value: ty.Union[str, resource.ResourceT],
value: ty.Union[str, resource.ResourceT, None],
base_path: ty.Optional[str] = None,
**attrs: ty.Any,
) -> resource.ResourceT:
"""Update a resource
:param resource_type: The type of resource to update.
:type resource_type: :class:`~openstack.resource.Resource`
:param value: The resource to update. This must either be a
:class:`~openstack.resource.Resource` or an id
that corresponds to a resource.
:param resource_type: The type of resource to update. This should be a
:class:`~openstack.resource.Resource` subclass.
:param value: The resource to update. This can be the ID of a resource,
a :class:`~openstack.resource.Resource` subclass instance, or None
for resources that don't have their own identifier or have
identifiers with multiple parts. If None, you must pass these other
identifiers as kwargs.
:param str base_path: Base part of the URI for updating resources, if
different from
:data:`~openstack.resource.Resource.base_path`.
Expand All @@ -626,13 +603,10 @@ def _create(
) -> resource.ResourceT:
"""Create a resource from attributes
:param resource_type: The type of resource to create.
:type resource_type: :class:`~openstack.resource.Resource`
:param str base_path: Base part of the URI for creating resources, if
different from
:data:`~openstack.resource.Resource.base_path`.
:param path_args: A dict containing arguments for forming the request
URL, if needed.
:param resource_type: The type of resource to create. This should be a
:class:`~openstack.resource.Resource` subclass.
:param base_path: Base part of the URI for creating resources, if
different from :data:`~openstack.resource.Resource.base_path`.
:param dict attrs: Attributes to be passed onto the
:meth:`~openstack.resource.Resource.create`
method to be created. These should correspond
Expand Down Expand Up @@ -662,9 +636,9 @@ def _bulk_create(
) -> ty.Generator[resource.ResourceT, None, None]:
"""Create a resource from attributes
:param resource_type: The type of resource to create.
:type resource_type: :class:`~openstack.resource.Resource`
:param list data: List of attributes dicts to be passed onto the
:param resource_type: The type of resource to create. This should be a
:class:`~openstack.resource.Resource` subclass.
:param data: List of attributes dicts to be passed onto the
:meth:`~openstack.resource.Resource.create`
method to be created. These should correspond
to either :class:`~openstack.resource.Body`
Expand All @@ -679,7 +653,6 @@ def _bulk_create(
"""
return resource_type.bulk_create(self, data, base_path=base_path)

@_check_resource(strict=False)
def _get(
self,
resource_type: type[resource.ResourceT],
Expand All @@ -691,17 +664,20 @@ def _get(
) -> resource.ResourceT:
"""Fetch a resource
:param resource_type: The type of resource to get.
:type resource_type: :class:`~openstack.resource.Resource`
:param value: The value to get. Can be either the ID of a
resource or a :class:`~openstack.resource.Resource`
subclass.
:param str base_path: Base part of the URI for fetching resources, if
:param resource_type: The type of resource to get. This should be a
:class:`~openstack.resource.Resource` subclass.
:param value: The resource to get. This can be the ID of a resource,
a :class:`~openstack.resource.Resource` subclass instance, or None
for resources that don't have their own identifier or have
identifiers with multiple parts. If None, you must pass these other
identifiers as kwargs.
:param requires_id: Whether the resource is identified by an ID or not.
:param base_path: Base part of the URI for fetching resources, if
different from
:data:`~openstack.resource.Resource.base_path`.
:param bool skip_cache: A boolean indicating whether optional API
:param skip_cache: A boolean indicating whether optional API
cache should be skipped for this invocation.
:param dict attrs: Attributes to be passed onto the
:param attrs: Attributes to be passed onto the
:meth:`~openstack.resource.Resource.get`
method. These should correspond
to either :class:`~openstack.resource.Body`
Expand Down Expand Up @@ -787,7 +763,7 @@ def _head(
:param resource_type: The type of resource to retrieve.
:type resource_type: :class:`~openstack.resource.Resource`
:param value: The value of a specific resource to retreive headers
:param value: The value of a specific resource to retrieve headers
for. Can be either the ID of a resource,
a :class:`~openstack.resource.Resource` subclass,
or ``None``.
Expand Down
42 changes: 0 additions & 42 deletions openstack/tests/unit/test_proxy.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,48 +73,6 @@ def method(self, expected_type, value):
self.fake_proxy = proxy.Proxy(self.session)
self.fake_proxy._connection = self.cloud

def _test_correct(self, value):
decorated = proxy._check_resource(strict=False)(self.sot.method)
rv = decorated(self.sot, resource.Resource, value)

self.assertEqual(value, rv)

def test__check_resource_correct_resource(self):
res = resource.Resource()
self._test_correct(res)

def test__check_resource_notstrict_id(self):
self._test_correct("abc123-id")

def test__check_resource_strict_id(self):
decorated = proxy._check_resource(strict=True)(self.sot.method)
self.assertRaisesRegex(
ValueError,
"A Resource must be passed",
decorated,
self.sot,
resource.Resource,
"this-is-not-a-resource",
)

def test__check_resource_incorrect_resource(self):
class OneType(resource.Resource):
pass

class AnotherType(resource.Resource):
pass

value = AnotherType()
decorated = proxy._check_resource(strict=False)(self.sot.method)
self.assertRaisesRegex(
ValueError,
"Expected OneType but received AnotherType",
decorated,
self.sot,
OneType,
value,
)

def test__get_uri_attribute_no_parent(self):
class Child(resource.Resource):
something = resource.Body("something")
Expand Down

0 comments on commit cdfff8f

Please sign in to comment.