From 246040a7325164eb0c7c3171dd21ceb7c7d149ce Mon Sep 17 00:00:00 2001
From: Sean McGinnis <sean.mcginnis@gmail.com>
Date: Thu, 18 Apr 2019 13:53:57 -0500
Subject: [PATCH] Drop support for --sort_key and --sort_dir
These arguments were deprecated in the kilo release in favor of a
combined --sort argument. This drops support for the deprecated
arguments.
Change-Id: If8f8ac44cc81f553009a15ca67257e86cb925b6f
Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com>
---
cinderclient/base.py | 31 ++--------------
cinderclient/tests/unit/v2/test_shell.py | 31 +---------------
cinderclient/tests/unit/v2/test_volumes.py | 26 +------------
cinderclient/tests/unit/v3/test_shell.py | 15 --------
cinderclient/v2/shell.py | 20 +---------
cinderclient/v2/volumes.py | 8 +---
cinderclient/v3/attachments.py | 5 +--
cinderclient/v3/shell.py | 37 ++-----------------
cinderclient/v3/volume_transfers.py | 8 ++--
.../cinderclient-5-de0508ce5a221d21.yaml | 3 ++
10 files changed, 22 insertions(+), 162 deletions(-)
diff --git a/cinderclient/base.py b/cinderclient/base.py
index e84eb2fac..a317ad432 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 22cb25ea5..a61075359 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 8704c667e..e4f9e323e 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 e33bbecc9..6e362395d 100644
--- a/cinderclient/tests/unit/v3/test_shell.py
+++ b/cinderclient/tests/unit/v3/test_shell.py
@@ -1404,21 +1404,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 20c7bd68e..b83175e8a 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 3cc0d4a99..4c380fbd0 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 a0732a3b7..e1e929003 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 784f37908..830185f94 100644
--- a/cinderclient/v3/shell.py
+++ b/cinderclient/v3/shell.py
@@ -323,14 +323,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,
@@ -397,24 +389,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
@@ -450,7 +433,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
@@ -2563,25 +2546,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 fe790f24f..f40c5199d 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 d3071823b..2be8179ed 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.