diff --git a/app/celery/celery.py b/app/celery/celery.py index 183e50bd6..59d240d07 100644 --- a/app/celery/celery.py +++ b/app/celery/celery.py @@ -1,17 +1,28 @@ -from celery import Celery +from flask import current_app +from celery import Celery, Task + + +class NotifyTask(Task): + abstract = True + + def on_failure(self, exc, task_id, args, kwargs, einfo): + # ensure task will log exceptions to correct handlers + current_app.logger.exception('Celery task failed') + super().on_failure(exc, task_id, args, kwargs, einfo) + + def __call__(self, *args, **kwargs): + # ensure task has flask context to access config, logger, etc + with current_app.app_context(): + return super().__call__(*args, **kwargs) class NotifyCelery(Celery): def init_app(self, app): - super().__init__(app.import_name, broker=app.config['BROKER_URL']) + super().__init__( + app.import_name, + broker=app.config['BROKER_URL'], + task_cls=NotifyTask, + ) + self.conf.update(app.config) - TaskBase = self.Task - - class ContextTask(TaskBase): - abstract = True - - def __call__(self, *args, **kwargs): - with app.app_context(): - return TaskBase.__call__(self, *args, **kwargs) - self.Task = ContextTask diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index bf10ceec7..e18cbe8e6 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -314,7 +314,7 @@ def populate_monthly_billing(): def run_letter_jobs(): job_ids = dao_get_letter_job_ids_by_status(JOB_STATUS_READY_TO_SEND) notify_celery.send_task( - name=TaskNames.DVLA_FILES, + name=TaskNames.DVLA_JOBS, args=(job_ids,), queue=QueueNames.PROCESS_FTP ) diff --git a/app/config.py b/app/config.py index c6c5291eb..daf552348 100644 --- a/app/config.py +++ b/app/config.py @@ -48,7 +48,8 @@ class QueueNames(object): class TaskNames(object): - DVLA_FILES = 'send-files-to-dvla' + DVLA_JOBS = 'send-jobs-to-dvla' + DVLA_NOTIFICATIONS = 'send-notifications-to-dvla' class Config(object): @@ -226,7 +227,7 @@ class Config(object): }, 'run-letter-jobs': { 'task': 'run-letter-jobs', - 'schedule': crontab(minute=30, hour=17), + 'schedule': crontab(hour=17, minute=30), 'options': {'queue': QueueNames.PERIODIC} } } diff --git a/app/letters/rest.py b/app/letters/rest.py index b9ce7bf35..611d5132d 100644 --- a/app/letters/rest.py +++ b/app/letters/rest.py @@ -16,7 +16,7 @@ register_errors(letter_job) @letter_job.route('/send-letter-jobs', methods=['POST']) def send_letter_jobs(): job_ids = validate(request.get_json(), letter_job_ids) - notify_celery.send_task(name=TaskNames.DVLA_FILES, args=(job_ids['job_ids'],), queue=QueueNames.PROCESS_FTP) + notify_celery.send_task(name=TaskNames.DVLA_JOBS, args=(job_ids['job_ids'],), queue=QueueNames.PROCESS_FTP) return jsonify(data={"response": "Task created to send files to DVLA"}), 201 diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index c2544ffd4..47a47a711 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -151,7 +151,7 @@ def process_letter_notification(*, letter_data, api_key, template): raise BadRequestError(message='Cannot send letters with a team api key', status_code=403) if api_key.service.restricted and api_key.key_type != KEY_TYPE_TEST: - raise BadRequestError(message='Cannot send letters when service is in trial mode', status_code=403) + raise BadRequestError(message='Cannot send letters when service is in trial mode', status_code=403) job = create_letter_api_job(template) notification = create_letter_notification(letter_data, job, api_key) diff --git a/requirements.txt b/requirements.txt index 5750eea63..e4f0757f8 100644 --- a/requirements.txt +++ b/requirements.txt @@ -26,6 +26,6 @@ notifications-python-client==4.4.0 awscli>=1.11,<1.12 awscli-cwlogs>=1.4,<1.5 -git+https://github.com/alphagov/notifications-utils.git@21.0.0#egg=notifications-utils==21.0.0 +git+https://github.com/alphagov/notifications-utils.git@21.2.0#egg=notifications-utils==21.2.0 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3 diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 6cfbcf57a..fca018597 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -690,6 +690,6 @@ def test_run_letter_jobs(client, mocker, sample_letter_template): run_letter_jobs() - mock_celery.assert_called_once_with(name=TaskNames.DVLA_FILES, + mock_celery.assert_called_once_with(name=TaskNames.DVLA_JOBS, args=(job_ids,), queue=QueueNames.PROCESS_FTP) diff --git a/tests/app/letters/test_send_letter_jobs.py b/tests/app/letters/test_send_letter_jobs.py index d899387b6..792d55269 100644 --- a/tests/app/letters/test_send_letter_jobs.py +++ b/tests/app/letters/test_send_letter_jobs.py @@ -25,7 +25,7 @@ def test_send_letter_jobs(client, mocker, sample_letter_template): assert response.status_code == 201 assert json.loads(response.get_data())['data'] == {'response': "Task created to send files to DVLA"} - mock_celery.assert_called_once_with(name="send-files-to-dvla", + mock_celery.assert_called_once_with(name="send-jobs-to-dvla", args=(job_ids['job_ids'],), queue="process-ftp-tasks")