Merge "Drop support for --sort_key and --sort_dir"5.0.0

10 files changed, 22 insertions, 162 deletions

diff --git a/cinderclient/base.py b/cinderclient/base.py
index e84eb2f..a317ad4 100644
--- a/cinderclient/base.py
+++ b/cinderclient/base.py
@@ -131,8 +131,7 @@ class Manager(common_base.HookableMixin):
             return common_base.ListWithMeta(items, resp)
 
     def _build_list_url(self, resource_type, detailed=True, search_opts=None,
-                        marker=None, limit=None, sort_key=None, sort_dir=None,
-                        sort=None, offset=None):
+                        marker=None, limit=None, sort=None, offset=None):
 
         if search_opts is None:
             search_opts = {}
@@ -151,16 +150,6 @@ class Manager(common_base.HookableMixin):
         if sort:
             query_params['sort'] = self._format_sort_param(sort,
                                                            resource_type)
-        else:
-            # sort_key and sort_dir deprecated in kilo, prefer sort
-            if sort_key:
-                query_params['sort_key'] = self._format_sort_key_param(
-                    sort_key,
-                    resource_type)
-
-            if sort_dir:
-                query_params['sort_dir'] = self._format_sort_dir_param(
-                    sort_dir)
 
         if offset:
             query_params['offset'] = offset
@@ -179,7 +168,7 @@ class Manager(common_base.HookableMixin):
                  "query_string": query_string})
 
     def _format_sort_param(self, sort, resource_type=None):
-        '''Formats the sort information into the sort query string parameter.
+        """Formats the sort information into the sort query string parameter.
 
         The input sort information can be any of the following:
         - Comma-separated string in the form of <key[:dir]>
@@ -195,7 +184,7 @@ class Manager(common_base.HookableMixin):
         :returns: Formatted query string parameter or None
         :raise ValueError: If an invalid sort direction or invalid sort key is
                            given
-        '''
+        """
         if not sort:
             return None
 
@@ -205,11 +194,7 @@ class Manager(common_base.HookableMixin):
 
         sort_array = []
         for sort_item in sort:
-            if isinstance(sort_item, tuple):
-                sort_key = sort_item[0]
-                sort_dir = sort_item[1]
-            else:
-                sort_key, _sep, sort_dir = sort_item.partition(':')
+            sort_key, _sep, sort_dir = sort_item.partition(':')
             sort_key = sort_key.strip()
             sort_key = self._format_sort_key_param(sort_key, resource_type)
             if sort_dir:
@@ -237,14 +222,6 @@ class Manager(common_base.HookableMixin):
                ', '.join(valid_sort_keys))
         raise ValueError(msg)
 
-    def _format_sort_dir_param(self, sort_dir):
-        if sort_dir in SORT_DIR_VALUES:
-            return sort_dir
-
-        msg = ('sort_dir must be one of the following: %s.'
-               % ', '.join(SORT_DIR_VALUES))
-        raise ValueError(msg)
-
     @contextlib.contextmanager
     def completion_cache(self, cache_type, obj_class, mode):
         """
diff --git a/cinderclient/tests/unit/v2/test_shell.py b/cinderclient/tests/unit/v2/test_shell.py
index 22cb25e..a610753 100644
--- a/cinderclient/tests/unit/v2/test_shell.py
+++ b/cinderclient/tests/unit/v2/test_shell.py
@@ -219,37 +219,11 @@ class ShellTest(utils.TestCase):
         mock_print.assert_called_once_with(mock.ANY, key_list,
             exclude_unavailable=True, sortby_index=0)
 
