mirror of
https://github.com/GSA/notifications-api.git
synced 2026-04-05 01:49:29 -04:00
only catch s3 and db errors in build-dvla-file-for-job
this reduces the amount of error messages we log (we'll no longer log at error level when build-dvla-file-for-job retries while waiting for the task to finish), and make sure we retry in those cases above - db or s3 having temporary troubl
This commit is contained in:
@@ -18,6 +18,8 @@ from requests import (
|
||||
RequestException
|
||||
)
|
||||
from sqlalchemy.exc import SQLAlchemyError
|
||||
from botocore.exceptions import ClientError as BotoClientError
|
||||
|
||||
from app import (
|
||||
create_uuid,
|
||||
create_random_identifier,
|
||||
@@ -325,11 +327,11 @@ def build_dvla_file(self, job_id):
|
||||
else:
|
||||
msg = "All notifications for job {} are not persisted".format(job_id)
|
||||
current_app.logger.info(msg)
|
||||
self.retry(queue=QueueNames.RETRY, exc=Exception(msg))
|
||||
except Exception as e:
|
||||
# ? should this retry?
|
||||
self.retry(queue=QueueNames.RETRY)
|
||||
# specifically don't catch celery.retry errors
|
||||
except (SQLAlchemyError, BotoClientError):
|
||||
current_app.logger.exception("build_dvla_file threw exception")
|
||||
raise e
|
||||
self.retry(queue=QueueNames.RETRY)
|
||||
|
||||
|
||||
@notify_celery.task(bind=True, name='update-letter-job-to-sent')
|
||||
|
||||
@@ -1,15 +1,17 @@
|
||||
import json
|
||||
import uuid
|
||||
from datetime import datetime, timedelta
|
||||
from unittest.mock import Mock, ANY
|
||||
from unittest.mock import Mock
|
||||
|
||||
import pytest
|
||||
import requests_mock
|
||||
from flask import current_app
|
||||
from freezegun import freeze_time
|
||||
from requests import RequestException
|
||||
from sqlalchemy.exc import SQLAlchemyError
|
||||
from notifications_utils.template import SMSMessageTemplate, WithSubjectTemplate, LetterDVLATemplate
|
||||
from celery.exceptions import Retry
|
||||
from botocore.exceptions import ClientError
|
||||
from notifications_utils.template import SMSMessageTemplate, WithSubjectTemplate, LetterDVLATemplate
|
||||
|
||||
from app import (encryption, DATETIME_FORMAT)
|
||||
from app.celery import provider_tasks
|
||||
@@ -1023,9 +1025,7 @@ def test_build_dvla_file(sample_letter_template, mocker):
|
||||
create_notification(template=job.template, job=job)
|
||||
mocked_upload = mocker.patch("app.celery.tasks.s3upload")
|
||||
mocked_send_task = mocker.patch("app.celery.tasks.notify_celery.send_task")
|
||||
mocked_letter_template = mocker.patch("app.celery.tasks.LetterDVLATemplate")
|
||||
mocked_letter_template_instance = mocked_letter_template.return_value
|
||||
mocked_letter_template_instance.__str__.return_value = "dvla|string"
|
||||
mocker.patch("app.celery.tasks.LetterDVLATemplate", return_value='dvla|string')
|
||||
build_dvla_file(job.id)
|
||||
|
||||
mocked_upload.assert_called_once_with(
|
||||
@@ -1048,14 +1048,25 @@ def test_build_dvla_file_retries_if_all_notifications_are_not_created(sample_let
|
||||
build_dvla_file(job.id)
|
||||
mocked.assert_not_called()
|
||||
|
||||
tasks.build_dvla_file.retry.assert_called_with(
|
||||
queue="retry-tasks",
|
||||
exc=ANY
|
||||
)
|
||||
tasks.build_dvla_file.retry.assert_called_with(queue="retry-tasks")
|
||||
assert Job.query.get(job.id).job_status == 'in progress'
|
||||
mocked_send_task.assert_not_called()
|
||||
|
||||
|
||||
def test_build_dvla_file_retries_if_s3_err(sample_letter_template, mocker):
|
||||
job = create_job(sample_letter_template, notification_count=1)
|
||||
create_notification(job.template, job=job)
|
||||
|
||||
mocker.patch('app.celery.tasks.LetterDVLATemplate', return_value='dvla|string')
|
||||
mocker.patch('app.celery.tasks.s3upload', side_effect=ClientError({}, 'operation_name'))
|
||||
retry_mock = mocker.patch('app.celery.tasks.build_dvla_file.retry', side_effect=Retry)
|
||||
|
||||
with pytest.raises(Retry):
|
||||
build_dvla_file(job.id)
|
||||
|
||||
retry_mock.assert_called_once_with(queue='retry-tasks')
|
||||
|
||||
|
||||
def test_create_dvla_file_contents(notify_db_session, mocker):
|
||||
service = create_service(service_permissions=SERVICE_PERMISSION_TYPES)
|
||||
create_letter_contact(service=service, contact_block='London,\nNW1A 1AA')
|
||||
|
||||
Reference in New Issue
Block a user