Merge "Deprecate unnecessary options, aliases"

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:
@@ -1430,15 +1432,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)
@@ -1452,7 +1458,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()

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=None):
-    def list_volumes(self, cache=True):
         """List all available volumes.
 
         :param cache: **DEPRECATED** This parameter no longer does anything.
         :returns: A list of volume ``Volume`` objects.
         """
-        warnings.warn(
+        if cache is not None:
-            "the 'cache' argument is deprecated and no longer does anything; "
+            warnings.warn(
-            "consider removing it from calls",
+                "the 'cache' argument is deprecated and no longer does "
-            os_warnings.RemovedInSDK50Warning,
+                "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.
 
@@ -243,14 +242,22 @@ class BlockStorageCloudMixin(openstackcloud._OpenStackCloudMixin):
 
         return True
 
-    # TODO(stephenfin): Remove 'cache' in a future major version
+    def get_volumes(self, server, cache=None):
-    def get_volumes(self, server, cache=True):
         """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']:
@@ -715,7 +722,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,
@@ -740,7 +746,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):

openstack/cloud/_identity.py

@@ -298,12 +298,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(

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=None, prefix=None):
-    def list_containers(self, full_listing=True, 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):

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,

openstack/config/loader.py

@@ -984,8 +984,13 @@ class OpenStackConfig:
                     )
         return clouds
 
-    # TODO(mordred) Backwards compat for OSC transition
+    def get_all_clouds(self):
-    get_all_clouds = get_all
+        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
+    def get_one_cloud(
-    get_one_cloud = get_one
+        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

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

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

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."""