From e5a0cde3d959a27b6bd9fd37e88e59aba100e340 Mon Sep 17 00:00:00 2001 From: Anthoni Gleeson Date: Fri, 22 Sep 2023 12:07:03 +0100 Subject: [PATCH 1/4] adding retry config --- common/celery.py | 4 ++++ notifications/tasks.py | 10 +++++++++- publishing/tasks.py | 9 ++++++++- settings/common.py | 44 +++++++++++++++++++++++++++++++++++++++--- 4 files changed, 62 insertions(+), 5 deletions(-) diff --git a/common/celery.py b/common/celery.py index 1c2f8c82c..9cf54f146 100644 --- a/common/celery.py +++ b/common/celery.py @@ -20,3 +20,7 @@ # this should be automactically configured via ^^ config_from_object # but it isn't so here it's configured here app.conf.task_routes = settings.CELERY_ROUTES + +# this means the celery task only sends the acknowledgement (removes from task queue) +# after it has completed +# app.conf.task_ack_late = True # What's the impact on all of our tasks? diff --git a/notifications/tasks.py b/notifications/tasks.py index a771c71b3..efafa4158 100644 --- a/notifications/tasks.py +++ b/notifications/tasks.py @@ -1,12 +1,20 @@ import logging from celery import shared_task +from django.conf import settings from django.db.transaction import atomic logger = logging.getLogger(__name__) -@shared_task +@shared_task( + default_retry_delay=settings.NOTIFICATIONS_API_DEFAULT_RETRY_DELAY, + max_retries=settings.NOTIFICATIONS_API_MAX_RETRIES, + retry_backoff=True, + retry_backoff_max=settings.NOTIFICATIONS_RETRY_BACKOFF_MAX, + retry_jitter=True, + autoretry_for=(Exception,), +) @atomic def send_emails_task(notification_pk: int): """Task for emailing all users signed up to receive packaging updates and diff --git a/publishing/tasks.py b/publishing/tasks.py index 57e2664f1..ea172ae89 100644 --- a/publishing/tasks.py +++ b/publishing/tasks.py @@ -11,7 +11,14 @@ logger = logging.getLogger(__name__) -@app.task +@app.task( + default_retry_delay=settings.ENVELOPE_GENERATION_DEFAULT_RETRY_DELAY, + max_retries=settings.ENVELOPE_GENERATION_MAX_RETRIES, + retry_backoff=True, + retry_backoff_max=settings.ENVELOPE_GENERATION_RETRY_BACKOFF_MAX, + retry_jitter=True, + autoretry_for=(Exception,), +) def create_xml_envelope_file( packaged_work_basket_id: int, notify_when_done: bool = True, diff --git a/settings/common.py b/settings/common.py index aa667ea4f..8cecb4e00 100644 --- a/settings/common.py +++ b/settings/common.py @@ -437,16 +437,54 @@ # Settings about retrying uploads if the api cannot be contacted. # Names correspond to celery settings for retrying tasks: # https://docs.celeryq.dev/en/stable/userguide/tasks.html#automatic-retry-for-known-exceptions +CELERY_MAX_RETRIES = int( + os.environ.get("CELERY_MAX_RETRIES", "3"), +) +CELERY_RETRY_BACKOFF_MAX = int( + os.environ.get("CELERY_RETRY_BACKOFF_MAX", "600"), +) +CELERY_DEFAULT_RETRY_DELAY = int( + os.environ.get("CELERY_DEFAULT_RETRY_DELAY", "8"), +) + CROWN_DEPENDENCIES_API_MAX_RETRIES = int( - os.environ.get("CROWN_DEPENDENCIES_API_MAX_RETRIES", "3"), + os.environ.get("CROWN_DEPENDENCIES_API_MAX_RETRIES", CELERY_MAX_RETRIES), ) CROWN_DEPENDENCIES_API_RETRY_BACKOFF_MAX = int( - os.environ.get("CROWN_DEPENDENCIES_API_RETRY_BACKOFF_MAX", "600"), + os.environ.get( + "CROWN_DEPENDENCIES_API_RETRY_BACKOFF_MAX", + CELERY_RETRY_BACKOFF_MAX, + ), ) CROWN_DEPENDENCIES_API_DEFAULT_RETRY_DELAY = int( - os.environ.get("CROWN_DEPENDENCIES_API_DEFAULT_RETRY_DELAY", "8"), + os.environ.get( + "CROWN_DEPENDENCIES_API_DEFAULT_RETRY_DELAY", + CELERY_DEFAULT_RETRY_DELAY, + ), ) +ENVELOPE_GENERATION_DEFAULT_RETRY_DELAY = int( + os.environ.get("ENVELOPE_GENERATION_DEFAULT_RETRY_DELAY", CELERY_MAX_RETRIES), +) +ENVELOPE_GENERATION_RETRY_BACKOFF_MAX = int( + os.environ.get("ENVELOPE_GENERATION_RETRY_BACKOFF_MAX", CELERY_RETRY_BACKOFF_MAX), +) +ENVELOPE_GENERATION_DEFAULT_RETRY_DELAY = int( + os.environ.get( + "ENVELOPE_GENERATION_DEFAULT_RETRY_DELAY", + CELERY_DEFAULT_RETRY_DELAY, + ), +) + +NOTIFICATIONS_API_DEFAULT_RETRY_DELAY = int( + os.environ.get("NOTIFICATIONS_API_DEFAULT_RETRY_DELAY", CELERY_MAX_RETRIES), +) +NOTIFICATIONS_RETRY_BACKOFF_MAX = int( + os.environ.get("NOTIFICATIONS_RETRY_BACKOFF_MAX", CELERY_RETRY_BACKOFF_MAX), +) +NOTIFICATIONS_DEFAULT_RETRY_DELAY = int( + os.environ.get("NOTIFICATIONS_DEFAULT_RETRY_DELAY", CELERY_DEFAULT_RETRY_DELAY), +) # SQLite AWS settings SQLITE_STORAGE_BUCKET_NAME = os.environ.get("SQLITE_STORAGE_BUCKET_NAME", "sqlite") From 9a4e1a9c4bf0585614a67c4ceb36c13d2abc2370 Mon Sep 17 00:00:00 2001 From: Anthoni Gleeson Date: Fri, 22 Sep 2023 12:51:55 +0100 Subject: [PATCH 2/4] fix config --- settings/common.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/settings/common.py b/settings/common.py index 8cecb4e00..43514b58d 100644 --- a/settings/common.py +++ b/settings/common.py @@ -463,8 +463,8 @@ ), ) -ENVELOPE_GENERATION_DEFAULT_RETRY_DELAY = int( - os.environ.get("ENVELOPE_GENERATION_DEFAULT_RETRY_DELAY", CELERY_MAX_RETRIES), +ENVELOPE_GENERATION_MAX_RETRIES = int( + os.environ.get("ENVELOPE_GENERATION_MAX_RETRIES", CELERY_MAX_RETRIES), ) ENVELOPE_GENERATION_RETRY_BACKOFF_MAX = int( os.environ.get("ENVELOPE_GENERATION_RETRY_BACKOFF_MAX", CELERY_RETRY_BACKOFF_MAX), @@ -476,8 +476,8 @@ ), ) -NOTIFICATIONS_API_DEFAULT_RETRY_DELAY = int( - os.environ.get("NOTIFICATIONS_API_DEFAULT_RETRY_DELAY", CELERY_MAX_RETRIES), +NOTIFICATIONS_API_MAX_RETRIES = int( + os.environ.get("NOTIFICATIONS_API_MAX_RETRIES", CELERY_MAX_RETRIES), ) NOTIFICATIONS_RETRY_BACKOFF_MAX = int( os.environ.get("NOTIFICATIONS_RETRY_BACKOFF_MAX", CELERY_RETRY_BACKOFF_MAX), From 28d2bce5d65ea81d460b0536db7f9b3b7f04702b Mon Sep 17 00:00:00 2001 From: Anthoni Gleeson Date: Fri, 22 Sep 2023 13:01:59 +0100 Subject: [PATCH 3/4] another syntax error --- notifications/tasks.py | 4 ++-- settings/common.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/notifications/tasks.py b/notifications/tasks.py index efafa4158..78cc12d52 100644 --- a/notifications/tasks.py +++ b/notifications/tasks.py @@ -8,8 +8,8 @@ @shared_task( - default_retry_delay=settings.NOTIFICATIONS_API_DEFAULT_RETRY_DELAY, - max_retries=settings.NOTIFICATIONS_API_MAX_RETRIES, + default_retry_delay=settings.NOTIFICATIONS_DEFAULT_RETRY_DELAY, + max_retries=settings.NOTIFICATIONS_MAX_RETRIES, retry_backoff=True, retry_backoff_max=settings.NOTIFICATIONS_RETRY_BACKOFF_MAX, retry_jitter=True, diff --git a/settings/common.py b/settings/common.py index 43514b58d..1eee9c599 100644 --- a/settings/common.py +++ b/settings/common.py @@ -476,8 +476,8 @@ ), ) -NOTIFICATIONS_API_MAX_RETRIES = int( - os.environ.get("NOTIFICATIONS_API_MAX_RETRIES", CELERY_MAX_RETRIES), +NOTIFICATIONS_MAX_RETRIES = int( + os.environ.get("NOTIFICATIONS_MAX_RETRIES", CELERY_MAX_RETRIES), ) NOTIFICATIONS_RETRY_BACKOFF_MAX = int( os.environ.get("NOTIFICATIONS_RETRY_BACKOFF_MAX", CELERY_RETRY_BACKOFF_MAX), From 120d1ffc6b94917fa090afefbe7d4a74f02b0218 Mon Sep 17 00:00:00 2001 From: Anthoni Gleeson Date: Fri, 22 Sep 2023 13:24:47 +0100 Subject: [PATCH 4/4] fix test --- publishing/tests/test_tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/publishing/tests/test_tasks.py b/publishing/tests/test_tasks.py index 4f7c0171f..7e66781b2 100644 --- a/publishing/tests/test_tasks.py +++ b/publishing/tests/test_tasks.py @@ -38,7 +38,7 @@ def test_create_and_upload_envelope( create_xml_envelope_file.apply( (packaged_work_basket.pk, True), ) - mock_save.assert_called_once() + mock_save.assert_called() assert expected_bucket in s3_bucket_names()