-    def test_list_sort_valid(self):
-        self.run_command('list --sort_key=id --sort_dir=asc')
-        self.assert_called('GET', '/volumes/detail?sort_dir=asc&sort_key=id')
-
-    def test_list_sort_key_name(self):
-        # Client 'name' key is mapped to 'display_name'
-        self.run_command('list --sort_key=name')
-        self.assert_called('GET', '/volumes/detail?sort_key=display_name')
-
     def test_list_sort_name(self):
         # Client 'name' key is mapped to 'display_name'
         self.run_command('list --sort=name')
         self.assert_called('GET', '/volumes/detail?sort=display_name')
 
-    def test_list_sort_key_invalid(self):
-        self.assertRaises(ValueError,
-                          self.run_command,
-                          'list --sort_key=foo --sort_dir=asc')
-
-    def test_list_sort_dir_invalid(self):
-        self.assertRaises(ValueError,
-                          self.run_command,
-                          'list --sort_key=id --sort_dir=foo')
-
-    def test_list_mix_sort_args(self):
-        cmds = ['list --sort name:desc --sort_key=status',
-                'list --sort name:desc --sort_dir=asc',
-                'list --sort name:desc --sort_key=status --sort_dir=asc']
-        for cmd in cmds:
-            self.assertRaises(exceptions.CommandError, self.run_command, cmd)
-
     def test_list_sort_single_key_only(self):
         self.run_command('list --sort=id')
         self.assert_called('GET', '/volumes/detail?sort=id')
@@ -277,10 +251,7 @@ class ShellTest(utils.TestCase):
 
     def test_list_reorder_with_sort(self):
         # sortby_index is None if there is sort information
-        for cmd in ['list --sort_key=name',
-                    'list --sort_dir=asc',
-                    'list --sort_key=name --sort_dir=asc',
-                    'list --sort=name',
+        for cmd in ['list --sort=name',
                     'list --sort=name:asc']:
             with mock.patch('cinderclient.utils.print_list') as mock_print:
                 self.run_command(cmd)
diff --git a/cinderclient/tests/unit/v2/test_volumes.py b/cinderclient/tests/unit/v2/test_volumes.py
index 8704c66..e4f9e32 100644
--- a/cinderclient/tests/unit/v2/test_volumes.py
+++ b/cinderclient/tests/unit/v2/test_volumes.py
@@ -29,19 +29,6 @@ class VolumesTest(utils.TestCase):
         cs.assert_called('GET', '/volumes/detail?limit=2&marker=1234')
         self._assert_request_id(lst)
 
-    def test_list_volumes_with_sort_key_dir(self):
-        lst = cs.volumes.list(sort_key='id', sort_dir='asc')
-        cs.assert_called('GET', '/volumes/detail?sort_dir=asc&sort_key=id')
-        self._assert_request_id(lst)
-
-    def test_list_volumes_with_invalid_sort_key(self):
-        self.assertRaises(ValueError,
-                          cs.volumes.list, sort_key='invalid', sort_dir='asc')
-
-    def test_list_volumes_with_invalid_sort_dir(self):
-        self.assertRaises(ValueError,
-                          cs.volumes.list, sort_key='id', sort_dir='invalid')
-
     def test__list(self):
         # There only 2 volumes available for our tests, so we set limit to 2.
         limit = 2
@@ -345,21 +332,10 @@ class FormatSortParamTestCase(utils.TestCase):
         self.assertEqual('id:asc,status,size:desc',
                          cs.volumes._format_sort_param(s))
 
-    def test_format_sort_list_of_tuples(self):
-        s = [('id', 'asc'), 'status', ('size', 'desc')]
-        self.assertEqual('id:asc,status,size:desc',
-                         cs.volumes._format_sort_param(s))
-
-    def test_format_sort_list_of_strings_and_tuples(self):
-        s = [('id', 'asc'), 'status', 'size:desc']
-        self.assertEqual('id:asc,status,size:desc',
-                         cs.volumes._format_sort_param(s))
-
     def test_format_sort_invalid_direction(self):
         for s in ['id:foo',
                   'id:asc,status,size:foo',
-                  ['id', 'status', 'size:foo'],
-                  ['id', 'status', ('size', 'foo')]]:
+                  ['id', 'status', 'size:foo']]:
             self.assertRaises(ValueError,
                               cs.volumes._format_sort_param,
                               s)
