Skip to content

Commit 246040a

Browse files
stmcginnisostackbrian
stmcginnis
authored and
ostackbrian
committed
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>
1 parent 3c1b417 commit 246040a

10 files changed

Lines changed: 22 additions & 162 deletions

File tree

‎cinderclient/base.py‎

Lines changed: 4 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,7 @@ def _list(self, url, response_key, obj_class=None, body=None,
131131
return common_base.ListWithMeta(items, resp)
132132

133133
def _build_list_url(self, resource_type, detailed=True, search_opts=None,
134-
marker=None, limit=None, sort_key=None, sort_dir=None,
135-
sort=None, offset=None):
134+
marker=None, limit=None, sort=None, offset=None):
136135

137136
if search_opts is None:
138137
search_opts = {}
@@ -151,16 +150,6 @@ def _build_list_url(self, resource_type, detailed=True, search_opts=None,
151150
if sort:
152151
query_params['sort'] = self._format_sort_param(sort,
153152
resource_type)
154-
else:
155-
# sort_key and sort_dir deprecated in kilo, prefer sort
156-
if sort_key:
157-
query_params['sort_key'] = self._format_sort_key_param(
158-
sort_key,
159-
resource_type)
160-
161-
if sort_dir:
162-
query_params['sort_dir'] = self._format_sort_dir_param(
163-
sort_dir)
164153

165154
if offset:
166155
query_params['offset'] = offset
@@ -179,7 +168,7 @@ def _build_list_url(self, resource_type, detailed=True, search_opts=None,
179168
"query_string": query_string})
180169

181170
def _format_sort_param(self, sort, resource_type=None):
182-
'''Formats the sort information into the sort query string parameter.
171+
"""Formats the sort information into the sort query string parameter.
183172
184173
The input sort information can be any of the following:
185174
- Comma-separated string in the form of <key[:dir]>
@@ -195,7 +184,7 @@ def _format_sort_param(self, sort, resource_type=None):
195184
:returns: Formatted query string parameter or None
196185
:raise ValueError: If an invalid sort direction or invalid sort key is
197186
given
198-
'''
187+
"""
199188
if not sort:
200189
return None
201190

@@ -205,11 +194,7 @@ def _format_sort_param(self, sort, resource_type=None):
205194

206195
sort_array = []
207196
for sort_item in sort:
208-
if isinstance(sort_item, tuple):
209-
sort_key = sort_item[0]
210-
sort_dir = sort_item[1]
211-
else:
212-
sort_key, _sep, sort_dir = sort_item.partition(':')
197+
sort_key, _sep, sort_dir = sort_item.partition(':')
213198
sort_key = sort_key.strip()
214199
sort_key = self._format_sort_key_param(sort_key, resource_type)
215200
if sort_dir:
@@ -237,14 +222,6 @@ def _format_sort_key_param(self, sort_key, resource_type=None):
237222
', '.join(valid_sort_keys))
238223
raise ValueError(msg)
239224

240-
def _format_sort_dir_param(self, sort_dir):
241-
if sort_dir in SORT_DIR_VALUES:
242-
return sort_dir
243-
244-
msg = ('sort_dir must be one of the following: %s.'
245-
% ', '.join(SORT_DIR_VALUES))
246-
raise ValueError(msg)
247-
248225
@contextlib.contextmanager
249226
def completion_cache(self, cache_type, obj_class, mode):
250227
"""

‎cinderclient/tests/unit/v2/test_shell.py‎

Lines changed: 1 addition & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -219,37 +219,11 @@ def test_list_field_with_tenant(self, mock_print):
219219
mock_print.assert_called_once_with(mock.ANY, key_list,
220220
exclude_unavailable=True, sortby_index=0)
221221

222-
def test_list_sort_valid(self):
223-
self.run_command('list --sort_key=id --sort_dir=asc')
224-
self.assert_called('GET', '/volumes/detail?sort_dir=asc&sort_key=id')
225-
226-
def test_list_sort_key_name(self):
227-
# Client 'name' key is mapped to 'display_name'
228-
self.run_command('list --sort_key=name')
229-
self.assert_called('GET', '/volumes/detail?sort_key=display_name')
230-
231222
def test_list_sort_name(self):
232223
# Client 'name' key is mapped to 'display_name'
233224
self.run_command('list --sort=name')
234225
self.assert_called('GET', '/volumes/detail?sort=display_name')
235226

