From 961691e2e715fd437fa1c7901bc64ddb4d6787bc Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Thu, 30 Apr 2026 13:00:44 +0000 Subject: [PATCH 1/7] Add pagination for transfer executions At the moment, listing endpoint instances is the only Coriolis API that supports pagination. For this reason, Coriolis clients often retrieve much more database records than needed, leading to poor performance. For this reason, we're now adding pagination to other Coriolis APIs, starting with the transfer executions. A transfer can have a large amount of executions, especially in case of cron jobs. New optional parameters: * limit - the maximum number of entries to retrieve * marker - the last seen id, will not be retrieved again The pagination will be performed on the db API side, leveraging the "utils.paginate_query" helper from oslo_db. --- coriolisclient/base.py | 8 +++++++- coriolisclient/cli/transfer_executions.py | 6 ++++++ coriolisclient/v1/transfer_executions.py | 11 +++++++++-- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/coriolisclient/base.py b/coriolisclient/base.py index 22172ef..c1d6f93 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 | 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,12 @@ 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 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/transfer_executions.py b/coriolisclient/cli/transfer_executions.py index 0b1e199..b7b88d6 100644 --- a/coriolisclient/cli/transfer_executions.py +++ b/coriolisclient/cli/transfer_executions.py @@ -180,6 +180,12 @@ 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.') return parser def take_action(self, args): diff --git a/coriolisclient/v1/transfer_executions.py b/coriolisclient/v1/transfer_executions.py index 0552aae..94b41b2 100644 --- a/coriolisclient/v1/transfer_executions.py +++ b/coriolisclient/v1/transfer_executions.py @@ -34,9 +34,16 @@ 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): + query = {} + if marker is not None: + query['marker'] = marker + if limit is not None: + query['limit'] = limit + 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( From 29dbdddd20d05737d9f992d036aa148c5f38f5d6 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Mon, 4 May 2026 12:30:12 +0000 Subject: [PATCH 2/7] Add sort key and direction parameters --- coriolisclient/base.py | 5 +++-- coriolisclient/cli/transfer_executions.py | 13 ++++++++++++- coriolisclient/cli/utils.py | 23 +++++++++++++++++++++++ coriolisclient/v1/transfer_executions.py | 16 ++++++++++++---- 4 files changed, 50 insertions(+), 7 deletions(-) diff --git a/coriolisclient/base.py b/coriolisclient/base.py index c1d6f93..752b5d6 100644 --- a/coriolisclient/base.py +++ b/coriolisclient/base.py @@ -167,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', query: dict | None=None): + 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, @@ -177,7 +177,8 @@ 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 containing query filters + :param query: an optional dict or list of (key, value) tuples + containing query filters """ if query: diff --git a/coriolisclient/cli/transfer_executions.py b/coriolisclient/cli/transfer_executions.py index b7b88d6..1f42ae7 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): @@ -186,9 +187,19 @@ def get_parser(self, prog_name): 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_args(arg.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/utils.py b/coriolisclient/cli/utils.py index 730363f..2f03214 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,25 @@ 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) -> 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 = [] + 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/v1/transfer_executions.py b/coriolisclient/v1/transfer_executions.py index 94b41b2..c11efe6 100644 --- a/coriolisclient/v1/transfer_executions.py +++ b/coriolisclient/v1/transfer_executions.py @@ -34,12 +34,20 @@ class TransferExecutionManager(base.BaseManager): def __init__(self, api): super(TransferExecutionManager, self).__init__(api) - def list(self, transfer, marker=None, limit=None): - query = {} + 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['marker'] = marker + query.append(("marker", marker)) if limit is not None: - query['limit'] = limit + 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', From d418a1175374f2f1447aa6c9ef252c861f094083 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Tue, 5 May 2026 08:35:30 +0000 Subject: [PATCH 3/7] Add deployment pagination --- coriolisclient/cli/deployments.py | 17 ++++++++++++++++- coriolisclient/v1/deployments.py | 18 ++++++++++++++++-- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/coriolisclient/cli/deployments.py b/coriolisclient/cli/deployments.py index 74c6139..f296e9f 100644 --- a/coriolisclient/cli/deployments.py +++ b/coriolisclient/cli/deployments.py @@ -273,8 +273,23 @@ 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 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): - obj_list = self.app.client_manager.coriolis.deployments.list() + 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/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( From 6138a56e76614d404efc37bcdd72fc035854bd8b Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Tue, 5 May 2026 10:14:57 +0000 Subject: [PATCH 4/7] Add transfer pagination --- coriolisclient/base.py | 2 +- coriolisclient/cli/deployments.py | 10 +++++----- coriolisclient/cli/transfer_executions.py | 12 ++++++------ coriolisclient/cli/transfers.py | 17 ++++++++++++++++- coriolisclient/v1/transfers.py | 18 ++++++++++++++++-- 5 files changed, 44 insertions(+), 15 deletions(-) diff --git a/coriolisclient/base.py b/coriolisclient/base.py index 752b5d6..4039d9e 100644 --- a/coriolisclient/base.py +++ b/coriolisclient/base.py @@ -167,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', query: dict | list | None=None): + 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, diff --git a/coriolisclient/cli/deployments.py b/coriolisclient/cli/deployments.py index f296e9f..8ce370a 100644 --- a/coriolisclient/cli/deployments.py +++ b/coriolisclient/cli/deployments.py @@ -281,15 +281,15 @@ def get_parser(self, prog_name): 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.') + 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( marker=args.marker, limit=args.limit, - sort_keys=sort_keys, - sort_dirs=sort_dirs) + sort_keys=args.sort_keys, + sort_dirs=args.sort_dirs) return DeploymentFormatter().list_objects(obj_list) diff --git a/coriolisclient/cli/transfer_executions.py b/coriolisclient/cli/transfer_executions.py index 1f42ae7..33a0d4a 100644 --- a/coriolisclient/cli/transfer_executions.py +++ b/coriolisclient/cli/transfer_executions.py @@ -189,17 +189,17 @@ def get_parser(self, prog_name): 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.') + 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_args(arg.sort) + sort_keys, sort_dirs = cli_utils.parse_sort_args(args.sort) obj_list = self.app.client_manager.coriolis.transfer_executions.list( args.transfer, marker=args.marker, limit=args.limit, - sort_keys=sort_keys, - sort_dirs=sort_dirs) + sort_keys=args.sort_keys, + sort_dirs=args.sort_dirs) return TransferExecutionFormatter().list_objects(obj_list) diff --git a/coriolisclient/cli/transfers.py b/coriolisclient/cli/transfers.py index 37e1bbb..91c31a9 100644 --- a/coriolisclient/cli/transfers.py +++ b/coriolisclient/cli/transfers.py @@ -320,10 +320,25 @@ 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 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): - obj_list = self.app.client_manager.coriolis.transfers.list() + obj_list = self.app.client_manager.coriolis.transfers.list( + marker=args.marker, + limit=args.limit, + sort_keys=args.sort_keys, + sort_dirs=args.sort_dirs) return TransferFormatter().list_objects(obj_list) 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') From 51f6d3e51464ba88d03739cce95bd89758e0f7f2 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Tue, 5 May 2026 12:18:05 +0000 Subject: [PATCH 5/7] Drop Python 3.8 and 3.9 tests Python 3.9 does not support modern type hints. It's no longer supported by coriolis-core, as such we'll drop it from the python-coriolisclient pipeline. --- .github/workflows/unit-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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: From 01529cf6903b604954f3e249347f4fe3980c4140 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Tue, 5 May 2026 13:07:39 +0000 Subject: [PATCH 6/7] Update unit tests, covering paging --- coriolisclient/cli/deployments.py | 6 ++- coriolisclient/cli/transfer_executions.py | 6 +-- coriolisclient/cli/transfers.py | 6 ++- coriolisclient/cli/utils.py | 5 ++- coriolisclient/tests/cli/test_deployments.py | 15 ++++++- .../tests/cli/test_transfer_executions.py | 13 +++++- coriolisclient/tests/cli/test_transfers.py | 21 +++++++-- coriolisclient/tests/cli/test_utils.py | 10 +++++ coriolisclient/tests/test_base.py | 43 ++++++++++++++++++- coriolisclient/tests/v1/test_deployments.py | 23 +++++++++- .../tests/v1/test_transfer_executions.py | 28 +++++++++++- coriolisclient/tests/v1/test_transfers.py | 28 +++++++++++- 12 files changed, 184 insertions(+), 20 deletions(-) diff --git a/coriolisclient/cli/deployments.py b/coriolisclient/cli/deployments.py index 8ce370a..591de14 100644 --- a/coriolisclient/cli/deployments.py +++ b/coriolisclient/cli/deployments.py @@ -287,9 +287,11 @@ def get_parser(self, prog_name): 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.deployments.list( marker=args.marker, limit=args.limit, - sort_keys=args.sort_keys, - sort_dirs=args.sort_dirs) + 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 33a0d4a..442dcc3 100644 --- a/coriolisclient/cli/transfer_executions.py +++ b/coriolisclient/cli/transfer_executions.py @@ -195,11 +195,11 @@ def get_parser(self, prog_name): return parser def take_action(self, args): - sort_keys, sort_dirs = cli_utils.parse_sort_args(args.sort) + sort_keys, sort_dirs = cli_utils.parse_sort_arg(args.sort) obj_list = self.app.client_manager.coriolis.transfer_executions.list( args.transfer, marker=args.marker, limit=args.limit, - sort_keys=args.sort_keys, - sort_dirs=args.sort_dirs) + 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 91c31a9..88d7cfa 100644 --- a/coriolisclient/cli/transfers.py +++ b/coriolisclient/cli/transfers.py @@ -334,11 +334,13 @@ def get_parser(self, prog_name): 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.transfers.list( marker=args.marker, limit=args.limit, - sort_keys=args.sort_keys, - sort_dirs=args.sort_dirs) + 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 2f03214..18c1f1d 100644 --- a/coriolisclient/cli/utils.py +++ b/coriolisclient/cli/utils.py @@ -267,7 +267,7 @@ def _split_pool_mapping_arg(arg): 'be of the form "INSTANCE_IDENTIFIER=MINION_POOL_ID".') -def parse_sort_arg(sort: str) -> tuple[list, list]: +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 @@ -277,6 +277,9 @@ def parse_sort_arg(sort: str) -> tuple[list, list]: """ 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: 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): From 8620dc161262de7d88ab91c17773e267535fbc1f Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Wed, 6 May 2026 09:41:51 +0000 Subject: [PATCH 7/7] Fix help strings --- coriolisclient/cli/deployments.py | 4 ++-- coriolisclient/cli/transfers.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/coriolisclient/cli/deployments.py b/coriolisclient/cli/deployments.py index 591de14..848adc9 100644 --- a/coriolisclient/cli/deployments.py +++ b/coriolisclient/cli/deployments.py @@ -275,10 +275,10 @@ 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 execution.') + help='The id of the last retrieved deployment.') parser.add_argument( '--limit', type=int, - help='Maximum number of executions to retrieve.') + help='Maximum number of deployments to retrieve.') parser.add_argument( '--sort', help='Comma-separated list of sort keys and directions in the ' diff --git a/coriolisclient/cli/transfers.py b/coriolisclient/cli/transfers.py index 88d7cfa..7ff54ed 100644 --- a/coriolisclient/cli/transfers.py +++ b/coriolisclient/cli/transfers.py @@ -322,10 +322,10 @@ 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 execution.') + help='The id of the last retrieved transfer.') parser.add_argument( '--limit', type=int, - help='Maximum number of executions to retrieve.') + help='Maximum number of transfers to retrieve.') parser.add_argument( '--sort', help='Comma-separated list of sort keys and directions in the '