diff --git a/cinderclient/tests/unit/v3/test_shell.py b/cinderclient/tests/unit/v3/test_shell.py
index 8969cab..ee35ddf 100644
--- a/cinderclient/tests/unit/v3/test_shell.py
+++ b/cinderclient/tests/unit/v3/test_shell.py
@@ -1438,21 +1438,6 @@ class ShellTest(utils.TestCase):
                                  }}
         self.assert_called('POST', '/volume-transfers', body=expected)
 
-    def test_list_transfer_sort_key(self):
-        self.run_command(
-            '--os-volume-api-version 3.59 transfer-list --sort=id')
-        url = ('/volume-transfers/detail?%s' %
-               parse.urlencode([('sort_key', 'id')]))
-        self.assert_called('GET', url)
-
-    def test_list_transfer_sort_key_dir(self):
-        self.run_command(
-            '--os-volume-api-version 3.59 transfer-list --sort=id:asc')
-        url = ('/volume-transfers/detail?%s' %
-               parse.urlencode([('sort_dir', 'asc'),
-                                ('sort_key', 'id')]))
-        self.assert_called('GET', url)
-
     def test_list_transfer_sorty_not_sorty(self):
         self.run_command(
             '--os-volume-api-version 3.59 transfer-list')
diff --git a/cinderclient/v2/shell.py b/cinderclient/v2/shell.py
index 20c7bd6..b83175e 100644
--- a/cinderclient/v2/shell.py
+++ b/cinderclient/v2/shell.py
@@ -100,14 +100,6 @@ def _translate_attachments(info):
                 'Use the show command to see which fields are available. '
                 'Unavailable/non-existent fields will be ignored. '
                 'Default=None.')
-@utils.arg('--sort_key',
-           metavar='<sort_key>',
-           default=None,
-           help=argparse.SUPPRESS)
-@utils.arg('--sort_dir',
-           metavar='<sort_dir>',
-           default=None,
-           help=argparse.SUPPRESS)
 @utils.arg('--sort',
            metavar='<key>[:<direction>]',
            default=None,
@@ -147,16 +139,8 @@ def do_list(cs, args):
         for field_title in args.fields.split(','):
             field_titles.append(field_title)
 
-    # --sort_key and --sort_dir deprecated in kilo and is not supported
-    # with --sort
-    if args.sort and (args.sort_key or args.sort_dir):
-        raise exceptions.CommandError(
-            'The --sort_key and --sort_dir arguments are deprecated and are '
-            'not supported with --sort.')
-
     volumes = cs.volumes.list(search_opts=search_opts, marker=args.marker,
-                              limit=args.limit, sort_key=args.sort_key,
-                              sort_dir=args.sort_dir, sort=args.sort)
+                              limit=args.limit, sort=args.sort)
     shell_utils.translate_volume_keys(volumes)
 
     # Create a list of servers to which the volume is attached
@@ -178,7 +162,7 @@ def do_list(cs, args):
         if search_opts['all_tenants']:
             key_list.insert(1, 'Tenant ID')
 
-    if args.sort_key or args.sort_dir or args.sort:
+    if args.sort:
         sortby_index = None
     else:
         sortby_index = 0