236-
def test_list_sort_key_invalid(self):
237-
self.assertRaises(ValueError,
238-
self.run_command,
239-
'list --sort_key=foo --sort_dir=asc')
240-
241-
def test_list_sort_dir_invalid(self):
242-
self.assertRaises(ValueError,
243-
self.run_command,
244-
'list --sort_key=id --sort_dir=foo')
245-
246-
def test_list_mix_sort_args(self):
247-
cmds = ['list --sort name:desc --sort_key=status',
248-
'list --sort name:desc --sort_dir=asc',
249-
'list --sort name:desc --sort_key=status --sort_dir=asc']
250-
for cmd in cmds:
251-
self.assertRaises(exceptions.CommandError, self.run_command, cmd)
252-
253227
def test_list_sort_single_key_only(self):
254228
self.run_command('list --sort=id')
255229
self.assert_called('GET', '/volumes/detail?sort=id')
@@ -277,10 +251,7 @@ def test_list_sort_multiple_keys_and_dirs(self):
277251

278252
def test_list_reorder_with_sort(self):
279253
# sortby_index is None if there is sort information
280-
for cmd in ['list --sort_key=name',
281-
'list --sort_dir=asc',
282-
'list --sort_key=name --sort_dir=asc',
283-
'list --sort=name',
254+
for cmd in ['list --sort=name',
284255
'list --sort=name:asc']:
285256
with mock.patch('cinderclient.utils.print_list') as mock_print:
286257
self.run_command(cmd)

‎cinderclient/tests/unit/v2/test_volumes.py‎

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,6 @@ def test_list_volumes_with_marker_limit(self):
2929
cs.assert_called('GET', '/volumes/detail?limit=2&marker=1234')
3030
self._assert_request_id(lst)
3131

32-
def test_list_volumes_with_sort_key_dir(self):
33-
lst = cs.volumes.list(sort_key='id', sort_dir='asc')
34-
cs.assert_called('GET', '/volumes/detail?sort_dir=asc&sort_key=id')
35-
self._assert_request_id(lst)
36-
37-
def test_list_volumes_with_invalid_sort_key(self):
38-
self.assertRaises(ValueError,
39-
cs.volumes.list, sort_key='invalid', sort_dir='asc')
40-
41-
def test_list_volumes_with_invalid_sort_dir(self):
42-
self.assertRaises(ValueError,
43-
cs.volumes.list, sort_key='id', sort_dir='invalid')
44-
4532
def test__list(self):
4633
# There only 2 volumes available for our tests, so we set limit to 2.
4734
limit = 2
@@ -345,21 +332,10 @@ def test_format_sort_list_of_strings(self):
345332
self.assertEqual('id:asc,status,size:desc',
346333
cs.volumes._format_sort_param(s))
347334

348-
def test_format_sort_list_of_tuples(self):
349-
s = [('id', 'asc'), 'status', ('size', 'desc')]
350-
self.assertEqual('id:asc,status,size:desc',
351-
cs.volumes._format_sort_param(s))
352-
353-
def test_format_sort_list_of_strings_and_tuples(self):
354-
s = [('id', 'asc'), 'status', 'size:desc']
355-
self.assertEqual('id:asc,status,size:desc',
356-
cs.volumes._format_sort_param(s))
357-
358335
def test_format_sort_invalid_direction(self):
359336
for s in ['id:foo',
360337
'id:asc,status,size:foo',
361-
['id', 'status', 'size:foo'],
362-
['id', 'status', ('size', 'foo')]]:
338+
['id', 'status', 'size:foo']]:
363339
self.assertRaises(ValueError,
364340
cs.volumes._format_sort_param,
365341
s)

‎cinderclient/tests/unit/v3/test_shell.py‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1404,21 +1404,6 @@ def test_create_transfer_no_snaps(self):
14041404
}}
14051405
self.assert_called('POST', '/volume-transfers', body=expected)
14061406

1407-
def test_list_transfer_sort_key(self):
1408-
self.run_command(
1409-
'--os-volume-api-version 3.59 transfer-list --sort=id')
1410-
url = ('/volume-transfers/detail?%s' %
1411-
parse.urlencode([('sort_key', 'id')]))
1412-
self.assert_called('GET', url)
1413-
1414-
def test_list_transfer_sort_key_dir(self):
1415-
self.run_command(
1416-
'--os-volume-api-version 3.59 transfer-list --sort=id:asc')
1417-
url = ('/volume-transfers/detail?%s' %
1418-
parse.urlencode([('sort_dir', 'asc'),
1419-
('sort_key', 'id')]))
1420-
self.assert_called('GET', url)
1421-
14221407
def test_list_transfer_sorty_not_sorty(self):
14231408
self.run_command(
14241409
'--os-volume-api-version 3.59 transfer-list')

