diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index b6cdd3f..333c846 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -13,7 +13,7 @@ jobs: runs-on: ubuntu-22.04 strategy: matrix: - python-version: ["3.8", "3.9", "3.10", "3.11"] + python-version: ["3.10", "3.11"] architecture: ["x64"] steps: diff --git a/coriolisclient/base.py b/coriolisclient/base.py index 22172ef..4039d9e 100644 --- a/coriolisclient/base.py +++ b/coriolisclient/base.py @@ -22,6 +22,7 @@ import traceback import six +from six.moves.urllib import parse as urlparse from keystoneauth1 import exceptions as keystoneauth_exceptions from oslo_utils import strutils @@ -166,7 +167,7 @@ def __init__(self, client): @wrap_unauthorized_exception def _list(self, url, response_key=None, obj_class=None, json=None, - values_key='values'): + values_key='values', query: dict | list | None = None): """List the collection. :param url: a partial URL, e.g., '/servers' :param response_key: the key to be looked up in response dictionary, @@ -176,7 +177,13 @@ def _list(self, url, response_key=None, obj_class=None, json=None, (self.resource_class will be used by default) :param json: data that will be encoded as JSON and passed in POST request (GET will be sent by default) + :param query: an optional dict or list of (key, value) tuples + containing query filters """ + + if query: + url += "?" + urlparse.urlencode(query) + if json: body = self.client.post(url, json=json).json() else: diff --git a/coriolisclient/cli/deployments.py b/coriolisclient/cli/deployments.py index 74c6139..848adc9 100644 --- a/coriolisclient/cli/deployments.py +++ b/coriolisclient/cli/deployments.py @@ -273,8 +273,25 @@ class ListDeployment(lister.Lister): def get_parser(self, prog_name): parser = super(ListDeployment, self).get_parser(prog_name) + parser.add_argument( + '--marker', + help='The id of the last retrieved deployment.') + parser.add_argument( + '--limit', type=int, + help='Maximum number of deployments to retrieve.') + parser.add_argument( + '--sort', + help='Comma-separated list of sort keys and directions in the ' + 'form of [:]. The direction defaults to ' + 'descending if not specified.') return parser def take_action(self, args): - obj_list = self.app.client_manager.coriolis.deployments.list() + sort_keys, sort_dirs = cli_utils.parse_sort_arg(args.sort) + obj_list = self.app.client_manager.coriolis.deployments.list( + marker=args.marker, + limit=args.limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs, + ) return DeploymentFormatter().list_objects(obj_list) diff --git a/coriolisclient/cli/transfer_executions.py b/coriolisclient/cli/transfer_executions.py index 0b1e199..442dcc3 100644 --- a/coriolisclient/cli/transfer_executions.py +++ b/coriolisclient/cli/transfer_executions.py @@ -22,6 +22,7 @@ from cliff import show from coriolisclient.cli import formatter +from coriolisclient.cli import utils as cli_utils class TransferExecutionFormatter(formatter.EntityFormatter): @@ -180,9 +181,25 @@ class ListTransferExecution(lister.Lister): def get_parser(self, prog_name): parser = super(ListTransferExecution, self).get_parser(prog_name) parser.add_argument('transfer', help='The transfer\'s id') + parser.add_argument( + '--marker', + help='The id of the last retrieved execution.') + parser.add_argument( + '--limit', type=int, + help='Maximum number of executions to retrieve.') + parser.add_argument( + '--sort', + help='Comma-separated list of sort keys and directions in the ' + 'form of [:]. The direction defaults to ' + 'descending if not specified.') return parser def take_action(self, args): + sort_keys, sort_dirs = cli_utils.parse_sort_arg(args.sort) obj_list = self.app.client_manager.coriolis.transfer_executions.list( - args.transfer) + args.transfer, + marker=args.marker, + limit=args.limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs) return TransferExecutionFormatter().list_objects(obj_list) diff --git a/coriolisclient/cli/transfers.py b/coriolisclient/cli/transfers.py index 37e1bbb..7ff54ed 100644 --- a/coriolisclient/cli/transfers.py +++ b/coriolisclient/cli/transfers.py @@ -320,10 +320,27 @@ class ListTransfer(lister.Lister): def get_parser(self, prog_name): parser = super(ListTransfer, self).get_parser(prog_name) + parser.add_argument( + '--marker', + help='The id of the last retrieved transfer.') + parser.add_argument( + '--limit', type=int, + help='Maximum number of transfers to retrieve.') + parser.add_argument( + '--sort', + help='Comma-separated list of sort keys and directions in the ' + 'form of [:]. The direction defaults to ' + 'descending if not specified.') return parser def take_action(self, args): - obj_list = self.app.client_manager.coriolis.transfers.list() + sort_keys, sort_dirs = cli_utils.parse_sort_arg(args.sort) + obj_list = self.app.client_manager.coriolis.transfers.list( + marker=args.marker, + limit=args.limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs, + ) return TransferFormatter().list_objects(obj_list) diff --git a/coriolisclient/cli/utils.py b/coriolisclient/cli/utils.py index 730363f..18c1f1d 100644 --- a/coriolisclient/cli/utils.py +++ b/coriolisclient/cli/utils.py @@ -20,6 +20,7 @@ import uuid from coriolisclient import constants +from coriolisclient import exceptions def add_storage_mappings_arguments_to_parser(parser): @@ -264,3 +265,28 @@ def _split_pool_mapping_arg(arg): 'same OS type and which are compatible with OSMorphing ' 'the guest OS of each afferent instance. The mappings must ' 'be of the form "INSTANCE_IDENTIFIER=MINION_POOL_ID".') + + +def parse_sort_arg(sort: str | None) -> tuple[list, list]: + """Parse sort CLI argument. + + :param sort: Comma-separated list of sort keys and directions in the form + of [:]. The direction defaults to descending if + not specified. + :returns: (sort_keys, sort_dirs) + """ + sort_keys = [] + sort_dirs = [] + if not sort: + return sort_keys, sort_dirs + + for sort_entry in sort.split(','): + sort_key, _sep, sort_dir = sort_entry.partition(':') + if not sort_dir: + sort_dir = 'desc' + elif sort_dir not in ('asc', 'desc'): + raise exceptions.CoriolisException( + 'Unknown sort direction: %s' % sort_dir) + sort_keys.append(sort_key) + sort_dirs.append(sort_dir) + return sort_keys, sort_dirs diff --git a/coriolisclient/tests/cli/test_deployments.py b/coriolisclient/tests/cli/test_deployments.py index f00065f..0aefa8a 100644 --- a/coriolisclient/tests/cli/test_deployments.py +++ b/coriolisclient/tests/cli/test_deployments.py @@ -329,12 +329,23 @@ def test_get_parser(self): parser = self.cli.get_parser('coriolis') self.assertIsInstance(parser, argparse.ArgumentParser) - def test_take_action(self): + @mock.patch("coriolisclient.cli.utils.parse_sort_arg") + def test_take_action(self, mock_parse_sort_arg): + mock_parse_sort_arg.return_value = ( + mock.sentinel.sort_keys, mock.sentinel.sort_dirs) + + mock_args = mock.MagicMock() mock_fun = self.mock_app.client_manager.coriolis.deployments.list mock_fun.return_value = [ v1_deployments.Deployment(mock.MagicMock(), DEPLOYMENT_LIST_DATA)] - columns, data = self.cli.take_action(mock.ANY) + columns, data = self.cli.take_action(mock_args) self.assertEqual(deployments.DeploymentFormatter().columns, columns) self.assertEqual([DEPLOYMENT_LIST_FORMATTED_DATA], list(data)) + mock_fun.assert_called_once_with( + marker=mock_args.marker, + limit=mock_args.limit, + sort_keys=mock.sentinel.sort_keys, + sort_dirs=mock.sentinel.sort_dirs, + ) diff --git a/coriolisclient/tests/cli/test_transfer_executions.py b/coriolisclient/tests/cli/test_transfer_executions.py index 1f3dc97..9dab178 100644 --- a/coriolisclient/tests/cli/test_transfer_executions.py +++ b/coriolisclient/tests/cli/test_transfer_executions.py @@ -419,10 +419,15 @@ def test_get_parser( @mock.patch.object(transfer_executions.TransferExecutionFormatter, 'list_objects') + @mock.patch("coriolisclient.cli.utils.parse_sort_arg") def test_take_action( self, + mock_parse_sort_arg, mock_list_objects ): + mock_parse_sort_arg.return_value = ( + mock.sentinel.sort_keys, mock.sentinel.sort_dirs) + args = mock.Mock() args.transfer = mock.sentinel.transfer mock_transfer_list = mock.Mock() @@ -435,6 +440,12 @@ def test_take_action( mock_list_objects.return_value, result ) - mock_transfer_list.assert_called_once_with(mock.sentinel.transfer) + mock_transfer_list.assert_called_once_with( + mock.sentinel.transfer, + marker=args.marker, + limit=args.limit, + sort_keys=mock.sentinel.sort_keys, + sort_dirs=mock.sentinel.sort_dirs, + ) mock_list_objects.assert_called_once_with( mock_transfer_list.return_value) diff --git a/coriolisclient/tests/cli/test_transfers.py b/coriolisclient/tests/cli/test_transfers.py index 9a49423..1ee4788 100644 --- a/coriolisclient/tests/cli/test_transfers.py +++ b/coriolisclient/tests/cli/test_transfers.py @@ -470,10 +470,16 @@ def test_get_parser(self, mock_get_parser): mock_get_parser.assert_called_once_with(mock.sentinel.prog_name) @mock.patch.object(transfers.TransferFormatter, 'list_objects') - def test_take_action(self, mock_list_objects): + @mock.patch("coriolisclient.cli.utils.parse_sort_arg") + def test_take_action(self, mock_parse_sort_arg, mock_list_objects): + mock_parse_sort_arg.return_value = ( + mock.sentinel.sort_keys, mock.sentinel.sort_dirs) + args = mock.Mock() - mock_transfer = mock.Mock() - self.mock_app.client_manager.coriolis.transfers.list = mock_transfer + args.sort = None + mock_transfer_list = mock.Mock() + self.mock_app.client_manager.coriolis.transfers.list = ( + mock_transfer_list) result = self.transfer.take_action(args) @@ -481,7 +487,14 @@ def test_take_action(self, mock_list_objects): mock_list_objects.return_value, result ) - mock_list_objects.assert_called_once_with(mock_transfer.return_value) + mock_list_objects.assert_called_once_with( + mock_transfer_list.return_value) + mock_transfer_list.assert_called_once_with( + marker=args.marker, + limit=args.limit, + sort_keys=mock.sentinel.sort_keys, + sort_dirs=mock.sentinel.sort_dirs, + ) class UpdateTransferTestCase(test_base.CoriolisBaseTestCase): diff --git a/coriolisclient/tests/cli/test_utils.py b/coriolisclient/tests/cli/test_utils.py index 61461fd..4dd6d63 100644 --- a/coriolisclient/tests/cli/test_utils.py +++ b/coriolisclient/tests/cli/test_utils.py @@ -323,3 +323,13 @@ def test_add_minion_pool_args_to_parser(self): [{'instance_id': 'mock_instance_id', 'pool_id': 'mock_pool_id'}], args.instance_osmorphing_minion_pool_mappings ) + + @ddt.data( + (None, ([], [])), + ("key0:asc,key1:desc,key2", + (["key0", "key1", "key2"], ["asc", "desc", "desc"])) + ) + @ddt.unpack + def test_parse_sort_arg(self, sort_arg, exp_ret): + ret = utils.parse_sort_arg(sort_arg) + self.assertEqual(exp_ret, ret) diff --git a/coriolisclient/tests/test_base.py b/coriolisclient/tests/test_base.py index dd6dd2f..305522b 100644 --- a/coriolisclient/tests/test_base.py +++ b/coriolisclient/tests/test_base.py @@ -279,7 +279,7 @@ def setUp(self): self.manager = base.BaseManager(mock_client) def test_list(self): - self.manager.client.get(mock.sentinel.url).json.return_value = { + self.manager.client.get.return_value.json.return_value = { "mock_response_key": { "data": [mock.sentinel.data1, mock.sentinel.data2]} } @@ -294,6 +294,7 @@ def test_list(self): values_key="data" ) + self.manager.client.get.assert_called_once_with(mock.sentinel.url) self.assertEqual( [obj_class.return_value] * 2, result @@ -303,6 +304,46 @@ def test_list(self): mock.call(self.manager, mock.sentinel.data2, loaded=True) ]) + def test_list_with_dict_query(self): + self.manager.client.get.return_value.json.return_value = { + "mock_response_key": {"data": []} + } + testutils.get_wrapped_function(self.manager._list)( + self.manager, + url="test-url", + response_key="mock_response_key", + obj_class=mock.Mock(), + json=None, + values_key="data", + query={ + "some_filter": "some_value", + "some_other_filter": "some_other_value" + } + ) + self.manager.client.get.assert_called_once_with( + "test-url?some_filter=some_value&" + "some_other_filter=some_other_value") + + def test_list_with_tuple_list_query(self): + self.manager.client.get.return_value.json.return_value = { + "mock_response_key": {"data": []} + } + testutils.get_wrapped_function(self.manager._list)( + self.manager, + url="test-url", + response_key="mock_response_key", + obj_class=mock.Mock(), + json=None, + values_key="data", + query=[ + ("some_filter", "some_value"), + ("some_other_filter", "some_other_value"), + ] + ) + self.manager.client.get.assert_called_once_with( + "test-url?some_filter=some_value&" + "some_other_filter=some_other_value") + def test_list_json(self): (self.manager.client.post(mock.sentinel.url, json=True).json. return_value) = [mock.sentinel.data] diff --git a/coriolisclient/tests/v1/test_deployments.py b/coriolisclient/tests/v1/test_deployments.py index f92fe9a..307d1ae 100644 --- a/coriolisclient/tests/v1/test_deployments.py +++ b/coriolisclient/tests/v1/test_deployments.py @@ -75,7 +75,28 @@ def test_list(self): result = self.deployments.list(detail=True) self.assertEqual(mock_list.return_value, result) mock_list.assert_called_once_with( - '/deployments/detail', 'deployments') + '/deployments/detail', 'deployments', query=[]) + + def test_list_with_pagination(self): + with mock.patch.object(self.deployments, '_list') as mock_list: + result = self.deployments.list( + detail=True, + marker=mock.sentinel.marker, + limit=mock.sentinel.limit, + sort_keys=[mock.sentinel.sort_key0, mock.sentinel.sort_key1], + sort_dirs=[mock.sentinel.sort_dir0, mock.sentinel.sort_dir1], + ) + exp_query = [ + ("marker", mock.sentinel.marker), + ("limit", mock.sentinel.limit), + ("sort_key", mock.sentinel.sort_key0), + ("sort_key", mock.sentinel.sort_key1), + ("sort_dir", mock.sentinel.sort_dir0), + ("sort_dir", mock.sentinel.sort_dir1), + ] + self.assertEqual(mock_list.return_value, result) + mock_list.assert_called_once_with( + '/deployments/detail', 'deployments', query=exp_query) def test_get(self): deployment = mock.Mock(uuid=DEPLOYMENT_ID) diff --git a/coriolisclient/tests/v1/test_transfer_executions.py b/coriolisclient/tests/v1/test_transfer_executions.py index ab5a7f0..11b20a2 100644 --- a/coriolisclient/tests/v1/test_transfer_executions.py +++ b/coriolisclient/tests/v1/test_transfer_executions.py @@ -49,7 +49,33 @@ def test_list(self, mock_list): result ) mock_list.assert_called_once_with( - '/transfers/%s/executions' % mock.sentinel.transfer, "executions") + '/transfers/%s/executions' % mock.sentinel.transfer, "executions", + query=[]) + + @mock.patch.object(transfer_executions.TransferExecutionManager, "_list") + def test_list_with_pagination(self, mock_list): + result = self.transfer_execution.list( + mock.sentinel.transfer, + marker=mock.sentinel.marker, + limit=mock.sentinel.limit, + sort_keys=[mock.sentinel.sort_key0, mock.sentinel.sort_key1], + sort_dirs=[mock.sentinel.sort_dir0, mock.sentinel.sort_dir1], + ) + exp_query = [ + ("marker", mock.sentinel.marker), + ("limit", mock.sentinel.limit), + ("sort_key", mock.sentinel.sort_key0), + ("sort_key", mock.sentinel.sort_key1), + ("sort_dir", mock.sentinel.sort_dir0), + ("sort_dir", mock.sentinel.sort_dir1), + ] + self.assertEqual( + mock_list.return_value, + result + ) + mock_list.assert_called_once_with( + '/transfers/%s/executions' % mock.sentinel.transfer, "executions", + query=exp_query) @mock.patch.object(transfer_executions.TransferExecutionManager, "_get") def test_get(self, mock_get): diff --git a/coriolisclient/tests/v1/test_transfers.py b/coriolisclient/tests/v1/test_transfers.py index 7966dc3..7987678 100644 --- a/coriolisclient/tests/v1/test_transfers.py +++ b/coriolisclient/tests/v1/test_transfers.py @@ -85,7 +85,30 @@ def test_list(self, mock_list): mock_list.return_value, result ) - mock_list.assert_called_once_with("/transfers", "transfers") + mock_list.assert_called_once_with("/transfers", "transfers", query=[]) + + @mock.patch.object(transfers.TransferManager, "_list") + def test_list_with_pagination(self, mock_list): + result = self.transfer.list( + detail=False, + marker=mock.sentinel.marker, + limit=mock.sentinel.limit, + sort_keys=[mock.sentinel.sort_key0, mock.sentinel.sort_key1], + sort_dirs=[mock.sentinel.sort_dir0, mock.sentinel.sort_dir1],) + exp_query = [ + ("marker", mock.sentinel.marker), + ("limit", mock.sentinel.limit), + ("sort_key", mock.sentinel.sort_key0), + ("sort_key", mock.sentinel.sort_key1), + ("sort_dir", mock.sentinel.sort_dir0), + ("sort_dir", mock.sentinel.sort_dir1), + ] + self.assertEqual( + mock_list.return_value, + result + ) + mock_list.assert_called_once_with( + "/transfers", "transfers", query=exp_query) @mock.patch.object(transfers.TransferManager, "_list") def test_list_details(self, mock_list): @@ -95,7 +118,8 @@ def test_list_details(self, mock_list): mock_list.return_value, result ) - mock_list.assert_called_once_with("/transfers/detail", "transfers") + mock_list.assert_called_once_with( + "/transfers/detail", "transfers", query=[]) @mock.patch.object(transfers.TransferManager, "_get") def test_get(self, mock_get): diff --git a/coriolisclient/v1/deployments.py b/coriolisclient/v1/deployments.py index 78558e7..a13e9a4 100644 --- a/coriolisclient/v1/deployments.py +++ b/coriolisclient/v1/deployments.py @@ -52,11 +52,25 @@ class DeploymentManager(base.BaseManager): def __init__(self, api): super(DeploymentManager, self).__init__(api) - def list(self, detail=False): + def list(self, detail=False, + marker=None, limit=None, + sort_keys=None, sort_dirs=None): + query = [] + if marker is not None: + query.append(("marker", marker)) + if limit is not None: + query.append(("limit", limit)) + if sort_keys is not None: + query.extend(('sort_key', sort_key) + for sort_key in sort_keys) + if sort_dirs is not None: + query.extend(('sort_dir', sort_dir) + for sort_dir in sort_dirs) + path = "/deployments" if detail: path = "%s/detail" % path - return self._list(path, 'deployments') + return self._list(path, 'deployments', query=query) def get(self, deployment): return self._get( diff --git a/coriolisclient/v1/transfer_executions.py b/coriolisclient/v1/transfer_executions.py index 0552aae..c11efe6 100644 --- a/coriolisclient/v1/transfer_executions.py +++ b/coriolisclient/v1/transfer_executions.py @@ -34,9 +34,24 @@ class TransferExecutionManager(base.BaseManager): def __init__(self, api): super(TransferExecutionManager, self).__init__(api) - def list(self, transfer): + def list(self, transfer, marker=None, limit=None, + sort_keys=None, sort_dirs=None): + # List of key-value tuples. + query = [] + if marker is not None: + query.append(("marker", marker)) + if limit is not None: + query.append(("limit", limit)) + if sort_keys is not None: + query.extend(('sort_key', sort_key) + for sort_key in sort_keys) + if sort_dirs is not None: + query.extend(('sort_dir', sort_dir) + for sort_dir in sort_dirs) + return self._list( - '/transfers/%s/executions' % base.getid(transfer), 'executions') + '/transfers/%s/executions' % base.getid(transfer), 'executions', + query=query) def get(self, transfer, execution): return self._get( diff --git a/coriolisclient/v1/transfers.py b/coriolisclient/v1/transfers.py index d4559d5..6a30da5 100644 --- a/coriolisclient/v1/transfers.py +++ b/coriolisclient/v1/transfers.py @@ -47,11 +47,25 @@ class TransferManager(base.BaseManager): def __init__(self, api): super(TransferManager, self).__init__(api) - def list(self, detail=False): + def list(self, detail=False, marker=None, limit=None, + sort_keys=None, sort_dirs=None): + # List of key-value tuples. + query = [] + if marker is not None: + query.append(("marker", marker)) + if limit is not None: + query.append(("limit", limit)) + if sort_keys is not None: + query.extend(('sort_key', sort_key) + for sort_key in sort_keys) + if sort_dirs is not None: + query.extend(('sort_dir', sort_dir) + for sort_dir in sort_dirs) + path = "/transfers" if detail: path = "%s/detail" % path - return self._list(path, 'transfers') + return self._list(path, 'transfers', query=query) def get(self, transfer): return self._get('/transfers/%s' % base.getid(transfer), 'transfer')