diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 8ac69557..46331c98 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -30,6 +30,8 @@ Added - Missing assay plugin ``__init__.py`` files (#2014) - Study plugin override via ISA-Tab comments (#1885) - Token auth support in study plugin IGV XML serving views (#1999, #2021) +- **Taskflowbackend** + - ``TaskflowAPI.raise_submit_api_exception()`` helper (#1847) Changed ------- @@ -52,16 +54,20 @@ Changed - **Landingzones** - Update REST API versioning (#1936) - Update REST API views for OpenAPI compatibility (#1951) + - Return ``503`` in ``ZoneSubmitMoveAPIView`` if project is locked (#1847) - **Samplesheets** - Update REST API versioning (#1936) - Update REST API views for OpenAPI compatibility (#1951) - Send iRODS delete request emails to all addresses of user (#2000) - Disable ontology term select box while querying (#1974) - Refactor ``SampleSheetAssayPluginPoint.get_assay_path()`` (#2016) + - Return ``503`` in ``IrodsCollsCreateAPIView`` if project is locked (#1847) + - Return ``503`` in ``IrodsDataRequestAcceptAPIView`` if project is locked (#1847) - **Taskflowbackend** - Refactor task tests (#2002) - Unify user name parameter naming in flows (#1653) - Refactor ``landing_zone_move`` flow (#1846) + - Move ``lock_project()`` into ``TaskflowTestMixin`` (#1847) Fixed ----- diff --git a/docs_manual/source/sodar_release_notes.rst b/docs_manual/source/sodar_release_notes.rst index 85194953..70a20ea6 100644 --- a/docs_manual/source/sodar_release_notes.rst +++ b/docs_manual/source/sodar_release_notes.rst @@ -22,6 +22,7 @@ Release for SODAR Core v1.0 upgrade, iRODS v4.3 upgrade and feature updates. - Update minimum supported iRODS version to v4.3.3 - Update REST API versioning - Update REST API views for OpenAPI support +- Update lock requiring REST API views to return 503 if project is locked - Upgrade to Postgres v16 - Upgrade to python-irodsclient v2.2.0 - Upgrade to SODAR Core v1.0.2 @@ -63,6 +64,18 @@ REST API Versioning Changes upgrading to this version due to the changed database requirements. Also it is strongly recommended to go through all documentation before upgrading. +REST API Updates +---------------- + +- Sample Sheets API + * ``IrodsCollsCreateAPIView`` + + Return ``503`` if project is locked + * ``IrodsDataRequestAcceptAPIView`` + + Return ``503`` if project is locked +- Landing Zones API + * ``ZoneSubmitMoveAPIView`` + + Return ``503`` if project is locked + v0.15.1 (2024-09-12) ==================== diff --git a/landingzones/tests/test_views_api_taskflow.py b/landingzones/tests/test_views_api_taskflow.py index 95091004..180a032c 100644 --- a/landingzones/tests/test_views_api_taskflow.py +++ b/landingzones/tests/test_views_api_taskflow.py @@ -448,6 +448,15 @@ def test_post_validate(self): 'Successfully validated 0 files', ) + def test_post_validate_locked(self): + """Test POST for validation with locked project (should fail)""" + self.lock_project(self.project) + self.landing_zone.status = ZONE_STATUS_FAILED + self.landing_zone.save() + response = self.request_knox(self.url, method='POST') + # NOTE: This should be updated to not require lock, see #1850 + self.assertEqual(response.status_code, 503) + def test_post_validate_invalid_status(self): """Test POST for validation with invalid zone status (should fail)""" self.landing_zone.status = ZONE_STATUS_MOVED @@ -486,6 +495,15 @@ def test_post_move_invalid_status(self): LandingZone.objects.first().status, ZONE_STATUS_DELETED ) + def test_post_move_locked(self): + """Test POST for moving with locked project (should fail)""" + self.lock_project(self.project) + response = self.request_knox(self.url_move, method='POST') + self.assertEqual(response.status_code, 503) + self.assertEqual(LandingZone.objects.count(), 1) + zone = LandingZone.objects.first() + self.assert_zone_status(zone, ZONE_STATUS_FAILED) + @override_settings(REDIS_URL=INVALID_REDIS_URL) def test_post_move_lock_failure(self): """Test POST for moving with project lock failure""" diff --git a/landingzones/views_api.py b/landingzones/views_api.py index f485a19f..4464469f 100644 --- a/landingzones/views_api.py +++ b/landingzones/views_api.py @@ -384,6 +384,8 @@ class ZoneSubmitMoveAPIView(ZoneMoveMixin, ZoneSubmitBaseAPIView): For validating data without moving it to the sample repository, this view should be called with ``submit/validate``. + Returns ``503`` if the project is currently locked by another operation. + **URL for Validation:** ``/landingzones/api/submit/validate/{LandingZone.sodar_uuid}`` **URL for Moving:** ``/landingzones/api/submit/move/{LandingZone.sodar_uuid}`` @@ -402,6 +404,7 @@ def get_operation_id_base(self, path, method, action): def post(self, request, *args, **kwargs): """POST request for initiating landing zone validation/moving""" + taskflow = get_backend_api('taskflow') zone = LandingZone.objects.filter( sodar_uuid=self.kwargs['landingzone'] ).first() @@ -423,9 +426,10 @@ def post(self, request, *args, **kwargs): try: self._submit_validate_move(zone, validate_only) except Exception as ex: - raise APIException( - 'Initiating landing zone {} failed: {}'.format(action_msg, ex) - ) + ex_msg = 'Initiating landing zone {} failed: '.format(action_msg) + if taskflow: + taskflow.raise_submit_api_exception(ex_msg, ex) + raise APIException('{}{}'.format(ex_msg, ex)) return Response( { 'detail': 'Landing zone {} initiated'.format(action_msg), diff --git a/samplesheets/tests/test_views_api_taskflow.py b/samplesheets/tests/test_views_api_taskflow.py index baa165be..90bcff9d 100644 --- a/samplesheets/tests/test_views_api_taskflow.py +++ b/samplesheets/tests/test_views_api_taskflow.py @@ -311,14 +311,17 @@ def test_get(self): class TestIrodsCollsCreateAPIView(SampleSheetAPITaskflowTestBase): """Tests for IrodsCollsCreateAPIView""" - def test_post(self): - """Test IrodsCollsCreateAPIView POST""" - self.assertEqual(self.investigation.irods_status, False) - url = reverse( + def setUp(self): + super().setUp() + self.url = reverse( 'samplesheets:api_irods_colls_create', kwargs={'project': self.project.sodar_uuid}, ) - response = self.request_knox(url, method='POST') + + def test_post(self): + """Test IrodsCollsCreateAPIView POST""" + self.assertEqual(self.investigation.irods_status, False) + response = self.request_knox(self.url, method='POST') self.assertEqual(response.status_code, 200) self.investigation.refresh_from_db() self.assertEqual(self.investigation.irods_status, True) @@ -331,13 +334,18 @@ def test_post_created(self): # Set up iRODS collections self.make_irods_colls(self.investigation) self.assertEqual(self.investigation.irods_status, True) - url = reverse( - 'samplesheets:api_irods_colls_create', - kwargs={'project': self.project.sodar_uuid}, - ) - response = self.request_knox(url, method='POST') + response = self.request_knox(self.url, method='POST') self.assertEqual(response.status_code, 400) + def test_post_locked(self): + """Test POST with locked project (should fail)""" + self.lock_project(self.project) + self.assertEqual(self.investigation.irods_status, False) + response = self.request_knox(self.url, method='POST') + self.assertEqual(response.status_code, 503) + self.investigation.refresh_from_db() + self.assertEqual(self.investigation.irods_status, False) + # NOTE: For TestIrodsAccessTicketListAPIView, see test_views_api # NOTE: For TestIrodsAccessTicketRetrieveAPIView, see test_views_api @@ -1034,9 +1042,22 @@ def test_accept_contributor(self): self.assert_alert_count(ACCEPT_ALERT, self.user_delegate, 0) self.assert_alert_count(ACCEPT_ALERT, self.user_contributor, 0) + def test_accept_locked(self): + """Test POST to accept request with locked project (should fail)""" + self.lock_project(self.project) + self.assert_irods_obj(self.obj_path) + response = self.request_knox(self.url, 'POST') + self.assertEqual(response.status_code, 503) + self.request.refresh_from_db() + self.assertEqual(self.request.status, IRODS_REQUEST_STATUS_FAILED) + self.assert_irods_obj(self.obj_path) + self.assert_alert_count(ACCEPT_ALERT, self.user, 0) + self.assert_alert_count(ACCEPT_ALERT, self.user_delegate, 0) + self.assert_alert_count(ACCEPT_ALERT, self.user_contributor, 0) + @override_settings(REDIS_URL=INVALID_REDIS_URL) def test_accept_lock_failure(self): - """Test POST toa ccept request with project lock failure""" + """Test POST to accept request with project lock failure""" self.assert_irods_obj(self.obj_path) response = self.request_knox(self.url, 'POST') self.assertEqual(response.status_code, 400) diff --git a/samplesheets/views_api.py b/samplesheets/views_api.py index d0115329..81265e55 100644 --- a/samplesheets/views_api.py +++ b/samplesheets/views_api.py @@ -153,6 +153,8 @@ class IrodsCollsCreateAPIView( """ Create iRODS collections for a project. + Returns ``503`` if the project is currently locked by another operation. + **URL:** ``/samplesheets/api/irods/collections/create/{Project.sodar_uuid}`` **Methods:** ``POST`` @@ -168,6 +170,7 @@ class IrodsCollsCreateAPIView( def post(self, request, *args, **kwargs): """POST request for creating iRODS collections""" irods_backend = get_backend_api('omics_irods') + taskflow = get_backend_api('taskflow') ex_msg = 'Creating iRODS collections failed: ' investigation = Investigation.objects.filter( project__sodar_uuid=self.kwargs.get('project'), active=True @@ -183,6 +186,8 @@ def post(self, request, *args, **kwargs): try: self.create_colls(investigation, request) except Exception as ex: + if taskflow: + taskflow.raise_submit_api_exception(ex_msg, ex) raise APIException('{}{}'.format(ex_msg, ex)) return Response( { @@ -750,7 +755,9 @@ class IrodsDataRequestAcceptAPIView( Accept an iRODS data request for a project. Accepting will delete the iRODS collection or data object targeted by the - request. This action can not be undone. + request. This action can not be undone. + + Returns ``503`` if the project is currently locked by another operation. **URL:** ``/samplesheets/api/irods/request/accept/{IrodsDataRequest.sodar_uuid}`` @@ -780,9 +787,10 @@ def post(self, request, *args, **kwargs): app_alerts=app_alerts, ) except Exception as ex: - raise ValidationError( - '{} {}'.format('Accepting ' + IRODS_REQUEST_EX_MSG + ':', ex) - ) + ex_msg = 'Accepting ' + IRODS_REQUEST_EX_MSG + ': ' + if taskflow: + taskflow.raise_submit_api_exception(ex_msg, ex, ValidationError) + raise ValidationError('{}{}'.format(ex_msg, ex)) return Response( {'detail': 'iRODS data request accepted'}, status=status.HTTP_200_OK ) diff --git a/taskflowbackend/api.py b/taskflowbackend/api.py index 89a4d0b3..74f0586e 100644 --- a/taskflowbackend/api.py +++ b/taskflowbackend/api.py @@ -3,6 +3,8 @@ import json import logging +from rest_framework.exceptions import APIException + # Landingzones dependency from landingzones.constants import ( ZONE_STATUS_NOT_CREATED, @@ -17,7 +19,7 @@ from projectroles.plugins import get_backend_api from taskflowbackend import flows -from taskflowbackend.lock_api import ProjectLockAPI +from taskflowbackend.lock_api import ProjectLockAPI, PROJECT_LOCKED_MSG from taskflowbackend.tasks_celery import submit_flow_task @@ -39,6 +41,9 @@ class TaskflowAPI: class FlowSubmitException(Exception): """SODAR Taskflow submission exception""" + #: Exception message for locked project + project_locked_msg = PROJECT_LOCKED_MSG + @classmethod def _raise_flow_exception(cls, ex_msg, tl_event=None, zone=None): """ @@ -87,6 +92,27 @@ def _raise_lock_exception(cls, ex_msg, tl_event=None, zone=None): lock_zone, ) + # HACK for returning 503 if project is locked (see #1505, #1847) + @classmethod + def raise_submit_api_exception( + cls, msg_prefix, ex, default_class=APIException + ): + """ + Raise zone submit API exception. Selects appropriate API response based + on exception type. + + :param msg_prefix: API response prefix + :param ex: Exception object + :param default_class: Default API exception class to be returned + :raises: Exception of varying type + """ + msg = '{}{}'.format(msg_prefix, ex) + if PROJECT_LOCKED_MSG in msg: + ex = APIException(msg) + ex.status_code = 503 + raise ex + raise default_class(msg) + @classmethod def get_flow( cls, diff --git a/taskflowbackend/lock_api.py b/taskflowbackend/lock_api.py index abc7545f..725a19d1 100644 --- a/taskflowbackend/lock_api.py +++ b/taskflowbackend/lock_api.py @@ -12,6 +12,7 @@ LOCK_ENABLED = settings.TASKFLOW_LOCK_ENABLED LOCK_RETRY_COUNT = settings.TASKFLOW_LOCK_RETRY_COUNT LOCK_RETRY_INTERVAL = settings.TASKFLOW_LOCK_RETRY_INTERVAL +PROJECT_LOCKED_MSG = 'Project is locked by another operation' logger = logging.getLogger('__name__') @@ -79,7 +80,7 @@ def acquire( return True time.sleep(retry_interval) cls._log_status(lock, unlock=False, failed=True) - raise LockAcquireException('Project is locked by another operation') + raise LockAcquireException(PROJECT_LOCKED_MSG) @classmethod def release(cls, lock): diff --git a/taskflowbackend/tests/base.py b/taskflowbackend/tests/base.py index f795fd77..34bc60cd 100644 --- a/taskflowbackend/tests/base.py +++ b/taskflowbackend/tests/base.py @@ -45,8 +45,11 @@ PROJECTROLES_API_DEFAULT_VERSION, ) +from taskflowbackend.lock_api import ProjectLockAPI + app_settings = AppSettingAPI() +lock_api = ProjectLockAPI() logger = logging.getLogger(__name__) @@ -73,11 +76,19 @@ class TaskflowTestMixin(ProjectMixin, RoleMixin, RoleAssignmentMixin): """Setup/teardown methods and helpers for taskflow tests""" + #: Project lock coordinator + coordinator = None #: iRODS backend object irods_backend = None #: iRODS session object irods = None + def lock_project(self, project): + self.coordinator = lock_api.get_coordinator() + lock_id = str(project.sodar_uuid) + lock = self.coordinator.get_lock(lock_id) + lock_api.acquire(lock) + def make_irods_object( self, coll, obj_name, content=None, content_length=1024, checksum=True ): diff --git a/taskflowbackend/tests/test_flows.py b/taskflowbackend/tests/test_flows.py index 5710f2c9..1123bf0f 100644 --- a/taskflowbackend/tests/test_flows.py +++ b/taskflowbackend/tests/test_flows.py @@ -59,7 +59,6 @@ PUBLIC_GROUP, ) from taskflowbackend.flows.sheet_delete import Flow as SheetDeleteFlow -from taskflowbackend.lock_api import ProjectLockAPI from taskflowbackend.tasks.irods_tasks import META_EMPTY_VALUE from taskflowbackend.tests.base import ( TaskflowViewTestBase, @@ -68,9 +67,6 @@ ) -lock_api = ProjectLockAPI() - - # SODAR constants PROJECT_TYPE_CATEGORY = SODAR_CONSTANTS['PROJECT_TYPE_CATEGORY'] PROJECT_TYPE_PROJECT = SODAR_CONSTANTS['PROJECT_TYPE_PROJECT'] @@ -96,12 +92,6 @@ def build_and_run(self, flow, force_fail=False): flow.build(force_fail) flow.run() - def lock_project(self, project): - self.coordinator = lock_api.get_coordinator() - lock_id = str(project.sodar_uuid) - lock = self.coordinator.get_lock(lock_id) - lock_api.acquire(lock) - class TestDataDelete(TaskflowbackendFlowTestBase): """Tests for the data_delete flow"""