‎cinderclient/v2/shell.py‎

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -100,14 +100,6 @@ def _translate_attachments(info):
100100
'Use the show command to see which fields are available. '
101101
'Unavailable/non-existent fields will be ignored. '
102102
'Default=None.')
103-
@utils.arg('--sort_key',
104-
metavar='<sort_key>',
105-
default=None,
106-
help=argparse.SUPPRESS)
107-
@utils.arg('--sort_dir',
108-
metavar='<sort_dir>',
109-
default=None,
110-
help=argparse.SUPPRESS)
111103
@utils.arg('--sort',
112104
metavar='<key>[:<direction>]',
113105
default=None,
@@ -147,16 +139,8 @@ def do_list(cs, args):
147139
for field_title in args.fields.split(','):
148140
field_titles.append(field_title)
149141

150-
# --sort_key and --sort_dir deprecated in kilo and is not supported
151-
# with --sort
152-
if args.sort and (args.sort_key or args.sort_dir):
153-
raise exceptions.CommandError(
154-
'The --sort_key and --sort_dir arguments are deprecated and are '
155-
'not supported with --sort.')
156-
157142
volumes = cs.volumes.list(search_opts=search_opts, marker=args.marker,
158-
limit=args.limit, sort_key=args.sort_key,
159-
sort_dir=args.sort_dir, sort=args.sort)
143+
limit=args.limit, sort=args.sort)
160144
shell_utils.translate_volume_keys(volumes)
161145

162146
# Create a list of servers to which the volume is attached
@@ -178,7 +162,7 @@ def do_list(cs, args):
178162
if search_opts['all_tenants']:
179163
key_list.insert(1, 'Tenant ID')
180164

181-
if args.sort_key or args.sort_dir or args.sort:
165+
if args.sort:
182166
sortby_index = None
183167
else:
184168
sortby_index = 0

‎cinderclient/v2/volumes.py‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -281,26 +281,22 @@ def get(self, volume_id):
281281
return self._get("/volumes/%s" % volume_id, "volume")
282282

283283
def list(self, detailed=True, search_opts=None, marker=None, limit=None,
284-
sort_key=None, sort_dir=None, sort=None):
284+
sort=None):
285285
"""Lists all volumes.
286286
287287
:param detailed: Whether to return detailed volume info.
288288
:param search_opts: Search options to filter out volumes.
289289
:param marker: Begin returning volumes that appear later in the volume
290290
list than that represented by this volume id.
291291
:param limit: Maximum number of volumes to return.
292-
:param sort_key: Key to be sorted; deprecated in kilo
293-
:param sort_dir: Sort direction, should be 'desc' or 'asc'; deprecated
294-
in kilo
295292
:param sort: Sort information
296293
:rtype: list of :class:`Volume`
297294
"""
298295

299296
resource_type = "volumes"
300297
url = self._build_list_url(resource_type, detailed=detailed,
301298
search_opts=search_opts, marker=marker,
302-
limit=limit, sort_key=sort_key,
303-
sort_dir=sort_dir, sort=sort)
299+
limit=limit, sort=sort)
304300
return self._list(url, resource_type, limit=limit)
305301

306302
def delete(self, volume, cascade=False):

‎cinderclient/v3/attachments.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,16 +45,15 @@ def delete(self, attachment):
4545

4646
@api_versions.wraps('3.27')
4747
def list(self, detailed=False, search_opts=None, marker=None, limit=None,
48-
sort_key=None, sort_dir=None, sort=None):
48+
sort=None):
4949
"""List all attachments."""
5050
resource_type = "attachments"
5151
url = self._build_list_url(resource_type,
5252
detailed=detailed,
5353
search_opts=search_opts,
5454
marker=marker,
5555
limit=limit,
56-
sort_key=sort_key,
57-
sort_dir=sort_dir, sort=sort)
56+
sort=sort)
5857
return self._list(url, resource_type, limit=limit)
5958

6059
@api_versions.wraps('3.27')

‎cinderclient/v3/shell.py‎

