From df1719117ec471757c7558d64bbf39983cd7f095 Mon Sep 17 00:00:00 2001 From: Michael Collins <15347726+michaeljcollinsuk@users.noreply.github.com> Date: Mon, 18 Sep 2023 14:03:37 +0100 Subject: [PATCH 1/8] ANPL-1704 display task status messages sent via django_channels when updating access --- controlpanel/api/models/policys3bucket.py | 4 ++++ controlpanel/frontend/jinja2/datasource-detail.html | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/controlpanel/api/models/policys3bucket.py b/controlpanel/api/models/policys3bucket.py index 925ea5a9f..6781f279e 100644 --- a/controlpanel/api/models/policys3bucket.py +++ b/controlpanel/api/models/policys3bucket.py @@ -21,6 +21,10 @@ class Meta: unique_together = ("policy", "s3bucket") ordering = ("id",) + def __init__(self, *args, **kwargs): + self.current_user = kwargs.pop("current_user", None) + super().__init__(*args, **kwargs) + def grant_bucket_access(self): if self.s3bucket.is_folder: return cluster.RoleGroup(self.policy).grant_folder_access( diff --git a/controlpanel/frontend/jinja2/datasource-detail.html b/controlpanel/frontend/jinja2/datasource-detail.html index 82932d011..0c133010a 100644 --- a/controlpanel/frontend/jinja2/datasource-detail.html +++ b/controlpanel/frontend/jinja2/datasource-detail.html @@ -25,7 +25,7 @@

{{ page_title }}

-
+

Users and groups with access

From d54c598f373d2f9b182a631d8a96387564a2fbe4 Mon Sep 17 00:00:00 2001 From: Michael Collins <15347726+michaeljcollinsuk@users.noreply.github.com> Date: Wed, 20 Sep 2023 16:36:30 +0100 Subject: [PATCH 2/8] ANPL-1704 set auth queue as default queue, to check if healthcheck messages are delaying IAM updates --- controlpanel/api/tasks/app.py | 1 + controlpanel/api/tasks/s3bucket.py | 1 + controlpanel/settings/common.py | 2 +- tests/api/fixtures/aws.py | 2 +- tests/api/models/test_app.py | 14 ++++++++------ tests/api/models/test_apps3bucket.py | 5 +++-- tests/api/models/test_users3bucket.py | 5 +++-- tests/api/views/test_apps3bucket.py | 9 +++++---- tests/api/views/test_users3bucket.py | 13 +++++++++---- 9 files changed, 32 insertions(+), 20 deletions(-) diff --git a/controlpanel/api/tasks/app.py b/controlpanel/api/tasks/app.py index 94468d322..ba399419d 100644 --- a/controlpanel/api/tasks/app.py +++ b/controlpanel/api/tasks/app.py @@ -7,6 +7,7 @@ class AppCreateRole(TaskBase): ENTITY_CLASS = "App" + QUEUE_NAME = settings.IAM_QUEUE_NAME @property def task_name(self): diff --git a/controlpanel/api/tasks/s3bucket.py b/controlpanel/api/tasks/s3bucket.py index c59f4070d..c990ca7a1 100644 --- a/controlpanel/api/tasks/s3bucket.py +++ b/controlpanel/api/tasks/s3bucket.py @@ -28,6 +28,7 @@ def _get_args_list(self): class S3AccessMixin: ACTION = None ROLE = None + QUEUE_NAME = settings.IAM_QUEUE_NAME @property def task_name(self): diff --git a/controlpanel/settings/common.py b/controlpanel/settings/common.py index 775215e00..79667506f 100644 --- a/controlpanel/settings/common.py +++ b/controlpanel/settings/common.py @@ -546,7 +546,7 @@ AUTH_QUEUE_NAME = os.environ.get("AUTH_QUEUE_NAME", "control-panel-auth") BROKER_URL = os.environ.get("BROKER_URL", "sqs://") -DEFAULT_QUEUE = IAM_QUEUE_NAME +DEFAULT_QUEUE = AUTH_QUEUE_NAME DEFAULT_BACKOFF_POLICY = {1: 10, 2: 20, 3: 40, 4: 80, 5: 320, 6: 640} PRE_DEFINED_QUEUES = [IAM_QUEUE_NAME, S3_QUEUE_NAME, AUTH_QUEUE_NAME] CELERY_DEFAULT_QUEUE = DEFAULT_QUEUE diff --git a/tests/api/fixtures/aws.py b/tests/api/fixtures/aws.py index 2c6404f82..0f858bae6 100644 --- a/tests/api/fixtures/aws.py +++ b/tests/api/fixtures/aws.py @@ -46,7 +46,7 @@ def sqs(aws_creds): with moto.mock_sqs(): sqs = boto3.resource("sqs") sqs.create_queue(QueueName=settings.DEFAULT_QUEUE) - sqs.create_queue(QueueName=settings.AUTH_QUEUE_NAME) + sqs.create_queue(QueueName=settings.IAM_QUEUE_NAME) sqs.create_queue(QueueName=settings.S3_QUEUE_NAME) yield sqs diff --git a/tests/api/models/test_app.py b/tests/api/models/test_app.py index 4c678be0e..f479a1f56 100644 --- a/tests/api/models/test_app.py +++ b/tests/api/models/test_app.py @@ -29,7 +29,7 @@ def update_aws_secrets_manager(): @pytest.fixture def app(): app = mommy.make("api.App") - app.repo_url="https://github.com/example.com/repo_name" + app.repo_url = "https://github.com/example.com/repo_name" auth_settings = dict( client_id="testing_client_id", group_id="testing_group_id" @@ -46,9 +46,9 @@ def app(): def test_create(sqs, helpers): repo_url = "https://example.com/foo__bar-baz!bat-1337" app = App.objects.create(repo_url=repo_url) - iam_messages = helpers.retrieve_messages(sqs, queue_name=settings.DEFAULT_QUEUE) + iam_messages = helpers.retrieve_messages(sqs, queue_name=settings.IAM_QUEUE_NAME) helpers.validate_task_with_sqs_messages( - iam_messages, App.__name__, app.id, queue_name=settings.DEFAULT_QUEUE + iam_messages, App.__name__, app.id, queue_name=settings.IAM_QUEUE_NAME ) auth_messages = helpers.retrieve_messages(sqs, queue_name=settings.AUTH_QUEUE_NAME) helpers.validate_task_with_sqs_messages( @@ -195,8 +195,11 @@ def test_app_allowed_ip_ranges(): ] app = mommy.make("api.App") # noqa:F841 for item in ip_allow_lists: - mommy.make("api.AppIPAllowList", - app_id=app.id, ip_allowlist_id=item.id, deployment_env="test" + mommy.make( + "api.AppIPAllowList", + app_id=app.id, + ip_allowlist_id=item.id, + deployment_env="test", ) app_ip_ranges = app.env_allowed_ip_ranges("test") assert " " not in app_ip_ranges @@ -205,4 +208,3 @@ def test_app_allowed_ip_ranges(): full_app_ip_ranges = app.app_allowed_ip_ranges assert " " not in full_app_ip_ranges assert len(full_app_ip_ranges.split(",")) == 4 - diff --git a/tests/api/models/test_apps3bucket.py b/tests/api/models/test_apps3bucket.py index 6402c2a47..39cfbd3a7 100644 --- a/tests/api/models/test_apps3bucket.py +++ b/tests/api/models/test_apps3bucket.py @@ -3,6 +3,7 @@ # Third-party import pytest +from django.conf import settings from django.db.utils import IntegrityError from model_mommy import mommy @@ -45,9 +46,9 @@ def test_aws_permissions(app, bucket, sqs, helpers): ) apps3bucket.save() - messages = helpers.retrieve_messages(sqs) + messages = helpers.retrieve_messages(sqs, queue_name=settings.IAM_QUEUE_NAME) helpers.validate_task_with_sqs_messages( - messages, AppS3Bucket.__name__, apps3bucket.id + messages, AppS3Bucket.__name__, apps3bucket.id, settings.IAM_QUEUE_NAME ) diff --git a/tests/api/models/test_users3bucket.py b/tests/api/models/test_users3bucket.py index 7d8effcda..a6fdec8b7 100644 --- a/tests/api/models/test_users3bucket.py +++ b/tests/api/models/test_users3bucket.py @@ -3,6 +3,7 @@ # Third-party import pytest +from django.conf import settings from django.db.utils import IntegrityError from model_mommy import mommy @@ -45,9 +46,9 @@ def test_aws_create_bucket(user, bucket, sqs, helpers): s3bucket=bucket, access_level=AccessToS3Bucket.READONLY ) - messages = helpers.retrieve_messages(sqs) + messages = helpers.retrieve_messages(sqs, settings.IAM_QUEUE_NAME) helpers.validate_task_with_sqs_messages( - messages, UserS3Bucket.__name__, users3bucket.id + messages, UserS3Bucket.__name__, users3bucket.id, settings.IAM_QUEUE_NAME ) diff --git a/tests/api/views/test_apps3bucket.py b/tests/api/views/test_apps3bucket.py index c61df0bac..3e125ff4e 100644 --- a/tests/api/views/test_apps3bucket.py +++ b/tests/api/views/test_apps3bucket.py @@ -4,6 +4,7 @@ # Third-party import pytest +from django.conf import settings from model_mommy import mommy from rest_framework import status from rest_framework.reverse import reverse @@ -80,9 +81,9 @@ def test_create(client, apps, buckets, sqs, helpers): response = client.post(reverse("apps3bucket-list"), data) assert response.status_code == status.HTTP_201_CREATED apps3bucket = AppS3Bucket.objects.get(app=apps[1], s3bucket=buckets[3]) - messages = helpers.retrieve_messages(sqs) + iam_messages = helpers.retrieve_messages(sqs, settings.IAM_QUEUE_NAME) helpers.validate_task_with_sqs_messages( - messages, AppS3Bucket.__name__, apps3bucket.id + iam_messages, AppS3Bucket.__name__, apps3bucket.id, settings.IAM_QUEUE_NAME ) @@ -100,9 +101,9 @@ def test_update(client, apps, apps3buckets, buckets, sqs, helpers): assert response.status_code == status.HTTP_200_OK assert response.data["access_level"] == data["access_level"] apps3bucket = AppS3Bucket.objects.get(app=apps[1], s3bucket=buckets[1]) - messages = helpers.retrieve_messages(sqs) + iam_messages = helpers.retrieve_messages(sqs, settings.IAM_QUEUE_NAME) helpers.validate_task_with_sqs_messages( - messages, AppS3Bucket.__name__, apps3bucket.id + iam_messages, AppS3Bucket.__name__, apps3bucket.id, settings.IAM_QUEUE_NAME ) diff --git a/tests/api/views/test_users3bucket.py b/tests/api/views/test_users3bucket.py index 5677b1579..14de497a5 100644 --- a/tests/api/views/test_users3bucket.py +++ b/tests/api/views/test_users3bucket.py @@ -4,6 +4,7 @@ # Third-party import pytest +from django.conf import settings from rest_framework import status from rest_framework.reverse import reverse @@ -56,8 +57,10 @@ def test_create(client, buckets, users, sqs, helpers): users3bucket = UserS3Bucket.objects.get( user=users["other_user"], s3bucket=buckets[1] ) - messages = helpers.retrieve_messages(sqs) - helpers.validate_task_with_sqs_messages(messages, UserS3Bucket.__name__, users3bucket.id) + messages = helpers.retrieve_messages(sqs, settings.IAM_QUEUE_NAME) + helpers.validate_task_with_sqs_messages( + messages, UserS3Bucket.__name__, users3bucket.id, settings.IAM_QUEUE_NAME + ) def test_delete(client, users3buckets): @@ -86,11 +89,13 @@ def test_update(client, buckets, users, users3buckets, sqs, helpers): ) assert response.status_code == status.HTTP_200_OK assert response.data["access_level"] == data["access_level"] - messages = helpers.retrieve_messages(sqs) + messages = helpers.retrieve_messages(sqs, settings.IAM_QUEUE_NAME) users3bucket = UserS3Bucket.objects.get( user=users["normal_user"], s3bucket=buckets[1] ) - helpers.validate_task_with_sqs_messages(messages, UserS3Bucket.__name__, users3bucket.id) + helpers.validate_task_with_sqs_messages( + messages, UserS3Bucket.__name__, users3bucket.id, settings.IAM_QUEUE_NAME + ) @pytest.mark.parametrize( From 68b94b59eb71daf83392d64e52c4515f98fe8b7f Mon Sep 17 00:00:00 2001 From: Michael Collins <15347726+michaeljcollinsuk@users.noreply.github.com> Date: Mon, 25 Sep 2023 14:40:59 +0100 Subject: [PATCH 3/8] ANPL-1704 fix bug where revoke access task was sent twice when deleting access --- controlpanel/api/models/access_to_s3bucket.py | 13 +++++++++++-- tests/api/models/test_apps3bucket.py | 4 ++-- tests/api/models/test_users3bucket.py | 4 ++-- tests/api/views/test_apps3bucket.py | 2 +- tests/api/views/test_users3bucket.py | 2 +- 5 files changed, 17 insertions(+), 8 deletions(-) diff --git a/controlpanel/api/models/access_to_s3bucket.py b/controlpanel/api/models/access_to_s3bucket.py index 38cc543cf..369f216fa 100644 --- a/controlpanel/api/models/access_to_s3bucket.py +++ b/controlpanel/api/models/access_to_s3bucket.py @@ -76,6 +76,15 @@ def resources(self): # MUST use signals because cascade deletes do not call delete() @receiver(models.signals.pre_delete) def revoke_access(sender, **kwargs): - if issubclass(sender, AccessToS3Bucket): - obj = kwargs["instance"] + """ + Revokes access when the delete is via cascade delete, as these do not call the + instance level delete() method. Checks that the origin is different to the instance, + to ensure that revoke_bucket_access is not called twice when deleting an access + object directly. + """ + if not issubclass(sender, AccessToS3Bucket): + return + + obj = kwargs["instance"] + if obj != kwargs["origin"]: obj.revoke_bucket_access() diff --git a/tests/api/models/test_apps3bucket.py b/tests/api/models/test_apps3bucket.py index 39cfbd3a7..2d27bf3e4 100644 --- a/tests/api/models/test_apps3bucket.py +++ b/tests/api/models/test_apps3bucket.py @@ -66,8 +66,8 @@ def test_delete_revoke_permissions(app, bucket): apps3bucket.delete() - revoke_bucket_access_task.assert_called_with( + revoke_bucket_access_task.assert_called_once_with( apps3bucket, None ) - revoke_bucket_access_task.return_value.create_task.assert_called() + revoke_bucket_access_task.return_value.create_task.assert_called_once() diff --git a/tests/api/models/test_users3bucket.py b/tests/api/models/test_users3bucket.py index a6fdec8b7..c67b80ea0 100644 --- a/tests/api/models/test_users3bucket.py +++ b/tests/api/models/test_users3bucket.py @@ -75,5 +75,5 @@ def test_delete_revoke_permissions(bucket, users3bucket): "controlpanel.api.tasks.S3BucketRevokeUserAccess" ) as revoke_user_access_task: users3bucket.delete() - revoke_user_access_task.assert_called_with(users3bucket, None) - revoke_user_access_task.return_value.create_task.assert_called() + revoke_user_access_task.assert_called_once_with(users3bucket, None) + revoke_user_access_task.return_value.create_task.assert_called_once() diff --git a/tests/api/views/test_apps3bucket.py b/tests/api/views/test_apps3bucket.py index 3e125ff4e..fd52cd79a 100644 --- a/tests/api/views/test_apps3bucket.py +++ b/tests/api/views/test_apps3bucket.py @@ -66,7 +66,7 @@ def test_delete(client, apps3buckets): response = client.delete(reverse("apps3bucket-detail", (apps3buckets[1].id,))) assert response.status_code == status.HTTP_204_NO_CONTENT - revoke_bucket_access.assert_called() + revoke_bucket_access.assert_called_once() response = client.get(reverse("apps3bucket-detail", (apps3buckets[1].id,))) assert response.status_code == status.HTTP_404_NOT_FOUND diff --git a/tests/api/views/test_users3bucket.py b/tests/api/views/test_users3bucket.py index 14de497a5..92c8071a5 100644 --- a/tests/api/views/test_users3bucket.py +++ b/tests/api/views/test_users3bucket.py @@ -70,7 +70,7 @@ def test_delete(client, users3buckets): response = client.delete(reverse("users3bucket-detail", (users3buckets[1].id,))) assert response.status_code == status.HTTP_204_NO_CONTENT - revoke_bucket_access.assert_called() + revoke_bucket_access.assert_called_once() response = client.get(reverse("users3bucket-detail", (users3buckets[1].id,))) assert response.status_code == status.HTTP_404_NOT_FOUND From 51d0bb1c6730b79c4209bfd7651aa2ed76367935 Mon Sep 17 00:00:00 2001 From: Michael Collins <15347726+michaeljcollinsuk@users.noreply.github.com> Date: Mon, 25 Sep 2023 16:11:56 +0100 Subject: [PATCH 4/8] ANPL-1704 update celery healthcheck to attempt to improve performance --- controlpanel/celery.py | 18 +++++++++++------- .../commands/celery_worker_health.py | 9 ++++++--- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/controlpanel/celery.py b/controlpanel/celery.py index 2a8679547..5aa3f345e 100644 --- a/controlpanel/celery.py +++ b/controlpanel/celery.py @@ -1,13 +1,16 @@ +# Standard library import os -from celery import Celery -import dotenv -from kombu import Queue from pathlib import Path + +# Third-party +import dotenv import structlog +from celery import Celery +from django.conf import settings +from kombu import Queue # First-party/Local from controlpanel.utils import load_app_conf_from_file -from django.conf import settings dotenv.load_dotenv() @@ -43,7 +46,8 @@ def worker_health_check(self): # ensures worker picks and runs tasks from all queues rather than just default queue # alternative is to run the worker and pass queue name to -Q flag app.conf.task_queues = [ - Queue(settings.IAM_QUEUE_NAME), - Queue(settings.AUTH_QUEUE_NAME), - Queue(settings.S3_QUEUE_NAME), + Queue(settings.IAM_QUEUE_NAME, routing_key=settings.IAM_QUEUE_NAME), + Queue(settings.AUTH_QUEUE_NAME, routing_key=settings.AUTH_QUEUE_NAME), + Queue(settings.S3_QUEUE_NAME, routing_key=settings.S3_QUEUE_NAME), ] +app.conf.task_default_exchange = "tasks" diff --git a/controlpanel/frontend/management/commands/celery_worker_health.py b/controlpanel/frontend/management/commands/celery_worker_health.py index 8ed30bd68..be1ed15a7 100644 --- a/controlpanel/frontend/management/commands/celery_worker_health.py +++ b/controlpanel/frontend/management/commands/celery_worker_health.py @@ -1,11 +1,12 @@ # Standard library +import random from datetime import datetime, timedelta from pathlib import Path from sys import exit # Third-party -from django.core.management.base import BaseCommand from django.conf import settings +from django.core.management.base import BaseCommand # First-party/Local from controlpanel.celery import worker_health_check @@ -24,8 +25,10 @@ def add_arguments(self, parser): def handle(self, *args, **options): stale_after_secs = options["stale_after_secs"] - - worker_health_check.delay().get() + # send task to randomly chosen queue + worker_health_check.apply_async( + queue=random.choice(settings.PRE_DEFINED_QUEUES) + ) # Attempt to read worker health ping file # NOTE: This may initially fail depending on timing of health task # execution but that's fine as Kubernetes' `failureThreashold` From 28f79bcac2548f9ff9a811415b4807c0b9484f97 Mon Sep 17 00:00:00 2001 From: Michael Collins <15347726+michaeljcollinsuk@users.noreply.github.com> Date: Tue, 26 Sep 2023 12:03:14 +0100 Subject: [PATCH 5/8] ANPL-1704 reduce celery polling interval to 1 second --- controlpanel/settings/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controlpanel/settings/common.py b/controlpanel/settings/common.py index 79667506f..c946d0cdb 100644 --- a/controlpanel/settings/common.py +++ b/controlpanel/settings/common.py @@ -553,7 +553,7 @@ SQS_REGION = os.environ.get("SQS_REGION", "eu-west-2") BROKER_TRANSPORT_OPTIONS = { - "polling_interval": 10, + "polling_interval": 1, "region": SQS_REGION, "wait_time_seconds": 20, "predefined_queues": {} From 4c9c5b44c7cf48fc301bbe0c565e126c26b95131 Mon Sep 17 00:00:00 2001 From: Michael Collins <15347726+michaeljcollinsuk@users.noreply.github.com> Date: Tue, 26 Sep 2023 17:35:57 +0100 Subject: [PATCH 6/8] ANPL-1704 dont run completed tasks again --- controlpanel/api/tasks/handlers/base.py | 33 +++++++++----- tests/api/tasks/test_base.py | 60 +++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 12 deletions(-) create mode 100644 tests/api/tasks/test_base.py diff --git a/controlpanel/api/tasks/handlers/base.py b/controlpanel/api/tasks/handlers/base.py index 4f1455566..ecccf684b 100644 --- a/controlpanel/api/tasks/handlers/base.py +++ b/controlpanel/api/tasks/handlers/base.py @@ -2,18 +2,31 @@ from celery import Task as CeleryTask # First-party/Local -from controlpanel.api.models import Task, User +from controlpanel.api.models import Task class BaseTaskHandler(CeleryTask): + # can be applied to project settings also + # these settings mean that messages are only removed from the queue (acknowledged) + # when returned. if an error occurs, they remain in the queue, and will be resent + # to the worker when the "visibility_timeout" has expired. "visibility_timeout" is + # a setting that is configured in SQS per queue. Currently set to 30secs + acks_late = True + acks_on_failure_or_timeout = False + task_obj = None def complete(self): - task = Task.objects.filter(task_id=self.request.id).first() - if task: - task.completed = True - task.save() + if self.task_obj: + self.task_obj.completed = True + self.task_obj.save() + + def get_task_obj(self): + return Task.objects.filter(task_id=self.request.id).first() def run(self, *args, **kwargs): + self.task_obj = self.get_task_obj() + if self.task_obj and self.task_obj.completed: + return self.handle(*args, **kwargs) def handle(self, *args, **kwargs): @@ -29,13 +42,6 @@ class BaseModelTaskHandler(BaseTaskHandler): model = None object = None task_user_pk = None - # can be applied to project settings also - # these settings mean that messages are only removed from the queue (acknowledged) - # when returned. if an error occurs, they remain in the queue, and will be resent - # to the worker when the "visibility_timeout" has expired. "visibility_timeout" is - # a setting that is configured in SQS per queue. Currently set to 30secs - acks_late = True - acks_on_failure_or_timeout = False def get_object(self, pk): try: @@ -53,6 +59,9 @@ def run(self, obj_pk, task_user_pk, *args, **kwargs): to look up the user later if required. The `handle` method is then called with any other args and kwargs sent. """ + self.task_obj = self.get_task_obj() + if self.task_obj and self.task_obj.completed: + return self.object = self.get_object(obj_pk) self.task_user_pk = task_user_pk self.handle(*args, **kwargs) diff --git a/tests/api/tasks/test_base.py b/tests/api/tasks/test_base.py new file mode 100644 index 000000000..bf2f386c7 --- /dev/null +++ b/tests/api/tasks/test_base.py @@ -0,0 +1,60 @@ +# Standard library +from unittest.mock import MagicMock, patch + +# Third-party +import pytest + +# First-party/Local +from controlpanel.api.models import Task +from controlpanel.api.tasks.handlers.base import BaseModelTaskHandler, BaseTaskHandler + + +@pytest.mark.parametrize("handler_cls, args", [ + (BaseTaskHandler, (None,)), + (BaseModelTaskHandler, (1, 1)), +]) +@patch("controlpanel.api.tasks.handlers.base.BaseTaskHandler.handle") +def test_completed_task_handle_not_run(handle, handler_cls, args): + completed_task = MagicMock(spec=Task, completed=True) + base_task_handler = handler_cls() + + with patch.object(base_task_handler, "get_task_obj", return_value=completed_task): + base_task_handler.run(*args) + + handle.assert_not_called() + + +@pytest.mark.parametrize("handler_cls, args", [ + (BaseTaskHandler, (None,)), + (BaseModelTaskHandler, (1, 1)), +]) +@patch("controlpanel.api.tasks.handlers.base.BaseTaskHandler.handle") +@patch( + "controlpanel.api.tasks.handlers.base.BaseModelTaskHandler.get_object", + new=MagicMock, +) +def test_uncompleted_task_handle_is_run(handle, handler_cls, args): + completed_task = MagicMock(spec=Task, completed=False) + base_task_handler = handler_cls() + + with patch.object(base_task_handler, "get_task_obj", return_value=completed_task): + base_task_handler.run(*args) + + handle.assert_called_once() + + +@pytest.mark.parametrize("handler_cls, args", [ + (BaseTaskHandler, (None,)), + (BaseModelTaskHandler, (1, 1)), +]) +@patch("controlpanel.api.tasks.handlers.base.BaseTaskHandler.handle") +@patch( + "controlpanel.api.tasks.handlers.base.BaseModelTaskHandler.get_object", + new=MagicMock, +) +def test_no_task_obj_handle_is_run(handle, handler_cls, args): + base_task_handler = handler_cls() + with patch.object(base_task_handler, "get_task_obj", return_value=None): + base_task_handler.run(*args) + + handle.assert_called_once() From a9a6a231cd3c04212322528a68ef3db543991795 Mon Sep 17 00:00:00 2001 From: Michael Collins <15347726+michaeljcollinsuk@users.noreply.github.com> Date: Wed, 27 Sep 2023 15:00:09 +0100 Subject: [PATCH 7/8] ANPL-1704 add task ID to incomplete task list page --- controlpanel/frontend/jinja2/includes/task-list.html | 2 ++ 1 file changed, 2 insertions(+) diff --git a/controlpanel/frontend/jinja2/includes/task-list.html b/controlpanel/frontend/jinja2/includes/task-list.html index 9dc1180d0..bd2c6b298 100644 --- a/controlpanel/frontend/jinja2/includes/task-list.html +++ b/controlpanel/frontend/jinja2/includes/task-list.html @@ -6,6 +6,7 @@ + +
Entity class Entity ID Entity descriptionTask ID Task description Create time @@ -19,6 +20,7 @@ {{ task.entity_class }} {{ task.entity_id }} {{ task.entity_description }}{{ task.task_id }} {{ task.task_description }} {{ task.created }} From ea69bf89d444345d1771f75176bb2327fa20c458 Mon Sep 17 00:00:00 2001 From: Michael Collins <15347726+michaeljcollinsuk@users.noreply.github.com> Date: Wed, 27 Sep 2023 16:39:49 +0100 Subject: [PATCH 8/8] ANPL-1704 reduce wait_time_seconds to 0 to test --- controlpanel/settings/common.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/controlpanel/settings/common.py b/controlpanel/settings/common.py index c946d0cdb..169e85040 100644 --- a/controlpanel/settings/common.py +++ b/controlpanel/settings/common.py @@ -551,14 +551,12 @@ PRE_DEFINED_QUEUES = [IAM_QUEUE_NAME, S3_QUEUE_NAME, AUTH_QUEUE_NAME] CELERY_DEFAULT_QUEUE = DEFAULT_QUEUE SQS_REGION = os.environ.get("SQS_REGION", "eu-west-2") - BROKER_TRANSPORT_OPTIONS = { "polling_interval": 1, "region": SQS_REGION, - "wait_time_seconds": 20, + "wait_time_seconds": 0, "predefined_queues": {} } - for queue in PRE_DEFINED_QUEUES: BROKER_TRANSPORT_OPTIONS['predefined_queues'][queue] = { 'url': f'https://sqs.{SQS_REGION}.amazonaws.com/{AWS_DATA_ACCOUNT_ID}/{queue}',