From 01ddab38ad9a0deb8ff5cb03f0e69736f9ba6e8f Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Thu, 30 Apr 2026 12:40:51 +0000 Subject: [PATCH 01/10] api: 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. 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. --- coriolis/api/v1/transfer_tasks_executions.py | 6 ++++- coriolis/conductor/rpc/client.py | 8 +++++-- coriolis/conductor/rpc/server.py | 9 ++++++-- coriolis/db/api.py | 23 +++++++++++++++++--- coriolis/exception.py | 5 +++++ coriolis/transfer_tasks_executions/api.py | 5 +++-- 6 files changed, 46 insertions(+), 10 deletions(-) diff --git a/coriolis/api/v1/transfer_tasks_executions.py b/coriolis/api/v1/transfer_tasks_executions.py index 9241b88f..8e4c5231 100644 --- a/coriolis/api/v1/transfer_tasks_executions.py +++ b/coriolis/api/v1/transfer_tasks_executions.py @@ -2,6 +2,7 @@ # All Rights Reserved. from coriolis.api.v1.views import transfer_tasks_execution_view +from coriolis.api import common from coriolis.api import wsgi as api_wsgi from coriolis import exception from coriolis.policies import transfer_tasks_executions as executions_policies @@ -31,9 +32,12 @@ def index(self, req, transfer_id): context.can( executions_policies.get_transfer_executions_policy_label("list")) + marker, limit = common.get_paging_params(req) + return transfer_tasks_execution_view.collection( self._transfer_tasks_execution_api.get_executions( - context, transfer_id, include_tasks=False)) + context, transfer_id, include_tasks=False, + marker=marker, limit=limit)) def detail(self, req, transfer_id): context = req.environ["coriolis.context"] diff --git a/coriolis/conductor/rpc/client.py b/coriolis/conductor/rpc/client.py index d77fb7f4..bf781d1f 100644 --- a/coriolis/conductor/rpc/client.py +++ b/coriolis/conductor/rpc/client.py @@ -143,11 +143,15 @@ def execute_transfer_tasks(self, ctxt, transfer_id, shutdown_instances=shutdown_instances, auto_deploy=auto_deploy) def get_transfer_tasks_executions(self, ctxt, transfer_id, - include_tasks=False): + include_tasks=False, + marker=None, + limit=None): return self._call( ctxt, 'get_transfer_tasks_executions', transfer_id=transfer_id, - include_tasks=include_tasks) + include_tasks=include_tasks, + marker=marker, + limit=limit) def get_transfer_tasks_execution(self, ctxt, transfer_id, execution_id, include_task_info=False): diff --git a/coriolis/conductor/rpc/server.py b/coriolis/conductor/rpc/server.py index 6db7d51c..d6623019 100644 --- a/coriolis/conductor/rpc/server.py +++ b/coriolis/conductor/rpc/server.py @@ -1152,10 +1152,15 @@ def execute_transfer_tasks(self, ctxt, transfer_id, shutdown_instances, @transfer_synchronized def get_transfer_tasks_executions(self, ctxt, transfer_id, include_tasks=False, - include_task_info=False): + include_task_info=False, + marker=None, + limit=None): return db_api.get_transfer_tasks_executions( ctxt, transfer_id, include_tasks, - include_task_info=include_task_info, to_dict=True) + include_task_info=include_task_info, + marker=marker, + limit=limit, + to_dict=True) @tasks_execution_synchronized def get_transfer_tasks_execution(self, ctxt, transfer_id, execution_id, diff --git a/coriolis/db/api.py b/coriolis/db/api.py index 816031a4..a7b3bdd5 100644 --- a/coriolis/db/api.py +++ b/coriolis/db/api.py @@ -7,6 +7,7 @@ from oslo_db import api as db_api from oslo_db import options as db_options from oslo_db.sqlalchemy import enginefacade +from oslo_db.sqlalchemy import utils as sqlalchemyutils from oslo_log import log as logging from oslo_utils import timeutils from sqlalchemy import func @@ -275,7 +276,10 @@ def delete_endpoint(context, endpoint_id): @enginefacade.reader def get_transfer_tasks_executions(context, transfer_id, include_tasks=False, - include_task_info=False, to_dict=False): + include_task_info=False, + marker=None, + limit=None, + to_dict=False): q = _soft_delete_aware_query(context, models.TasksExecution) q = q.join(models.Transfer) if include_task_info: @@ -285,8 +289,21 @@ def get_transfer_tasks_executions(context, transfer_id, include_tasks=False, if is_user_context(context): q = q.filter(models.Transfer.project_id == context.project_id) - db_result = q.filter( - models.Transfer.id == transfer_id).all() + q = q.filter(models.Transfer.id == transfer_id) + + if marker: + try: + marker = get_transfer_tasks_execution( + context, transfer_id, marker) + except exception.NotFound: + raise exception.MarkerNotFound(marker=marker) + + if marker or limit: + q = sqlalchemy.paginate_query( + q, models.TasksExecution, limit, + sort_keys=['id'], marker=marker) + + db_result = q.all() if to_dict: return [e.to_dict() for e in db_result] return db_result diff --git a/coriolis/exception.py b/coriolis/exception.py index 86671502..7de82395 100644 --- a/coriolis/exception.py +++ b/coriolis/exception.py @@ -289,6 +289,11 @@ class NotFound(CoriolisException): safe = True +class MarkerNotFound(NotFound): + message = _( + "Could not find database record " + "identified by marker: %(marker)s") + class RegionNotFound(NotFound): message = _("The specified Coriolis region(s) could not be found.") diff --git a/coriolis/transfer_tasks_executions/api.py b/coriolis/transfer_tasks_executions/api.py index 6e738599..ce71a083 100644 --- a/coriolis/transfer_tasks_executions/api.py +++ b/coriolis/transfer_tasks_executions/api.py @@ -20,9 +20,10 @@ def cancel(self, ctxt, transfer_id, execution_id, force): self._rpc_client.cancel_transfer_tasks_execution( ctxt, transfer_id, execution_id, force) - def get_executions(self, ctxt, transfer_id, include_tasks=False): + def get_executions(self, ctxt, transfer_id, include_tasks=False, + marker=None, limit=None): return self._rpc_client.get_transfer_tasks_executions( - ctxt, transfer_id, include_tasks) + ctxt, transfer_id, include_tasks, marker, limit) def get_execution(self, ctxt, transfer_id, execution_id): return self._rpc_client.get_transfer_tasks_execution( From 287ddbecfb05a05f9530c26ea0001a66d383233f Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Mon, 4 May 2026 06:35:21 +0000 Subject: [PATCH 02/10] Add paginiation integration tests --- coriolis/tests/integration/test_pagination.py | 141 ++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100755 coriolis/tests/integration/test_pagination.py diff --git a/coriolis/tests/integration/test_pagination.py b/coriolis/tests/integration/test_pagination.py new file mode 100755 index 00000000..73301184 --- /dev/null +++ b/coriolis/tests/integration/test_pagination.py @@ -0,0 +1,141 @@ +# Copyright 2026 Cloudbase Solutions Srl +# All Rights Reserved. + +"""API pagination tests.""" + +import datetime +import operator +import uuid + +from oslo_utils import timeutils + +from coriolis import constants +from coriolis import context as coriolis_context +from coriolis.db import api as db_api +from coriolis.db.sqlalchemy import models +from coriolis.tests.integration import base + + + +class PaginationTest(base.CoriolisIntegrationTestBase): + FAKE_USER_ID = "fake-user-id" + FAKE_PROJECT_ID = "fake-project-id" + + TRANSFER_COUNT = 5 + EXECUTIONS_PER_TRANSFER = 5 + + @classmethod + def setUpClass(cls): + super().setUpClass() + + cls._admin_ctx = coriolis_context.get_admin_context() + cls._admin_ctx.user_id = cls.FAKE_USER_ID + cls._admin_ctx.project_id = cls.FAKE_PROJECT_ID + + cls._setup_mocks() + + @classmethod + def _create_transfer( + cls, + origin_endpoint_id: str, + destination_endpoint_id: str, + instances: list[str] | None = None, + **kwargs, + ) -> models.Transfer: + kwargs["instances"] = instances or [] + kwargs["origin_endpoint_id"] = origin_endpoint_id + kwargs["destination_endpoint_id"] = destination_endpoint_id + kwargs["info"] = {instance: { + 'volumes_info': []} for instance in kwargs["instances"]} + transfer = models.Transfer(**kwargs) + db_api.add_transfer(cls._admin_ctx, transfer) + return transfer + + @classmethod + def _create_execution( + cls, + transfer: models.Transfer, + **kwargs, + ) -> models.TasksExecution: + kwargs["action_id"] = transfer.id + kwargs["status"] = kwargs.get( + "status", + constants.EXECUTION_STATUS_UNEXECUTED) + kwargs["type"] = kwargs.get( + "type", + constants.EXECUTION_TYPE_TRANSFER_EXECUTION) + execution = models.TasksExecution(**kwargs) + # "add_transfer_tasks_execution" expects "action" to be set, + # despite not being declared by the model. + execution.action = transfer + db_api.add_transfer_tasks_execution(cls._admin_ctx, execution) + return execution + + @classmethod + def _create_endpoint( + cls, + **kwargs, + ) -> models.Endpoint: + kwargs["id"] = kwargs.get("id", str(uuid.uuid4())) + kwargs["name"] = kwargs.get("name", f"test-endpoint-{kwargs["id"]}") + kwargs["type"] = kwargs.get("type", "openstack") + endpoint = models.Endpoint( + **kwargs) + db_api.add_endpoint(cls._admin_ctx, endpoint) + return endpoint + + @classmethod + def _setup_mocks(cls): + cls._src_endpoint = cls._create_endpoint() + cls._dst_endpoint = cls._create_endpoint() + + cls._transfers = [] + cls._executions = {} + for transfer_idx in range(cls.TRANSFER_COUNT): + # For testing purposes, we'll set the "created_at" field + # explicitly, adding a small time delta between records. + transfer = cls._create_transfer( + origin_endpoint_id=cls._src_endpoint.id, + destination_endpoint_id=cls._dst_endpoint.id, + created_at=timeutils.utcnow() + datetime.timedelta( + seconds=transfer_idx)) + cls._transfers.append(transfer) + + cls._executions[transfer.id] = [] + for execution_idx in range(cls.EXECUTIONS_PER_TRANSFER): + execution = cls._create_execution( + transfer=transfer, + created_at=timeutils.utcnow() + datetime.timedelta( + seconds=execution_idx)) + cls._executions[transfer.id].append(execution) + + @staticmethod + def _get_record_summary(record): + # Extract a few fields from the db records and entries returned by + # the API so that we can compare them. We don't intend to validate + # *all* fields, just the ones that are relevant for pagination. + created_at = record.created_at + if isinstance(created_at, str): + created_at = datetime.datetime.fromisoformat(created_at) + # The service may not have microsecond level precision + # and we need to compare records. + created_at = created_at.replace(microsecond=0) + return { + "id": record.id, + "created_at": created_at, + } + + def test_transfer_execution_list(self): + executions = self._client.transfer_executions.list( + self._transfers[0].id) + ret_exec_summary = [self._get_record_summary(e) for e in executions] + + exp_exec = self._executions[self._transfers[0].id] + sorted_exp_exec = sorted( + exp_exec, + key=operator.attrgetter('created_at'), + reverse=True) + exp_sorted_exec_summary = [ + self._get_record_summary(e) for e in sorted_exp_exec] + + self.assertEqual(exp_sorted_exec_summary, ret_exec_summary) From fcecf92621cb788b32f371bc4f140c60166d7ab5 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Mon, 4 May 2026 09:15:41 +0000 Subject: [PATCH 03/10] Add sort keys/dirs --- coriolis/api/common.py | 33 +++++++ coriolis/api/v1/transfer_tasks_executions.py | 4 +- coriolis/conductor/rpc/client.py | 9 +- coriolis/conductor/rpc/server.py | 6 +- coriolis/db/api.py | 94 +++++++++++++++++-- coriolis/tests/integration/test_pagination.py | 32 +++++++ coriolis/transfer_tasks_executions/api.py | 6 +- 7 files changed, 172 insertions(+), 12 deletions(-) diff --git a/coriolis/api/common.py b/coriolis/api/common.py index 5ff97ae3..8ce0226e 100644 --- a/coriolis/api/common.py +++ b/coriolis/api/common.py @@ -10,3 +10,36 @@ def get_paging_params(req): if limit is not None: limit = utils.parse_int_value(limit) return marker, limit + + +def get_sort_params(req, + default_key='created_at', + default_dir='desc'): + """Retrieves sort keys/directions parameters. + + Processes the parameters to create a list of sort keys and sort directions + that correspond to the 'sort_key' and 'sort_dir' parameter values. These + sorting parameters can be specified multiple times in order to generate + the list of sort keys and directions. + + The input parameters are not modified. + + :param req: coriolis.api.wsgi.Request object + :param default_key: default sort key value, added to the list if no + 'sort_key' parameters are supplied + :param default_dir: default sort dir value, added to the list if no + 'sort_dir' parameters are supplied + :returns: list of sort keys, list of sort dirs + """ + params = req.params.copy() + sort_keys = [] + sort_dirs = [] + while 'sort_key' in params: + sort_keys.append(params.pop('sort_key').strip()) + while 'sort_dir' in params: + sort_dirs.append(params.pop('sort_dir').strip()) + if len(sort_keys) == 0 and default_key: + sort_keys.append(default_key) + if len(sort_dirs) == 0 and default_dir: + sort_dirs.append(default_dir) + return sort_keys, sort_dirs diff --git a/coriolis/api/v1/transfer_tasks_executions.py b/coriolis/api/v1/transfer_tasks_executions.py index 8e4c5231..786e06e0 100644 --- a/coriolis/api/v1/transfer_tasks_executions.py +++ b/coriolis/api/v1/transfer_tasks_executions.py @@ -33,11 +33,13 @@ def index(self, req, transfer_id): executions_policies.get_transfer_executions_policy_label("list")) marker, limit = common.get_paging_params(req) + sort_keys, sort_dirs = common.get_sort_params(req) return transfer_tasks_execution_view.collection( self._transfer_tasks_execution_api.get_executions( context, transfer_id, include_tasks=False, - marker=marker, limit=limit)) + marker=marker, limit=limit, + sort_keys=sort_keys, sort_dirs=sort_dirs)) def detail(self, req, transfer_id): context = req.environ["coriolis.context"] diff --git a/coriolis/conductor/rpc/client.py b/coriolis/conductor/rpc/client.py index bf781d1f..d0066735 100644 --- a/coriolis/conductor/rpc/client.py +++ b/coriolis/conductor/rpc/client.py @@ -145,13 +145,18 @@ def execute_transfer_tasks(self, ctxt, transfer_id, def get_transfer_tasks_executions(self, ctxt, transfer_id, include_tasks=False, marker=None, - limit=None): + limit=None, + sort_keys=None, + sort_dirs=None): return self._call( ctxt, 'get_transfer_tasks_executions', transfer_id=transfer_id, include_tasks=include_tasks, marker=marker, - limit=limit) + limit=limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs, + ) def get_transfer_tasks_execution(self, ctxt, transfer_id, execution_id, include_task_info=False): diff --git a/coriolis/conductor/rpc/server.py b/coriolis/conductor/rpc/server.py index d6623019..2d1f4ea2 100644 --- a/coriolis/conductor/rpc/server.py +++ b/coriolis/conductor/rpc/server.py @@ -1154,12 +1154,16 @@ def get_transfer_tasks_executions(self, ctxt, transfer_id, include_tasks=False, include_task_info=False, marker=None, - limit=None): + limit=None, + sort_keys=None, + sort_dirs=None): return db_api.get_transfer_tasks_executions( ctxt, transfer_id, include_tasks, include_task_info=include_task_info, marker=marker, limit=limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs, to_dict=True) @tasks_execution_synchronized diff --git a/coriolis/db/api.py b/coriolis/db/api.py index a7b3bdd5..cd7fc59d 100644 --- a/coriolis/db/api.py +++ b/coriolis/db/api.py @@ -7,7 +7,7 @@ from oslo_db import api as db_api from oslo_db import options as db_options from oslo_db.sqlalchemy import enginefacade -from oslo_db.sqlalchemy import utils as sqlalchemyutils +from oslo_db.sqlalchemy import utils as sqlalchemy_utils from oslo_log import log as logging from oslo_utils import timeutils from sqlalchemy import func @@ -279,6 +279,8 @@ def get_transfer_tasks_executions(context, transfer_id, include_tasks=False, include_task_info=False, marker=None, limit=None, + sort_keys: list[str] | None = None, + sort_dirs: list[str] | None = None, to_dict=False): q = _soft_delete_aware_query(context, models.TasksExecution) q = q.join(models.Transfer) @@ -291,17 +293,22 @@ def get_transfer_tasks_executions(context, transfer_id, include_tasks=False, q = q.filter(models.Transfer.id == transfer_id) + sort_keys, sort_dirs = process_sort_params( + sort_keys, + sort_dirs, + ) if marker: try: marker = get_transfer_tasks_execution( context, transfer_id, marker) except exception.NotFound: raise exception.MarkerNotFound(marker=marker) - - if marker or limit: - q = sqlalchemy.paginate_query( - q, models.TasksExecution, limit, - sort_keys=['id'], marker=marker) + q = sqlalchemy_utils.paginate_query( + q, models.TasksExecution, limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs, + marker=marker, + ) db_result = q.all() if to_dict: @@ -1521,3 +1528,78 @@ def update_minion_pool(context, minion_pool_id, updated_values): # the oslo_db library uses this method for both the `created_at` and # `updated_at` fields setattr(lifecycle, 'updated_at', timeutils.utcnow()) + + +def process_sort_params( + sort_keys, + sort_dirs, + default_keys=None, + default_dir='desc', +): + """Process the sort parameters to include default keys. + + Creates a list of sort keys and a list of sort directions. Adds the default + keys to the end of the list if they are not already included. + + When adding the default keys to the sort keys list, the associated + direction is: + 1) The first element in the 'sort_dirs' list (if specified), else + 2) 'default_dir' value (Note that 'asc' is the default value since this is + the default in sqlalchemy.utils.paginate_query) + + :param sort_keys: List of sort keys to include in the processed list + :param sort_dirs: List of sort directions to include in the processed list + :param default_keys: List of sort keys that need to be included in the + processed list, they are added at the end of the list if not already + specified. + :param default_dir: Sort direction associated with each of the default + keys that are not supplied, used when they are added to the processed + list + :returns: list of sort keys, list of sort directions + :raise exception.InvalidInput: If more sort directions than sort keys + are specified or if an invalid sort direction is specified + """ + if default_keys is None: + default_keys = ['created_at', 'id'] + + # Determine direction to use for when adding default keys + if sort_dirs and len(sort_dirs): + default_dir_value = sort_dirs[0] + else: + default_dir_value = default_dir + + # Create list of keys (do not modify the input list) + if sort_keys: + result_keys = list(sort_keys) + else: + result_keys = [] + + # If a list of directions is not provided, use the default sort direction + # for all provided keys. + if sort_dirs: + result_dirs = [] + # Verify sort direction + for sort_dir in sort_dirs: + if sort_dir not in ('asc', 'desc'): + msg = (f"Unknown sort direction: {sort_dir}, " + "must be 'desc' or 'asc'.") + raise exception.InvalidInput(reason=msg) + result_dirs.append(sort_dir) + else: + result_dirs = [default_dir_value for _sort_key in result_keys] + + # Ensure that the key and direction length match + while len(result_dirs) < len(result_keys): + result_dirs.append(default_dir_value) + # Unless more direction are specified, which is an error + if len(result_dirs) > len(result_keys): + msg = "Sort direction array size exceeds sort key array size." + raise exception.InvalidInput(reason=msg) + + # Ensure defaults are included + for key in default_keys: + if key not in result_keys: + result_keys.append(key) + result_dirs.append(default_dir_value) + + return result_keys, result_dirs diff --git a/coriolis/tests/integration/test_pagination.py b/coriolis/tests/integration/test_pagination.py index 73301184..243ac975 100755 --- a/coriolis/tests/integration/test_pagination.py +++ b/coriolis/tests/integration/test_pagination.py @@ -139,3 +139,35 @@ def test_transfer_execution_list(self): self._get_record_summary(e) for e in sorted_exp_exec] self.assertEqual(exp_sorted_exec_summary, ret_exec_summary) + + def test_transfer_execution_list_pagination(self): + # Get the first 2 entries, sorted by ID in ascending order. + executions = self._client.transfer_executions.list( + self._transfers[0].id, + limit=2, + sort_keys=['id'], + sort_dirs=['asc']) + ret_exec_summary = [self._get_record_summary(e) for e in executions] + + exp_exec = self._executions[self._transfers[0].id] + sorted_exp_exec = sorted( + exp_exec, + key=operator.attrgetter('id')) + exp_sorted_exec_summary = [ + self._get_record_summary(e) for e in sorted_exp_exec][:2] + self.assertEqual(exp_sorted_exec_summary, ret_exec_summary) + + # Get the next 2 entries. + next_executions = self._client.transfer_executions.list( + self._transfers[0].id, + limit=2, + marker=executions[-1].id, + sort_keys=['id'], + sort_dirs=['asc']) + ret_exec_summary = [ + self._get_record_summary(e) + for e in next_executions] + + exp_sorted_exec_summary = [ + self._get_record_summary(e) for e in sorted_exp_exec][2:4] + self.assertEqual(exp_sorted_exec_summary, ret_exec_summary) \ No newline at end of file diff --git a/coriolis/transfer_tasks_executions/api.py b/coriolis/transfer_tasks_executions/api.py index ce71a083..7158f93e 100644 --- a/coriolis/transfer_tasks_executions/api.py +++ b/coriolis/transfer_tasks_executions/api.py @@ -21,9 +21,11 @@ def cancel(self, ctxt, transfer_id, execution_id, force): ctxt, transfer_id, execution_id, force) def get_executions(self, ctxt, transfer_id, include_tasks=False, - marker=None, limit=None): + marker=None, limit=None, + sort_keys=None, sort_dirs=None): return self._rpc_client.get_transfer_tasks_executions( - ctxt, transfer_id, include_tasks, marker, limit) + ctxt, transfer_id, include_tasks, marker, limit, + sort_keys, sort_dirs) def get_execution(self, ctxt, transfer_id, execution_id): return self._rpc_client.get_transfer_tasks_execution( From ec2c562120e3950193f244376908e32c14df2ba2 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Tue, 5 May 2026 08:35:09 +0000 Subject: [PATCH 04/10] Add deployment pagination --- coriolis/api/v1/deployments.py | 9 ++- coriolis/conductor/rpc/client.py | 11 ++- coriolis/conductor/rpc/server.py | 8 ++- coriolis/db/api.py | 26 ++++++- coriolis/deployments/api.py | 8 ++- coriolis/tests/integration/test_pagination.py | 70 ++++++++++++++++++- 6 files changed, 122 insertions(+), 10 deletions(-) diff --git a/coriolis/api/v1/deployments.py b/coriolis/api/v1/deployments.py index 14c0382a..2f62ddd7 100644 --- a/coriolis/api/v1/deployments.py +++ b/coriolis/api/v1/deployments.py @@ -4,6 +4,7 @@ from oslo_log import log as logging from webob import exc +from coriolis.api import common from coriolis.api.v1 import utils as api_utils from coriolis.api.v1.views import deployment_view from coriolis.api import wsgi as api_wsgi @@ -43,11 +44,17 @@ def _list(self, req): context.can(deployment_policies.get_deployments_policy_label("list")) include_task_info = api_utils.get_bool_url_arg( req, "include_task_info", default=False) + + marker, limit = common.get_paging_params(req) + sort_keys, sort_dirs = common.get_sort_params(req) + return deployment_view.collection( self._deployment_api.get_deployments( context, include_tasks=include_task_info, - include_task_info=include_task_info + include_task_info=include_task_info, + marker=marker, limit=limit, + sort_keys=sort_keys, sort_dirs=sort_dirs, )) def index(self, req): diff --git a/coriolis/conductor/rpc/client.py b/coriolis/conductor/rpc/client.py index d0066735..18e17abf 100644 --- a/coriolis/conductor/rpc/client.py +++ b/coriolis/conductor/rpc/client.py @@ -226,10 +226,17 @@ def delete_transfer_disks(self, ctxt, transfer_id): ctxt, 'delete_transfer_disks', transfer_id=transfer_id) def get_deployments(self, ctxt, include_tasks=False, - include_task_info=False): + include_task_info=False, + marker=None, limit=None, + sort_keys=None, sort_dirs=None): return self._call( ctxt, 'get_deployments', include_tasks=include_tasks, - include_task_info=include_task_info) + include_task_info=include_task_info, + marker=marker, + limit=limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs, + ) def get_deployment(self, ctxt, deployment_id, include_task_info=False): return self._call( diff --git a/coriolis/conductor/rpc/server.py b/coriolis/conductor/rpc/server.py index 2d1f4ea2..417350c4 100644 --- a/coriolis/conductor/rpc/server.py +++ b/coriolis/conductor/rpc/server.py @@ -1374,10 +1374,16 @@ def _get_transfer(self, ctxt, transfer_id, include_task_info=False, return transfer @staticmethod - def get_deployments(ctxt, include_tasks, include_task_info=False): + def get_deployments(ctxt, include_tasks, include_task_info=False, + marker=None, limit=None, + sort_keys=None, sort_dirs=None): return db_api.get_deployments( ctxt, include_tasks, include_task_info=include_task_info, + marker=marker, + limit=limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs, to_dict=True) @deployment_synchronized diff --git a/coriolis/db/api.py b/coriolis/db/api.py index cd7fc59d..035a9768 100644 --- a/coriolis/db/api.py +++ b/coriolis/db/api.py @@ -563,6 +563,10 @@ def get_transfer_deployments(context, transfer_id): def get_deployments(context, include_tasks=False, include_task_info=False, + marker=None, + limit=None, + sort_keys: list[str] | None = None, + sort_dirs: list[str] | None = None, to_dict=False): q = _soft_delete_aware_query(context, models.Deployment) if include_tasks: @@ -572,10 +576,26 @@ def get_deployments(context, if include_task_info: q = q.options(orm.undefer('info')) - args = {} if is_user_context(context): - args["project_id"] = context.project_id - result = q.filter_by(**args).all() + q = q.filter_by(project_id=context.project_id) + + sort_keys, sort_dirs = process_sort_params( + sort_keys, + sort_dirs, + ) + if marker: + try: + marker = get_deployment(context, marker) + except exception.NotFound: + raise exception.MarkerNotFound(marker=marker) + q = sqlalchemy_utils.paginate_query( + q, models.Deployment, limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs, + marker=marker, + ) + + result = q.all() if to_dict: return [i.to_dict( include_task_info=include_task_info, diff --git a/coriolis/deployments/api.py b/coriolis/deployments/api.py index 8cc21eec..695665fd 100644 --- a/coriolis/deployments/api.py +++ b/coriolis/deployments/api.py @@ -26,9 +26,13 @@ def cancel(self, ctxt, deployment_id, force): self._rpc_client.cancel_deployment(ctxt, deployment_id, force) def get_deployments(self, ctxt, include_tasks=False, - include_task_info=False): + include_task_info=False, + marker=None, limit=None, + sort_keys=None, sort_dirs=None): return self._rpc_client.get_deployments( - ctxt, include_tasks, include_task_info=include_task_info) + ctxt, include_tasks, include_task_info=include_task_info, + marker=marker, limit=limit, + sort_keys=sort_keys, sort_dirs=sort_dirs) def get_deployment(self, ctxt, deployment_id, include_task_info=False): return self._rpc_client.get_deployment( diff --git a/coriolis/tests/integration/test_pagination.py b/coriolis/tests/integration/test_pagination.py index 243ac975..dfb6df7a 100755 --- a/coriolis/tests/integration/test_pagination.py +++ b/coriolis/tests/integration/test_pagination.py @@ -23,6 +23,7 @@ class PaginationTest(base.CoriolisIntegrationTestBase): TRANSFER_COUNT = 5 EXECUTIONS_PER_TRANSFER = 5 + DEPLOYMENTS_PER_TRANSFER = 5 @classmethod def setUpClass(cls): @@ -84,6 +85,23 @@ def _create_endpoint( db_api.add_endpoint(cls._admin_ctx, endpoint) return endpoint + @classmethod + def _create_deployment( + cls, + transfer_id, + origin_endpoint_id: str, + destination_endpoint_id: str, + **kwargs, + ) -> models.Deployment: + kwargs["id"] = kwargs.get("id", str(uuid.uuid4())) + kwargs["transfer_id"] = transfer_id + kwargs["origin_endpoint_id"] = origin_endpoint_id + kwargs["destination_endpoint_id"] = destination_endpoint_id + deployment = models.Deployment( + **kwargs) + db_api.add_deployment(cls._admin_ctx, deployment) + return deployment + @classmethod def _setup_mocks(cls): cls._src_endpoint = cls._create_endpoint() @@ -91,6 +109,8 @@ def _setup_mocks(cls): cls._transfers = [] cls._executions = {} + cls._deployments = {} + cls._all_deployments = [] for transfer_idx in range(cls.TRANSFER_COUNT): # For testing purposes, we'll set the "created_at" field # explicitly, adding a small time delta between records. @@ -109,6 +129,17 @@ def _setup_mocks(cls): seconds=execution_idx)) cls._executions[transfer.id].append(execution) + cls._deployments[transfer.id] = [] + for deployment_idx in range(cls.DEPLOYMENTS_PER_TRANSFER): + deployment = cls._create_deployment( + transfer_id=transfer.id, + origin_endpoint_id=cls._src_endpoint.id, + destination_endpoint_id=cls._dst_endpoint.id, + created_at=timeutils.utcnow() + datetime.timedelta( + seconds=deployment_idx)) + cls._deployments[transfer.id].append(deployment) + cls._all_deployments.append(deployment) + @staticmethod def _get_record_summary(record): # Extract a few fields from the db records and entries returned by @@ -170,4 +201,41 @@ def test_transfer_execution_list_pagination(self): exp_sorted_exec_summary = [ self._get_record_summary(e) for e in sorted_exp_exec][2:4] - self.assertEqual(exp_sorted_exec_summary, ret_exec_summary) \ No newline at end of file + self.assertEqual(exp_sorted_exec_summary, ret_exec_summary) + + def test_deployment_list(self): + deployments = self._client.deployments.list() + ret_depl_summary = [self._get_record_summary(d) for d in deployments] + + exp_sorted_depl_summary = [ + self._get_record_summary(d) for d in self._all_deployments] + exp_sorted_depl_summary = sorted( + exp_sorted_depl_summary, + key=lambda x: (x["created_at"], x["id"]), + reverse=True) + self.assertEqual(exp_sorted_depl_summary, ret_depl_summary) + + def test_deployment_list_pagination(self): + # Get the first 2 entries, sorted by ID in ascending order. + deployments = self._client.deployments.list( + limit=2, + sort_keys=['id'], + sort_dirs=['asc']) + ret_depl_summary = [self._get_record_summary(d) for d in deployments] + + exp_sorted_depl_summary = [ + self._get_record_summary(d) for d in self._all_deployments] + exp_sorted_depl_summary = sorted( + exp_sorted_depl_summary, + key=lambda x: x["id"]) + self.assertEqual(exp_sorted_depl_summary[:2], ret_depl_summary) + + # Get the next 2 entries. + deployments = self._client.deployments.list( + limit=2, + sort_keys=['id'], + sort_dirs=['asc'], + marker=ret_depl_summary[-1]["id"], + ) + ret_depl_summary = [self._get_record_summary(d) for d in deployments] + self.assertEqual(exp_sorted_depl_summary[2:4], ret_depl_summary) From 713ef690617f31a69dc8fcb1c83383170cb0e885 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Tue, 5 May 2026 10:27:02 +0000 Subject: [PATCH 05/10] Add transfer pagination --- coriolis/api/v1/transfers.py | 8 ++++- coriolis/conductor/rpc/client.py | 11 ++++-- coriolis/conductor/rpc/server.py | 11 ++++-- coriolis/db/api.py | 21 +++++++++++ coriolis/tests/integration/test_pagination.py | 36 +++++++++++++++++++ coriolis/transfers/api.py | 8 +++-- 6 files changed, 88 insertions(+), 7 deletions(-) diff --git a/coriolis/api/v1/transfers.py b/coriolis/api/v1/transfers.py index 013ac0d4..92df1fde 100644 --- a/coriolis/api/v1/transfers.py +++ b/coriolis/api/v1/transfers.py @@ -1,6 +1,7 @@ # Copyright 2016 Cloudbase Solutions Srl # All Rights Reserved. +from coriolis.api import common from coriolis.api.v1 import utils as api_utils from coriolis.api.v1.views import transfer_tasks_execution_view from coriolis.api.v1.views import transfer_view @@ -49,11 +50,16 @@ def _list(self, req): context.can(transfer_policies.get_transfers_policy_label("list")) include_task_info = api_utils.get_bool_url_arg( req, "include_task_info", default=False) + marker, limit = common.get_paging_params(req) + sort_keys, sort_dirs = common.get_sort_params(req) return transfer_view.collection( self._transfer_api.get_transfers( context, include_tasks_executions=include_task_info, - include_task_info=include_task_info)) + include_task_info=include_task_info, + marker=marker, limit=limit, + sort_keys=sort_keys, sort_dirs=sort_dirs, + )) def index(self, req): return self._list(req) diff --git a/coriolis/conductor/rpc/client.py b/coriolis/conductor/rpc/client.py index 18e17abf..a927aab3 100644 --- a/coriolis/conductor/rpc/client.py +++ b/coriolis/conductor/rpc/client.py @@ -206,11 +206,18 @@ def create_instances_transfer(self, ctxt, skip_os_morphing=skip_os_morphing) def get_transfers(self, ctxt, include_tasks_executions=False, - include_task_info=False): + include_task_info=False, + marker=None, limit=None, + sort_keys=None, sort_dirs=None): return self._call( ctxt, 'get_transfers', include_tasks_executions=include_tasks_executions, - include_task_info=include_task_info) + include_task_info=include_task_info, + marker=marker, + limit=limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs, + ) def get_transfer(self, ctxt, transfer_id, include_task_info=False): return self._call( diff --git a/coriolis/conductor/rpc/server.py b/coriolis/conductor/rpc/server.py index 417350c4..98bec5e1 100644 --- a/coriolis/conductor/rpc/server.py +++ b/coriolis/conductor/rpc/server.py @@ -1215,10 +1215,17 @@ def _get_transfer_tasks_execution(ctxt, transfer_id, execution_id, @staticmethod def get_transfers(ctxt, include_tasks_executions=False, - include_task_info=False): + include_task_info=False, + marker=None, limit=None, + sort_keys=None, sort_dirs=None): return db_api.get_transfers( ctxt, include_tasks_executions=include_tasks_executions, - include_task_info=include_task_info, to_dict=True) + include_task_info=include_task_info, + marker=marker, + limit=limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs, + to_dict=True) @transfer_synchronized def get_transfer(self, ctxt, transfer_id, include_task_info=False): diff --git a/coriolis/db/api.py b/coriolis/db/api.py index 035a9768..52f674bb 100644 --- a/coriolis/db/api.py +++ b/coriolis/db/api.py @@ -454,6 +454,10 @@ def get_transfers(context, transfer_scenario=None, include_tasks_executions=False, include_task_info=False, + marker=None, + limit=None, + sort_keys: list[str] | None = None, + sort_dirs: list[str] | None = None, to_dict=False): q = _soft_delete_aware_query(context, models.Transfer) if include_tasks_executions: @@ -466,6 +470,23 @@ def get_transfers(context, if is_user_context(context): q = q.filter( models.Transfer.project_id == context.project_id) + + sort_keys, sort_dirs = process_sort_params( + sort_keys, + sort_dirs, + ) + if marker: + try: + marker = get_transfer(context, marker) + except exception.NotFound: + raise exception.MarkerNotFound(marker=marker) + q = sqlalchemy_utils.paginate_query( + q, models.Transfer, limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs, + marker=marker, + ) + db_result = q.all() if to_dict: return [ diff --git a/coriolis/tests/integration/test_pagination.py b/coriolis/tests/integration/test_pagination.py index dfb6df7a..8e294b1e 100755 --- a/coriolis/tests/integration/test_pagination.py +++ b/coriolis/tests/integration/test_pagination.py @@ -239,3 +239,39 @@ def test_deployment_list_pagination(self): ) ret_depl_summary = [self._get_record_summary(d) for d in deployments] self.assertEqual(exp_sorted_depl_summary[2:4], ret_depl_summary) + + def test_transfer_list(self): + transfers = self._client.transfers.list() + ret_transfer_summary = [self._get_record_summary(t) for t in transfers] + + exp_sorted_transfer_summary = [ + self._get_record_summary(d) for d in self._transfers] + exp_sorted_transfer_summary = sorted( + exp_sorted_transfer_summary, + key=lambda x: (x["created_at"], x["id"]), + reverse=True) + self.assertEqual(exp_sorted_transfer_summary, ret_transfer_summary) + + def test_transfer_list_pagination(self): + # Get the first 2 entries, sorted by ID in ascending order. + transfers = self._client.transfers.list( + limit=2, + sort_keys=['id'], + sort_dirs=['asc']) + ret_transfer_summary = [self._get_record_summary(t) for t in transfers] + + exp_sorted_transfer_summary = [ + self._get_record_summary(d) for d in self._transfers] + exp_sorted_transfer_summary = sorted( + exp_sorted_transfer_summary, + key=lambda x: x["id"]) + self.assertEqual(exp_sorted_transfer_summary[:2], ret_transfer_summary) + + # Get the next 2 entries. + transfers = self._client.transfers.list( + limit=2, + sort_keys=['id'], + sort_dirs=['asc'], + marker=transfers[-1].id) + ret_transfer_summary = [self._get_record_summary(t) for t in transfers] + self.assertEqual(exp_sorted_transfer_summary[2:4], ret_transfer_summary) diff --git a/coriolis/transfers/api.py b/coriolis/transfers/api.py index 9d56ed9c..31ff9623 100644 --- a/coriolis/transfers/api.py +++ b/coriolis/transfers/api.py @@ -32,10 +32,14 @@ def delete(self, ctxt, transfer_id): self._rpc_client.delete_transfer(ctxt, transfer_id) def get_transfers(self, ctxt, include_tasks_executions=False, - include_task_info=False): + include_task_info=False, + marker=None, limit=None, + sort_keys=None, sort_dirs=None): return self._rpc_client.get_transfers( ctxt, include_tasks_executions, - include_task_info=include_task_info) + include_task_info=include_task_info, + marker=marker, limit=limit, + sort_keys=sort_keys, sort_dirs=sort_dirs) def get_transfer(self, ctxt, transfer_id, include_task_info=False): return self._rpc_client.get_transfer( From 7fb3241bac88e75c1d1dfe1ee0a16b16442e4872 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Tue, 5 May 2026 10:52:17 +0000 Subject: [PATCH 06/10] Add an example --- README.rst | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/README.rst b/README.rst index 89a0b66c..fc974fb2 100644 --- a/README.rst +++ b/README.rst @@ -148,6 +148,21 @@ Delete a migration job: Note: only completed, failed or cancelled jobs can be deleted. +The following Coriolis APIs support pagination: +* transfers +* transfer executions +* deployments +* endpoint instances (only marker and limit parameters) + +Pagination parameters: +* ``sort_key`` - sort key, repeatable. `created_at` and `id` are used by default. +* ``sort_dir`` - sort direction, repeatable. `asc` or `desc` (default). +* ``marker`` - the last seen ID, omitted from the results. +* ``limit`` - the maximum number of records to retrieve. + +Example: + + GET http://server:7667/v1/transfers?marker=a7061715-e56c-470c-a6ac-80bb02f1f198&limit=2&sort_key=id&sort_dir=asc API Documentation ----------------- From 14be34ef9c4128e8fd8f5a38d3f4b106e44ccdf1 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Tue, 5 May 2026 11:56:43 +0000 Subject: [PATCH 07/10] pagination test: avoid admin context Non-admin operations will be tenant scoped, which means that we don't have to care about other entries from the database. --- coriolis/tests/integration/test_pagination.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/coriolis/tests/integration/test_pagination.py b/coriolis/tests/integration/test_pagination.py index 8e294b1e..8a7684c2 100755 --- a/coriolis/tests/integration/test_pagination.py +++ b/coriolis/tests/integration/test_pagination.py @@ -29,9 +29,9 @@ class PaginationTest(base.CoriolisIntegrationTestBase): def setUpClass(cls): super().setUpClass() - cls._admin_ctx = coriolis_context.get_admin_context() - cls._admin_ctx.user_id = cls.FAKE_USER_ID - cls._admin_ctx.project_id = cls.FAKE_PROJECT_ID + cls._ctx = coriolis_context.RequestContext( + user=cls.FAKE_USER_ID, + project_id=cls.FAKE_PROJECT_ID) cls._setup_mocks() @@ -49,7 +49,7 @@ def _create_transfer( kwargs["info"] = {instance: { 'volumes_info': []} for instance in kwargs["instances"]} transfer = models.Transfer(**kwargs) - db_api.add_transfer(cls._admin_ctx, transfer) + db_api.add_transfer(cls._ctx, transfer) return transfer @classmethod @@ -69,7 +69,7 @@ def _create_execution( # "add_transfer_tasks_execution" expects "action" to be set, # despite not being declared by the model. execution.action = transfer - db_api.add_transfer_tasks_execution(cls._admin_ctx, execution) + db_api.add_transfer_tasks_execution(cls._ctx, execution) return execution @classmethod @@ -82,7 +82,7 @@ def _create_endpoint( kwargs["type"] = kwargs.get("type", "openstack") endpoint = models.Endpoint( **kwargs) - db_api.add_endpoint(cls._admin_ctx, endpoint) + db_api.add_endpoint(cls._ctx, endpoint) return endpoint @classmethod @@ -99,7 +99,7 @@ def _create_deployment( kwargs["destination_endpoint_id"] = destination_endpoint_id deployment = models.Deployment( **kwargs) - db_api.add_deployment(cls._admin_ctx, deployment) + db_api.add_deployment(cls._ctx, deployment) return deployment @classmethod From 93edfa7641b0a81ed65a64998e4bfada0ac46f1a Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Tue, 5 May 2026 12:09:02 +0000 Subject: [PATCH 08/10] Fix pep8 errors --- coriolis/api/v1/transfer_tasks_executions.py | 2 +- coriolis/exception.py | 1 + coriolis/tests/integration/test_pagination.py | 11 ++++++----- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/coriolis/api/v1/transfer_tasks_executions.py b/coriolis/api/v1/transfer_tasks_executions.py index 786e06e0..6fcb5eef 100644 --- a/coriolis/api/v1/transfer_tasks_executions.py +++ b/coriolis/api/v1/transfer_tasks_executions.py @@ -1,8 +1,8 @@ # Copyright 2016 Cloudbase Solutions Srl # All Rights Reserved. -from coriolis.api.v1.views import transfer_tasks_execution_view from coriolis.api import common +from coriolis.api.v1.views import transfer_tasks_execution_view from coriolis.api import wsgi as api_wsgi from coriolis import exception from coriolis.policies import transfer_tasks_executions as executions_policies diff --git a/coriolis/exception.py b/coriolis/exception.py index 7de82395..73f38488 100644 --- a/coriolis/exception.py +++ b/coriolis/exception.py @@ -294,6 +294,7 @@ class MarkerNotFound(NotFound): "Could not find database record " "identified by marker: %(marker)s") + class RegionNotFound(NotFound): message = _("The specified Coriolis region(s) could not be found.") diff --git a/coriolis/tests/integration/test_pagination.py b/coriolis/tests/integration/test_pagination.py index 8a7684c2..970c38d0 100755 --- a/coriolis/tests/integration/test_pagination.py +++ b/coriolis/tests/integration/test_pagination.py @@ -16,7 +16,6 @@ from coriolis.tests.integration import base - class PaginationTest(base.CoriolisIntegrationTestBase): FAKE_USER_ID = "fake-user-id" FAKE_PROJECT_ID = "fake-project-id" @@ -77,8 +76,9 @@ def _create_endpoint( cls, **kwargs, ) -> models.Endpoint: - kwargs["id"] = kwargs.get("id", str(uuid.uuid4())) - kwargs["name"] = kwargs.get("name", f"test-endpoint-{kwargs["id"]}") + endpoint_id = kwargs.get("id", str(uuid.uuid4())) + kwargs["id"] = endpoint_id + kwargs["name"] = kwargs.get("name", f"test-endpoint-{endpoint_id}") kwargs["type"] = kwargs.get("type", "openstack") endpoint = models.Endpoint( **kwargs) @@ -147,7 +147,7 @@ def _get_record_summary(record): # *all* fields, just the ones that are relevant for pagination. created_at = record.created_at if isinstance(created_at, str): - created_at = datetime.datetime.fromisoformat(created_at) + created_at = datetime.datetime.fromisoformat(created_at) # The service may not have microsecond level precision # and we need to compare records. created_at = created_at.replace(microsecond=0) @@ -274,4 +274,5 @@ def test_transfer_list_pagination(self): sort_dirs=['asc'], marker=transfers[-1].id) ret_transfer_summary = [self._get_record_summary(t) for t in transfers] - self.assertEqual(exp_sorted_transfer_summary[2:4], ret_transfer_summary) + self.assertEqual( + exp_sorted_transfer_summary[2:4], ret_transfer_summary) From 2e89253eafc66f3dd41e5d2117973f62cb331417 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Tue, 5 May 2026 12:45:35 +0000 Subject: [PATCH 09/10] Update unit tests, covering pagination --- coriolis/api/common.py | 20 +++--- coriolis/tests/api/test_common.py | 43 ++++++++++++ .../api/v1/test_transfer_tasks_executions.py | 20 +++++- coriolis/tests/api/v1/test_transfers.py | 17 ++++- coriolis/tests/conductor/rpc/test_client.py | 8 ++- coriolis/tests/conductor/rpc/test_server.py | 15 ++-- coriolis/tests/db/test_api.py | 68 +++++++++++++++++++ .../transfer_tasks_executions/test_api.py | 20 ++++-- coriolis/tests/transfers/test_api.py | 11 ++- 9 files changed, 197 insertions(+), 25 deletions(-) create mode 100644 coriolis/tests/api/test_common.py diff --git a/coriolis/api/common.py b/coriolis/api/common.py index 8ce0226e..5582fea2 100644 --- a/coriolis/api/common.py +++ b/coriolis/api/common.py @@ -13,8 +13,8 @@ def get_paging_params(req): def get_sort_params(req, - default_key='created_at', - default_dir='desc'): + default_keys=('created_at', 'id'), + default_dirs=('desc', 'desc')): """Retrieves sort keys/directions parameters. Processes the parameters to create a list of sort keys and sort directions @@ -25,10 +25,10 @@ def get_sort_params(req, The input parameters are not modified. :param req: coriolis.api.wsgi.Request object - :param default_key: default sort key value, added to the list if no - 'sort_key' parameters are supplied - :param default_dir: default sort dir value, added to the list if no - 'sort_dir' parameters are supplied + :param default_keys: default sort key values, added to the list if no + 'sort_key' parameters are supplied + :param default_dirs: default sort dir values, added to the list if no + 'sort_dir' parameters are supplied :returns: list of sort keys, list of sort dirs """ params = req.params.copy() @@ -38,8 +38,8 @@ def get_sort_params(req, sort_keys.append(params.pop('sort_key').strip()) while 'sort_dir' in params: sort_dirs.append(params.pop('sort_dir').strip()) - if len(sort_keys) == 0 and default_key: - sort_keys.append(default_key) - if len(sort_dirs) == 0 and default_dir: - sort_dirs.append(default_dir) + if len(sort_keys) == 0 and default_keys: + sort_keys.extend(default_keys) + if len(sort_dirs) == 0 and default_dirs: + sort_dirs.extend(default_dirs) return sort_keys, sort_dirs diff --git a/coriolis/tests/api/test_common.py b/coriolis/tests/api/test_common.py new file mode 100644 index 00000000..1d4a46b7 --- /dev/null +++ b/coriolis/tests/api/test_common.py @@ -0,0 +1,43 @@ +# Copyright 2026 Cloudbase Solutions Srl +# All Rights Reserved. + +import webob + +from coriolis.api import common +from coriolis.tests import test_base + + +class ApiCommonTestCase(test_base.CoriolisBaseTestCase): + def test_get_paging_params(self): + req = webob.Request.blank('/some-path?marker=fake-marker&limit=10') + + marker, limit = common.get_paging_params(req) + + self.assertEqual("fake-marker", marker) + self.assertEqual(10, limit) + + def test_get_paging_params_unspecified(self): + req = webob.Request.blank('/some-path') + + marker, limit = common.get_paging_params(req) + + self.assertIsNone(marker) + self.assertIsNone(limit) + + def test_get_sort_params(self): + req = webob.Request.blank( + '/some-path?' + 'sort_key=key0&sort_dir=dir0&sort_key=key1&sort_dir=dir1') + + sort_keys, sort_dirs = common.get_sort_params(req) + + self.assertEqual(["key0", "key1"], sort_keys) + self.assertEqual(["dir0", "dir1"], sort_dirs) + + def test_get_sort_params_unspecified(self): + req = webob.Request.blank('/some-path?') + + sort_keys, sort_dirs = common.get_sort_params(req) + + self.assertEqual(["created_at", "id"], sort_keys) + self.assertEqual(["desc", "desc"], sort_dirs) diff --git a/coriolis/tests/api/v1/test_transfer_tasks_executions.py b/coriolis/tests/api/v1/test_transfer_tasks_executions.py index fd57a007..0bf6fabb 100644 --- a/coriolis/tests/api/v1/test_transfer_tasks_executions.py +++ b/coriolis/tests/api/v1/test_transfer_tasks_executions.py @@ -73,17 +73,28 @@ def test_show_not_found( mock_context, transfer_id, id) mock_single.assert_not_called() + @mock.patch("coriolis.api.common.get_paging_params") + @mock.patch("coriolis.api.common.get_sort_params") @mock.patch.object(transfer_tasks_execution_view, 'collection') @mock.patch.object(api.API, 'get_executions') def test_index( self, mock_get_executions, - mock_collection + mock_collection, + mock_get_sort_params, + mock_get_paging_params, ): mock_req = mock.Mock() mock_context = mock.Mock() mock_req.environ = {'coriolis.context': mock_context} transfer_id = mock.sentinel.transfer_id + mock_get_sort_params.return_value = ( + mock.sentinel.sort_keys, + mock.sentinel.sort_dirs) + mock_get_paging_params.return_value = ( + mock.sentinel.marker, + mock.sentinel.limit, + ) result = self.transfer_api.index(mock_req, transfer_id) @@ -95,7 +106,12 @@ def test_index( mock_context.can.assert_called_once_with( "migration:transfer_executions:list") mock_get_executions.assert_called_once_with( - mock_context, transfer_id, include_tasks=False) + mock_context, transfer_id, include_tasks=False, + marker=mock.sentinel.marker, + limit=mock.sentinel.limit, + sort_keys=mock.sentinel.sort_keys, + sort_dirs=mock.sentinel.sort_dirs, + ) mock_collection.assert_called_once_with( mock_get_executions.return_value) diff --git a/coriolis/tests/api/v1/test_transfers.py b/coriolis/tests/api/v1/test_transfers.py index 7cc50acb..25fdfea9 100644 --- a/coriolis/tests/api/v1/test_transfers.py +++ b/coriolis/tests/api/v1/test_transfers.py @@ -84,6 +84,8 @@ def test_show_no_transfer( include_task_info=mock_get_bool_url_arg.return_value) mock_single.assert_not_called() + @mock.patch("coriolis.api.common.get_paging_params") + @mock.patch("coriolis.api.common.get_sort_params") @mock.patch.object(transfer_view, 'collection') @mock.patch.object(api.API, 'get_transfers') @mock.patch.object(api_utils, 'get_bool_url_arg') @@ -92,10 +94,19 @@ def test_list( mock_get_bool_url_arg, mock_get_transfers, mock_collection, + mock_get_sort_params, + mock_get_paging_params, ): mock_req = mock.Mock() mock_context = mock.Mock() mock_req.environ = {'coriolis.context': mock_context} + mock_get_sort_params.return_value = ( + mock.sentinel.sort_keys, + mock.sentinel.sort_dirs) + mock_get_paging_params.return_value = ( + mock.sentinel.marker, + mock.sentinel.limit, + ) mock_get_bool_url_arg.side_effect = [False, False] @@ -116,7 +127,11 @@ def test_list( mock_get_transfers.assert_called_once_with( mock_context, include_tasks_executions=False, - include_task_info=False + include_task_info=False, + marker=mock.sentinel.marker, + limit=mock.sentinel.limit, + sort_keys=mock.sentinel.sort_keys, + sort_dirs=mock.sentinel.sort_dirs, ) mock_collection.assert_called_once_with( mock_get_transfers.return_value) diff --git a/coriolis/tests/conductor/rpc/test_client.py b/coriolis/tests/conductor/rpc/test_client.py index 2fc6084b..1b884128 100644 --- a/coriolis/tests/conductor/rpc/test_client.py +++ b/coriolis/tests/conductor/rpc/test_client.py @@ -37,6 +37,10 @@ def setUp(self): super(ConductorClientTestCase, self).setUp() self.client = client.ConductorClient() + self._mock_pagination_args = dict( + marker="mock_marker", limit=5, + sort_keys=["mock_column"], sort_dirs=["desc"]) + def test_create_endpoint(self): args = { "name": "mock_name", @@ -161,7 +165,8 @@ def test_execute_transfer_tasks(self): def test_get_transfer_tasks_executions(self): args = { "transfer_id": "mock_transfer_id", - "include_tasks": False + "include_tasks": False, + **self._mock_pagination_args, } self._test(self.client.get_transfer_tasks_executions, args) @@ -205,6 +210,7 @@ def test_get_transfers(self): args = { "include_tasks_executions": False, "include_task_info": False, + **self._mock_pagination_args, } self._test(self.client.get_transfers, args) diff --git a/coriolis/tests/conductor/rpc/test_server.py b/coriolis/tests/conductor/rpc/test_server.py index c37d6eef..096cd33a 100644 --- a/coriolis/tests/conductor/rpc/test_server.py +++ b/coriolis/tests/conductor/rpc/test_server.py @@ -37,6 +37,9 @@ def setUp(self): self.server = server.ConductorServerEndpoint() self._licensing_client = mock.Mock() self.server._licensing_client = self._licensing_client + self._mock_pagination_args = dict( + marker="mock_marker", limit=5, + sort_keys=["mock_column"], sort_dirs=["desc"]) @mock.patch.object( rpc_worker_client.WorkerClient, "from_service_definition" @@ -1447,7 +1450,8 @@ def test_get_transfer_tasks_executions( mock.sentinel.context, mock.sentinel.transfer_id, mock.sentinel.execution_id, - include_task_info=False + include_task_info=False, + **self._mock_pagination_args, ) self.assertEqual( @@ -1459,7 +1463,8 @@ def test_get_transfer_tasks_executions( mock.sentinel.transfer_id, mock.sentinel.execution_id, include_task_info=False, - to_dict=True + **self._mock_pagination_args, + to_dict=True, ) @mock.patch.object(db_api, "get_transfer_tasks_execution") @@ -1675,7 +1680,8 @@ def test_get_transfers(self, mock_get_transfers): result = self.server.get_transfers( mock.sentinel.context, include_tasks_executions=False, - include_task_info=False + include_task_info=False, + **self._mock_pagination_args, ) self.assertEqual( @@ -1686,7 +1692,8 @@ def test_get_transfers(self, mock_get_transfers): mock.sentinel.context, include_tasks_executions=False, include_task_info=False, - to_dict=True + to_dict=True, + **self._mock_pagination_args, ) @mock.patch.object(server.ConductorServerEndpoint, '_get_transfer') diff --git a/coriolis/tests/db/test_api.py b/coriolis/tests/db/test_api.py index 33e90c2d..91ae6d15 100644 --- a/coriolis/tests/db/test_api.py +++ b/coriolis/tests/db/test_api.py @@ -326,6 +326,74 @@ def test__soft_delete_aware_query_context_show_deleted(self): self.assertEqual(result.id, valid_endpoint.id) self.assertIsNotNone(result.deleted_at) +class DBAPISortParamsTestCase(BaseDBAPITestCase): + def test_invalid_sort_dirs(self): + self.assertRaises( + exception.InvalidInput, + api.process_sort_params, + sort_keys=["created_at", "id"], + sort_dirs=["asc", "descending"], + ) + + def test_too_many_sort_dirs(self): + self.assertRaises( + exception.InvalidInput, + api.process_sort_params, + sort_keys=["id"], + sort_dirs=["asc", "asc"], + ) + + def test_unmodified_input(self): + sort_keys = ["created_at", "id"] + sort_dirs = ["desc", "desc"] + + ret_keys, ret_dirs = api.process_sort_params( + sort_keys, sort_dirs) + + self.assertEqual(sort_keys, ret_keys) + self.assertEqual(sort_dirs, ret_dirs) + + def test_unmatched_input(self): + sort_keys = ["created_at", "id"] + sort_dirs = ["asc"] + exp_dirs = ["asc", "asc"] + + ret_keys, ret_dirs = api.process_sort_params( + sort_keys, sort_dirs) + + self.assertEqual(sort_keys, ret_keys) + self.assertEqual(exp_dirs, ret_dirs) + + def test_default_keys_appended(self): + sort_keys = ["created_at"] + sort_dirs = ["asc"] + exp_sort_keys = ["created_at", "id"] + exp_dirs = ["asc", "asc"] + + ret_keys, ret_dirs = api.process_sort_params( + sort_keys, sort_dirs, + default_keys=["id"], + default_dir="asc", + ) + + self.assertEqual(exp_sort_keys, ret_keys) + self.assertEqual(exp_dirs, ret_dirs) + + def test_default_keys_without_input(self): + sort_keys = None + sort_dirs = None + exp_sort_keys = ["id"] + exp_dirs = ["asc"] + + ret_keys, ret_dirs = api.process_sort_params( + sort_keys, sort_dirs, + default_keys=["id"], + default_dir="asc", + ) + + self.assertEqual(exp_sort_keys, ret_keys) + self.assertEqual(exp_dirs, ret_dirs) + class EndpointDBAPITestCase(BaseDBAPITestCase): diff --git a/coriolis/tests/transfer_tasks_executions/test_api.py b/coriolis/tests/transfer_tasks_executions/test_api.py index 116c774c..36192446 100644 --- a/coriolis/tests/transfer_tasks_executions/test_api.py +++ b/coriolis/tests/transfer_tasks_executions/test_api.py @@ -48,13 +48,23 @@ def test_cancel(self): self.ctxt, self.transfer_id, self.execution_id, force)) def test_get_executions(self): - include_tasks = mock.sentinel.include_tasks - - result = self.api.get_executions(self.ctxt, self.transfer_id, - include_tasks) + result = self.api.get_executions( + self.ctxt, self.transfer_id, + mock.sentinel.include_tasks, + mock.sentinel.marker, + mock.sentinel.limit, + mock.sentinel.sort_keys, + mock.sentinel.sort_dirs, + ) self.rpc_client.get_transfer_tasks_executions.assert_called_once_with( - self.ctxt, self.transfer_id, include_tasks) + self.ctxt, self.transfer_id, + mock.sentinel.include_tasks, + mock.sentinel.marker, + mock.sentinel.limit, + mock.sentinel.sort_keys, + mock.sentinel.sort_dirs, + ) self.assertEqual( result, self.rpc_client.get_transfer_tasks_executions.return_value) diff --git a/coriolis/tests/transfers/test_api.py b/coriolis/tests/transfers/test_api.py index 308d2b75..9992e9eb 100644 --- a/coriolis/tests/transfers/test_api.py +++ b/coriolis/tests/transfers/test_api.py @@ -17,6 +17,9 @@ def setUp(self): self.api._rpc_client = self.rpc_client self.ctxt = mock.sentinel.ctxt self.transfer_id = mock.sentinel.transfer_id + self._mock_pagination_args = dict( + marker="mock_marker", limit=5, + sort_keys=["mock_column"], sort_dirs=["desc"]) def test_create(self): origin_endpoint_id = mock.sentinel.origin_endpoint_id @@ -66,10 +69,14 @@ def test_delete(self): def test_get_transfers(self): result = self.api.get_transfers( - self.ctxt, include_tasks_executions=False, include_task_info=False) + self.ctxt, include_tasks_executions=False, include_task_info=False, + **self._mock_pagination_args, + ) self.rpc_client.get_transfers.assert_called_once_with( - self.ctxt, False, include_task_info=False) + self.ctxt, False, include_task_info=False, + **self._mock_pagination_args, + ) self.assertEqual(result, self.rpc_client.get_transfers.return_value) def test_get_transfer(self): From 49ef112e2bc2e8c9191c736b5659ac1705677fb3 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Wed, 6 May 2026 09:38:25 +0000 Subject: [PATCH 10/10] pagination test: perform cleanup --- coriolis/tests/db/test_api.py | 1 + coriolis/tests/integration/base.py | 5 ++- coriolis/tests/integration/test_pagination.py | 39 ++++++++++++++----- 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/coriolis/tests/db/test_api.py b/coriolis/tests/db/test_api.py index 91ae6d15..088010a8 100644 --- a/coriolis/tests/db/test_api.py +++ b/coriolis/tests/db/test_api.py @@ -326,6 +326,7 @@ def test__soft_delete_aware_query_context_show_deleted(self): self.assertEqual(result.id, valid_endpoint.id) self.assertIsNotNone(result.deleted_at) + class DBAPISortParamsTestCase(BaseDBAPITestCase): def test_invalid_sort_dirs(self): self.assertRaises( diff --git a/coriolis/tests/integration/base.py b/coriolis/tests/integration/base.py index dc5fee95..0fd65553 100644 --- a/coriolis/tests/integration/base.py +++ b/coriolis/tests/integration/base.py @@ -101,12 +101,13 @@ def _create_transfer(self, src_id, dst_id, instances): return transfer - def _ignoreExc(self, func): + @staticmethod + def _ignoreExc(func, ignored_exc=Exception): """Wrap the given function, ignoring exceptions.""" def f(*args, **kwargs): try: return func(*args, **kwargs) - except Exception as ex: + except ignored_exc as ex: LOG.warn("Exception encountered: %s", ex) return f diff --git a/coriolis/tests/integration/test_pagination.py b/coriolis/tests/integration/test_pagination.py index 970c38d0..fefa125a 100755 --- a/coriolis/tests/integration/test_pagination.py +++ b/coriolis/tests/integration/test_pagination.py @@ -13,6 +13,7 @@ from coriolis import context as coriolis_context from coriolis.db import api as db_api from coriolis.db.sqlalchemy import models +from coriolis import exception from coriolis.tests.integration import base @@ -31,17 +32,19 @@ def setUpClass(cls): cls._ctx = coriolis_context.RequestContext( user=cls.FAKE_USER_ID, project_id=cls.FAKE_PROJECT_ID) + cls._admin_ctx = coriolis_context.get_admin_context() cls._setup_mocks() @classmethod - def _create_transfer( + def _create_db_transfer( cls, origin_endpoint_id: str, destination_endpoint_id: str, instances: list[str] | None = None, **kwargs, ) -> models.Transfer: + kwargs["id"] = kwargs.get("id", str(uuid.uuid4())) kwargs["instances"] = instances or [] kwargs["origin_endpoint_id"] = origin_endpoint_id kwargs["destination_endpoint_id"] = destination_endpoint_id @@ -49,10 +52,13 @@ def _create_transfer( 'volumes_info': []} for instance in kwargs["instances"]} transfer = models.Transfer(**kwargs) db_api.add_transfer(cls._ctx, transfer) + cls.addClassCleanup( + cls._ignoreExc(db_api.delete_transfer, exception.NotFound), + cls._admin_ctx, transfer.id) return transfer @classmethod - def _create_execution( + def _create_db_execution( cls, transfer: models.Transfer, **kwargs, @@ -68,11 +74,15 @@ def _create_execution( # "add_transfer_tasks_execution" expects "action" to be set, # despite not being declared by the model. execution.action = transfer - db_api.add_transfer_tasks_execution(cls._ctx, execution) + db_api.add_transfer_tasks_execution(cls._admin_ctx, execution) + cls.addClassCleanup( + cls._ignoreExc(db_api.delete_transfer_tasks_execution, + exception.NotFound), + cls._admin_ctx, execution.id) return execution @classmethod - def _create_endpoint( + def _create_db_endpoint( cls, **kwargs, ) -> models.Endpoint: @@ -83,10 +93,13 @@ def _create_endpoint( endpoint = models.Endpoint( **kwargs) db_api.add_endpoint(cls._ctx, endpoint) + cls.addClassCleanup( + cls._ignoreExc(db_api.delete_endpoint, exception.NotFound), + cls._admin_ctx, endpoint.id) return endpoint @classmethod - def _create_deployment( + def _create_db_deployment( cls, transfer_id, origin_endpoint_id: str, @@ -100,12 +113,18 @@ def _create_deployment( deployment = models.Deployment( **kwargs) db_api.add_deployment(cls._ctx, deployment) + cls.addClassCleanup( + cls._ignoreExc(db_api.delete_deployment, exception.NotFound), + cls._admin_ctx, deployment.id) return deployment @classmethod def _setup_mocks(cls): - cls._src_endpoint = cls._create_endpoint() - cls._dst_endpoint = cls._create_endpoint() + # Note that we're using an admin context when performing cleanups. + # In case of already deleted records we'll get a "NotFound" error + # instead of "NotAuthorized". + cls._src_endpoint = cls._create_db_endpoint() + cls._dst_endpoint = cls._create_db_endpoint() cls._transfers = [] cls._executions = {} @@ -114,7 +133,7 @@ def _setup_mocks(cls): for transfer_idx in range(cls.TRANSFER_COUNT): # For testing purposes, we'll set the "created_at" field # explicitly, adding a small time delta between records. - transfer = cls._create_transfer( + transfer = cls._create_db_transfer( origin_endpoint_id=cls._src_endpoint.id, destination_endpoint_id=cls._dst_endpoint.id, created_at=timeutils.utcnow() + datetime.timedelta( @@ -123,7 +142,7 @@ def _setup_mocks(cls): cls._executions[transfer.id] = [] for execution_idx in range(cls.EXECUTIONS_PER_TRANSFER): - execution = cls._create_execution( + execution = cls._create_db_execution( transfer=transfer, created_at=timeutils.utcnow() + datetime.timedelta( seconds=execution_idx)) @@ -131,7 +150,7 @@ def _setup_mocks(cls): cls._deployments[transfer.id] = [] for deployment_idx in range(cls.DEPLOYMENTS_PER_TRANSFER): - deployment = cls._create_deployment( + deployment = cls._create_db_deployment( transfer_id=transfer.id, origin_endpoint_id=cls._src_endpoint.id, destination_endpoint_id=cls._dst_endpoint.id,