Lines changed: 4 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -323,14 +323,6 @@ def do_get_pools(cs, args):
323323
'Use the show command to see which fields are available. '
324324
'Unavailable/non-existent fields will be ignored. '
325325
'Default=None.')
326-
@utils.arg('--sort_key',
327-
metavar='<sort_key>',
328-
default=None,
329-
help=argparse.SUPPRESS)
330-
@utils.arg('--sort_dir',
331-
metavar='<sort_dir>',
332-
default=None,
333-
help=argparse.SUPPRESS)
334326
@utils.arg('--sort',
335327
metavar='<key>[:<direction>]',
336328
default=None,
@@ -397,24 +389,15 @@ def do_list(cs, args):
397389
for field_title in args.fields.split(','):
398390
field_titles.append(field_title)
399391

400-
# --sort_key and --sort_dir deprecated in kilo and is not supported
401-
# with --sort
402-
if args.sort and (args.sort_key or args.sort_dir):
403-
raise exceptions.CommandError(
404-
'The --sort_key and --sort_dir arguments are deprecated and are '
405-
'not supported with --sort.')
406-
407392
total_count = 0
408393
if show_count:
409394
search_opts['with_count'] = args.with_count
410395
volumes, total_count = cs.volumes.list(
411396
search_opts=search_opts, marker=args.marker,
412-
limit=args.limit, sort_key=args.sort_key,
413-
sort_dir=args.sort_dir, sort=args.sort)
397+
limit=args.limit, sort=args.sort)
414398
else:
415399
volumes = cs.volumes.list(search_opts=search_opts, marker=args.marker,
416-
limit=args.limit, sort_key=args.sort_key,
417-
sort_dir=args.sort_dir, sort=args.sort)
400+
limit=args.limit, sort=args.sort)
418401
shell_utils.translate_volume_keys(volumes)
419402

420403
# Create a list of servers to which the volume is attached
@@ -450,7 +433,7 @@ def do_list(cs, args):
450433
if search_opts['all_tenants']:
451434
key_list.insert(1, 'Tenant ID')
452435

453-
if args.sort_key or args.sort_dir or args.sort:
436+
if args.sort:
454437
sortby_index = None
455438
else:
456439
sortby_index = 0
@@ -2563,25 +2546,13 @@ def do_transfer_list(cs, args):
25632546
}
25642547

25652548
sort = getattr(args, 'sort', None)
2566-
sort_key = None
2567-
sort_dir = None
25682549
if sort:
2569-
# We added this feature with sort_key and sort_dir, but that was a
2570-
# mistake as we've deprecated that construct a long time ago and should
2571-
# be removing it in favor of --sort. Too late for the service side, but
2572-
# to make the client experience consistent, we handle the compatibility
2573-
# here.
25742550
sort_args = sort.split(':')
25752551
if len(sort_args) > 2:
25762552
raise exceptions.CommandError(
25772553
'Invalid sort parameter provided. Argument must be in the '
25782554
'form "key[:<asc|desc>]".')
25792555

2580-
sort_key = sort_args[0]
2581-
if len(sort_args) == 2:
2582-
sort_dir = sort_args[1]
2583-
2584-
transfers = cs.transfers.list(
2585-
search_opts=search_opts, sort_key=sort_key, sort_dir=sort_dir)
2556+
transfers = cs.transfers.list(search_opts=search_opts, sort=sort)
25862557
columns = ['ID', 'Volume ID', 'Name']
25872558
utils.print_list(transfers, columns)

‎cinderclient/v3/volume_transfers.py‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,14 +62,12 @@ def get(self, transfer_id):
6262

6363
return self._get("/os-volume-transfer/%s" % transfer_id, "transfer")
6464

65-
def list(self, detailed=True, search_opts=None, sort_key=None,
66-
sort_dir=None):
65+
def list(self, detailed=True, search_opts=None, sort=None):
6766
"""Get a list of all volume transfer.
6867
6968
:param detailed: Get detailed object information.
7069
:param search_opts: Filtering options.
71-
:param sort_key: Optional key to sort on.
72-
:param sort_dir: Optional direction to sort.
70+
:param sort: Sort information
7371
:rtype: list of :class:`VolumeTransfer`
7472
"""
7573
resource_type = 'os-volume-transfer'
@@ -78,7 +76,7 @@ def list(self, detailed=True, search_opts=None, sort_key=None,
7876

7977
url = self._build_list_url(resource_type, detailed=detailed,
8078
search_opts=search_opts,
81-
sort_key=sort_key, sort_dir=sort_dir)
79+
sort=sort)
8280
return self._list(url, 'transfers')
8381

8482
def delete(self, transfer_id):

‎releasenotes/notes/cinderclient-5-de0508ce5a221d21.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,6 @@ upgrade:
2626
The deprecated volume create option ``--allow-multiattach`` has now been
2727
removed. Multiattach capability is now controlled using `volume-type extra
2828
specs <https://docs.openstack.org/cinder/latest/admin/blockstorage-volume-multiattach.html>`_.
29+
- |
30+
Support for the deprecated ``--sort_key`` and ``--sort_dir`` arguments have
31+
now been dropped. Use the supported ``--sort`` argument instead.

0 commit comments

Comments
 (0)