openstack/python-cinderclient
Code Issues Proposed changes

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

Browse Source
This commit is contained in:
Zuul 2019-09-11 20:29:58 +00:00 committed by Gerrit Code Review
commit a63d4d651a
10 changed files with 22 additions and 162 deletions

31
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):
"""

31
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)

26
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)

15
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')

20
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

8
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):

5
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')

37
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)

8
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):

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