diff --git a/cinderclient/v2/volumes.py b/cinderclient/v2/volumes.py
index 3cc0d4a..4c380fb 100644
--- a/cinderclient/v2/volumes.py
+++ b/cinderclient/v2/volumes.py
@@ -281,7 +281,7 @@ class VolumeManager(base.ManagerWithFind):
         return self._get("/volumes/%s" % volume_id, "volume")
 
     def list(self, detailed=True, search_opts=None, marker=None, limit=None,
-             sort_key=None, sort_dir=None, sort=None):
+             sort=None):
         """Lists all volumes.
 
         :param detailed: Whether to return detailed volume info.
@@ -289,9 +289,6 @@ class VolumeManager(base.ManagerWithFind):
         :param marker: Begin returning volumes that appear later in the volume
                        list than that represented by this volume id.
         :param limit: Maximum number of volumes to return.
-        :param sort_key: Key to be sorted; deprecated in kilo
-        :param sort_dir: Sort direction, should be 'desc' or 'asc'; deprecated
-                         in kilo
         :param sort: Sort information
         :rtype: list of :class:`Volume`
         """
@@ -299,8 +296,7 @@ class VolumeManager(base.ManagerWithFind):
         resource_type = "volumes"
         url = self._build_list_url(resource_type, detailed=detailed,
                                    search_opts=search_opts, marker=marker,
-                                   limit=limit, sort_key=sort_key,
-                                   sort_dir=sort_dir, sort=sort)
+                                   limit=limit, sort=sort)
         return self._list(url, resource_type, limit=limit)
 
     def delete(self, volume, cascade=False):
diff --git a/cinderclient/v3/attachments.py b/cinderclient/v3/attachments.py
index a0732a3..e1e9290 100644
--- a/cinderclient/v3/attachments.py
+++ b/cinderclient/v3/attachments.py
@@ -45,7 +45,7 @@ class VolumeAttachmentManager(base.ManagerWithFind):
 
     @api_versions.wraps('3.27')
     def list(self, detailed=False, search_opts=None, marker=None, limit=None,
-             sort_key=None, sort_dir=None, sort=None):
+             sort=None):
         """List all attachments."""
         resource_type = "attachments"
         url = self._build_list_url(resource_type,
@@ -53,8 +53,7 @@ class VolumeAttachmentManager(base.ManagerWithFind):
                                    search_opts=search_opts,
                                    marker=marker,
                                    limit=limit,
-                                   sort_key=sort_key,
-                                   sort_dir=sort_dir, sort=sort)
+                                   sort=sort)
         return self._list(url, resource_type, limit=limit)
 
     @api_versions.wraps('3.27')
diff --git a/cinderclient/v3/shell.py b/cinderclient/v3/shell.py
index b076804..fa1cc9c 100644
--- a/cinderclient/v3/shell.py
+++ b/cinderclient/v3/shell.py
@@ -337,14 +337,6 @@ RESET_STATE_RESOURCES = {'volume': utils.find_volume,
                 'Use the show command to see which fields are available. '
                 'Unavailable/non-existent fields will be ignored. '
                 'Default=None.')
-@utils.arg('--sort_key',
-           metavar='<sort_key>',
-           default=None,
-           help=argparse.SUPPRESS)
-@utils.arg('--sort_dir',
-           metavar='<sort_dir>',
-           default=None,
-           help=argparse.SUPPRESS)
 @utils.arg('--sort',
            metavar='<key>[:<direction>]',
            default=None,
@@ -412,24 +404,15 @@ def do_list(cs, args):
         for field_title in args.fields.split(','):
             field_titles.append(field_title)
 
-    # --sort_key and --sort_dir deprecated in kilo and is not supported
-    # with --sort
-    if args.sort and (args.sort_key or args.sort_dir):
-        raise exceptions.CommandError(
-            'The --sort_key and --sort_dir arguments are deprecated and are '
-            'not supported with --sort.')
-
     total_count = 0
     if show_count:
         search_opts['with_count'] = args.with_count
         volumes, total_count = cs.volumes.list(
             search_opts=search_opts, marker=args.marker,
-            limit=args.limit, sort_key=args.sort_key,
-            sort_dir=args.sort_dir, sort=args.sort)
+            limit=args.limit, sort=args.sort)
     else:
         volumes = cs.volumes.list(search_opts=search_opts, marker=args.marker,
-                                  limit=args.limit, sort_key=args.sort_key,
-                                  sort_dir=args.sort_dir, sort=args.sort)
+                                  limit=args.limit, sort=args.sort)
     shell_utils.translate_volume_keys(volumes)
 
     # Create a list of servers to which the volume is attached
