From 7069879b584d092dcf93fe4861061b7fcbd4f9d8 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 16 Jul 2024 12:57:24 +0100 Subject: [PATCH] Deprecate unnecessary options, aliases Change-Id: Ia2203bea15c8593611f01eca6ab511c0e11ae8b6 Signed-off-by: Stephen Finucane --- openstack/baremetal/v1/node.py | 14 ++++++++---- openstack/cloud/_block_storage.py | 36 ++++++++++++++++++++----------- openstack/cloud/_identity.py | 9 +++++++- openstack/cloud/_object_store.py | 12 +++++++++-- openstack/compute/v2/_proxy.py | 2 -- openstack/config/loader.py | 25 +++++++++++++++++---- openstack/proxy.py | 9 +++++++- openstack/tests/fixtures.py | 2 +- openstack/warnings.py | 11 ++++++++-- 9 files changed, 91 insertions(+), 29 deletions(-) diff --git a/openstack/baremetal/v1/node.py b/openstack/baremetal/v1/node.py index 8605e9e91..6688a1030 100644 --- a/openstack/baremetal/v1/node.py +++ b/openstack/baremetal/v1/node.py @@ -13,11 +13,13 @@ import collections import enum import typing as ty +import warnings from openstack.baremetal.v1 import _common from openstack import exceptions from openstack import resource from openstack import utils +from openstack import warnings as os_warnings class ValidationResult: @@ -1399,15 +1401,19 @@ class Node(_common.Resource): ) exceptions.raise_from_response(response, error_message=msg) - # TODO(stephenfin): Drop 'node_id' and use 'self.id' instead or convert to - # a classmethod def get_node_inventory(self, session, node_id): """Get a node's inventory. :param session: The session to use for making this request. - :param node_id: The ID of the node. + :param node_id: **DEPRECATED** The ID of the node. :returns: The HTTP response. """ + if node_id is not None: + warnings.warn( + "The 'node_id' field is unnecessary and will be removed in " + "a future release.", + os_warnings.RemovedInSDK60Warning, + ) session = self._get_session(session) version = self._get_microversion(session, action='fetch') request = self._prepare_request(requires_id=True) @@ -1421,7 +1427,7 @@ class Node(_common.Resource): ) msg = "Failed to get inventory for node {node}".format( - node=node_id, + node=self.id, ) exceptions.raise_from_response(response, error_message=msg) return response.json() diff --git a/openstack/cloud/_block_storage.py b/openstack/cloud/_block_storage.py index 7cebb80e2..b2a4b0027 100644 --- a/openstack/cloud/_block_storage.py +++ b/openstack/cloud/_block_storage.py @@ -20,21 +20,20 @@ from openstack import warnings as os_warnings class BlockStorageCloudMixin(openstackcloud._OpenStackCloudMixin): - # TODO(stephenfin): Remove 'cache' in a future major version - def list_volumes(self, cache=True): + def list_volumes(self, cache=None): """List all available volumes. :param cache: **DEPRECATED** This parameter no longer does anything. :returns: A list of volume ``Volume`` objects. """ - warnings.warn( - "the 'cache' argument is deprecated and no longer does anything; " - "consider removing it from calls", - os_warnings.RemovedInSDK50Warning, - ) + if cache is not None: + warnings.warn( + "the 'cache' argument is deprecated and no longer does " + "anything; consider removing it from calls", + os_warnings.RemovedInSDK50Warning, + ) return list(self.block_storage.volumes()) - # TODO(stephenfin): Remove 'get_extra' in a future major version def list_volume_types(self, get_extra=None): """List all available volume types. @@ -248,14 +247,22 @@ class BlockStorageCloudMixin(openstackcloud._OpenStackCloudMixin): return True - # TODO(stephenfin): Remove 'cache' in a future major version - def get_volumes(self, server, cache=True): + def get_volumes(self, server, cache=None): """Get volumes for a server. :param server: The server to fetch volumes for. :param cache: **DEPRECATED** This parameter no longer does anything. :returns: A list of volume ``Volume`` objects. """ + if cache is not None: + warnings.warn( + "the 'cache' argument is deprecated and no longer does " + "anything; consider removing it from calls", + os_warnings.RemovedInSDK50Warning, + ) + # avoid spamming warnings + cache = None + volumes = [] for volume in self.list_volumes(cache=cache): for attach in volume['attachments']: @@ -722,7 +729,6 @@ class BlockStorageCloudMixin(openstackcloud._OpenStackCloudMixin): volume_backups = self.list_volume_backups() return _utils._filter_list(volume_backups, name_or_id, filters) - # TODO(stephenfin): Remove 'get_extra' in a future major version def search_volume_types( self, name_or_id=None, @@ -747,7 +753,13 @@ class BlockStorageCloudMixin(openstackcloud._OpenStackCloudMixin): :returns: A list of volume ``Type`` objects, if any are found. """ - volume_types = self.list_volume_types(get_extra=get_extra) + if get_extra is not None: + warnings.warn( + "the 'get_extra' argument is deprecated and no longer does " + "anything; consider removing it from calls", + os_warnings.RemovedInSDK50Warning, + ) + volume_types = self.list_volume_types() return _utils._filter_list(volume_types, name_or_id, filters) def get_volume_type_access(self, name_or_id): diff --git a/openstack/cloud/_identity.py b/openstack/cloud/_identity.py index 986bbe794..2e8e5795b 100644 --- a/openstack/cloud/_identity.py +++ b/openstack/cloud/_identity.py @@ -300,12 +300,19 @@ class IdentityCloudMixin(openstackcloud._OpenStackCloudMixin): return _utils._get_entity(self, 'user', name_or_id, filters, **kwargs) # TODO(stephenfin): Remove normalize since it doesn't do anything - def get_user_by_id(self, user_id, normalize=True): + def get_user_by_id(self, user_id, normalize=None): """Get a user by ID. :param string user_id: user ID :returns: an identity ``User`` object """ + if normalize is not None: + warnings.warn( + "The 'normalize' field is unnecessary and will be removed in " + "a future release.", + os_warnings.RemovedInSDK60Warning, + ) + return self.identity.get_user(user_id) @_utils.valid_kwargs( diff --git a/openstack/cloud/_object_store.py b/openstack/cloud/_object_store.py index 3d95d799e..594a06856 100644 --- a/openstack/cloud/_object_store.py +++ b/openstack/cloud/_object_store.py @@ -12,12 +12,15 @@ import concurrent.futures import urllib.parse +import warnings import keystoneauth1.exceptions from openstack.cloud import _utils from openstack.cloud import openstackcloud from openstack import exceptions +from openstack import warnings as os_warnings + OBJECT_CONTAINER_ACLS = { 'public': '.r:*,.rlistings', @@ -26,8 +29,7 @@ OBJECT_CONTAINER_ACLS = { class ObjectStoreCloudMixin(openstackcloud._OpenStackCloudMixin): - # TODO(stephenfin): Remove 'full_listing' as it's a noop - def list_containers(self, full_listing=True, prefix=None): + def list_containers(self, full_listing=None, prefix=None): """List containers. :param full_listing: Ignored. Present for backwards compat @@ -37,6 +39,12 @@ class ObjectStoreCloudMixin(openstackcloud._OpenStackCloudMixin): :raises: :class:`~openstack.exceptions.SDKException` on operation error. """ + if full_listing is not None: + warnings.warn( + "The 'full_listing' field is unnecessary and will be removed " + "in a future release.", + os_warnings.RemovedInSDK60Warning, + ) return list(self.object_store.containers(prefix=prefix)) def search_containers(self, name=None, filters=None): diff --git a/openstack/compute/v2/_proxy.py b/openstack/compute/v2/_proxy.py index 9ca015f10..d32becf40 100644 --- a/openstack/compute/v2/_proxy.py +++ b/openstack/compute/v2/_proxy.py @@ -1366,8 +1366,6 @@ class Proxy(proxy.Proxy): **attrs, ) - # TODO(stephenfin): Does this work? There's no 'value' parameter for the - # call to '_delete' def delete_server_interface( self, server_interface, diff --git a/openstack/config/loader.py b/openstack/config/loader.py index bee1a5f16..01f35f3df 100644 --- a/openstack/config/loader.py +++ b/openstack/config/loader.py @@ -984,8 +984,13 @@ class OpenStackConfig: ) return clouds - # TODO(mordred) Backwards compat for OSC transition - get_all_clouds = get_all + def get_all_clouds(self): + warnings.warn( + "The 'get_all_clouds' method is a deprecated alias for " + "'get_clouds' and will be removed in a future release.", + os_warnings.RemovedInSDK60Warning, + ) + return self.get_all() def _fix_args(self, args=None, argparse=None): """Massage the passed-in options @@ -1343,8 +1348,20 @@ class OpenStackConfig: influxdb_config=influxdb_config, ) - # TODO(mordred) Backwards compat for OSC transition - get_one_cloud = get_one + def get_one_cloud( + self, cloud=None, validate=True, argparse=None, **kwargs + ): + warnings.warn( + "The 'get_one_cloud' method is a deprecated alias for 'get_one' " + "and will be removed in a future release.", + os_warnings.RemovedInSDK60Warning, + ) + return self.get_one( + cloud=cloud, + validate=validate, + argparse=argparse, + **kwargs, + ) def get_one_cloud_osc( self, cloud=None, validate=True, argparse=None, **kwargs diff --git a/openstack/proxy.py b/openstack/proxy.py index 41bb93fa9..36f72db47 100644 --- a/openstack/proxy.py +++ b/openstack/proxy.py @@ -14,6 +14,7 @@ import functools import typing as ty import urllib from urllib.parse import urlparse +import warnings try: import simplejson @@ -28,6 +29,7 @@ from keystoneauth1 import adapter from openstack import _log from openstack import exceptions from openstack import resource +from openstack import warnings as os_warnings ResourceType = ty.TypeVar('ResourceType', bound=resource.Resource) @@ -208,7 +210,6 @@ class Proxy(adapter.Adapter): self._report_stats(None, url, method, e) raise - # TODO(stephenfin): service_type is unused and should be dropped @functools.lru_cache(maxsize=256) def _extract_name(self, url, service_type=None, project_id=None): """Produce a key name to use in logging/metrics from the URL path. @@ -225,6 +226,12 @@ class Proxy(adapter.Adapter): /servers/{id}/os-security-groups -> ['server', 'os-security-groups'] /v2.0/networks.json -> ['networks'] """ + if service_type is not None: + warnings.warn( + "The 'service_type' parameter is unnecesary and will be " + "removed in a future release.", + os_warnings.RemovedInSDK60Warning, + ) url_path = urllib.parse.urlparse(url).path.strip() # Remove / from the beginning to keep the list indexes of interesting diff --git a/openstack/tests/fixtures.py b/openstack/tests/fixtures.py index 3ee41849c..b5d76bace 100644 --- a/openstack/tests/fixtures.py +++ b/openstack/tests/fixtures.py @@ -47,7 +47,7 @@ class WarningsFixture(fixtures.Fixture): ) warnings.filterwarnings( "ignore", - category=os_warnings.RemovedInSDK50Warning, + category=os_warnings._RemovedInSDKWarning, ) # also ignore our own general warnings diff --git a/openstack/warnings.py b/openstack/warnings.py index e12d85d13..885bd4e52 100644 --- a/openstack/warnings.py +++ b/openstack/warnings.py @@ -42,11 +42,18 @@ class LegacyAPIWarning(OpenStackDeprecationWarning): # function parameters. -class RemovedInSDK50Warning(PendingDeprecationWarning): +class _RemovedInSDKWarning(PendingDeprecationWarning): + """Indicates an argument that is deprecated for removal. + + This is a base class and should not be used directly. + """ + + +class RemovedInSDK50Warning(_RemovedInSDKWarning): """Indicates an argument that is deprecated for removal in SDK 5.0.""" -class RemovedInSDK60Warning(PendingDeprecationWarning): +class RemovedInSDK60Warning(_RemovedInSDKWarning): """Indicates an argument that is deprecated for removal in SDK 6.0."""