@@ -465,7 +448,7 @@ def do_list(cs, args):
         if search_opts['all_tenants']:
             key_list.insert(1, 'Tenant ID')
 
-    if args.sort_key or args.sort_dir or args.sort:
+    if args.sort:
         sortby_index = None
     else:
         sortby_index = 0
@@ -2591,25 +2574,13 @@ def do_transfer_list(cs, args):
     }
 
     sort = getattr(args, 'sort', None)
-    sort_key = None
-    sort_dir = None
     if sort:
-        # We added this feature with sort_key and sort_dir, but that was a
-        # mistake as we've deprecated that construct a long time ago and should
-        # be removing it in favor of --sort. Too late for the service side, but
-        # to make the client experience consistent, we handle the compatibility
-        # here.
         sort_args = sort.split(':')
         if len(sort_args) > 2:
             raise exceptions.CommandError(
                 'Invalid sort parameter provided. Argument must be in the '
                 'form "key[:<asc|desc>]".')
 
-        sort_key = sort_args[0]
-        if len(sort_args) == 2:
-            sort_dir = sort_args[1]
-
-    transfers = cs.transfers.list(
-        search_opts=search_opts, sort_key=sort_key, sort_dir=sort_dir)
+    transfers = cs.transfers.list(search_opts=search_opts, sort=sort)
     columns = ['ID', 'Volume ID', 'Name']
     utils.print_list(transfers, columns)
diff --git a/cinderclient/v3/volume_transfers.py b/cinderclient/v3/volume_transfers.py
index fe790f2..f40c519 100644
--- a/cinderclient/v3/volume_transfers.py
+++ b/cinderclient/v3/volume_transfers.py
@@ -62,14 +62,12 @@ class VolumeTransferManager(volume_transfers.VolumeTransferManager):
 
         return self._get("/os-volume-transfer/%s" % transfer_id, "transfer")
 
-    def list(self, detailed=True, search_opts=None, sort_key=None,
-             sort_dir=None):
+    def list(self, detailed=True, search_opts=None, sort=None):
         """Get a list of all volume transfer.
 
         :param detailed: Get detailed object information.
         :param search_opts: Filtering options.
-        :param sort_key: Optional key to sort on.
-        :param sort_dir: Optional direction to sort.
+        :param sort: Sort information
         :rtype: list of :class:`VolumeTransfer`
         """
         resource_type = 'os-volume-transfer'
@@ -78,7 +76,7 @@ class VolumeTransferManager(volume_transfers.VolumeTransferManager):
 
         url = self._build_list_url(resource_type, detailed=detailed,
                                    search_opts=search_opts,
-                                   sort_key=sort_key, sort_dir=sort_dir)
+                                   sort=sort)
         return self._list(url, 'transfers')
 
     def delete(self, transfer_id):
diff --git a/releasenotes/notes/cinderclient-5-de0508ce5a221d21.yaml b/releasenotes/notes/cinderclient-5-de0508ce5a221d21.yaml
index d307182..2be8179 100644
--- a/releasenotes/notes/cinderclient-5-de0508ce5a221d21.yaml
+++ b/releasenotes/notes/cinderclient-5-de0508ce5a221d21.yaml
@@ -26,3 +26,6 @@ upgrade:
     The deprecated volume create option ``--allow-multiattach`` has now been
     removed. Multiattach capability is now controlled using `volume-type extra
     specs <https://docs.openstack.org/cinder/latest/admin/blockstorage-volume-multiattach.html>`_.
+  - |
+    Support for the deprecated ``--sort_key`` and ``--sort_dir`` arguments have
+    now been dropped. Use the supported ``--sort`` argument instead.