Commit the db session before sending notification to the queue.

When a high volume service posts a notification we want to commit the db session before we send the message to SQS. This will release the db connection earlier.

app/notifications/process_letter_notifications.py

@@ -5,7 +5,9 @@ from app.models import LETTER_TYPE
 from app.notifications.process_notifications import persist_notification
 
 
-def create_letter_notification(letter_data, template, api_key, status, reply_to_text=None, billable_units=None):
+def create_letter_notification(
+    letter_data, template, api_key, status, reply_to_text=None, billable_units=None, updated_at=None
+):
     notification = persist_notification(
         template_id=template.id,
         template_version=template.version,
@@ -24,6 +26,7 @@ def create_letter_notification(letter_data, template, api_key, status, reply_to_
         status=status,
         reply_to_text=reply_to_text,
         billable_units=billable_units,
-        postage=letter_data.get('postage')
+        postage=letter_data.get('postage'),
+        updated_at=updated_at
     )
     return notification

app/notifications/process_notifications.py

@@ -79,7 +79,8 @@ def persist_notification(
     billable_units=None,
     postage=None,
     template_postage=None,
-    document_download_count=None
+    document_download_count=None,
+    updated_at=None
 ):
     notification_created_at = created_at or datetime.utcnow()
     if not notification_id:
@@ -105,6 +106,7 @@ def persist_notification(
         reply_to_text=reply_to_text,
         billable_units=billable_units,
         document_download_count=document_download_count,
+        updated_at=updated_at
     )
 
     if notification_type == SMS_TYPE:

app/v2/notifications/post_notifications.py

@@ -15,8 +15,8 @@ from app import (
     notify_celery,
     document_download_client,
     encryption,
-    DATETIME_FORMAT
-)
+    DATETIME_FORMAT,
+    db)
 from app.celery.letters_pdf_tasks import get_pdf_for_templated_letter, sanitise_letter
 from app.celery.research_mode_tasks import create_fake_letter_response_file
 from app.celery.tasks import save_api_email
@@ -187,8 +187,10 @@ def process_sms_or_email_notification(
     if document_download_count:
         # We changed personalisation which means we need to update the content
         template.values = personalisation
+    api_key_id = api_key.id
     key_type = api_key.key_type
     service_in_research_mode = service.research_mode
+    template_version = template._template['version']
     resp = create_response_for_post_notification(
         notification_id=notification_id,
         client_reference=form.get('reference', None),
@@ -200,19 +202,22 @@ def process_sms_or_email_notification(
         scheduled_for=form.get("scheduled_for", None),
         template_with_content=template)
 
-    if str(service.id) in current_app.config.get('HIGH_VOLUME_SERVICE') and api_key.key_type == KEY_TYPE_NORMAL \
+    if str(service.id) in current_app.config.get('HIGH_VOLUME_SERVICE') and key_type == KEY_TYPE_NORMAL \
        and notification_type == EMAIL_TYPE:
         # Put GOV.UK Email notifications onto a queue
         # To take the pressure off the db for API requests put the notification for our high volume service onto a queue
         # the task will then save the notification, then call send_notification_to_queue.
         # We know that this team does not use the GET request, but relies on callbacks to get the status updates.
         try:
+            db.session.commit()
             save_email_to_queue(
                 form=form,
                 notification_id=str(notification_id),
                 notification_type=notification_type,
-                api_key=api_key,
-                template=template,
+                api_key_id=api_key_id,
+                key_type=key_type,
+                template_id=template.id,
+                template_version=template_version,
                 service_id=service.id,
                 personalisation=personalisation,
                 document_download_count=document_download_count,
@@ -229,12 +234,12 @@ def process_sms_or_email_notification(
     persist_notification(
         notification_id=notification_id,
         template_id=template.id,
-        template_version=template._template['version'],
+        template_version=template_version,
         recipient=form_send_to,
         service=service,
         personalisation=personalisation,
         notification_type=notification_type,
-        api_key_id=api_key.id,
+        api_key_id=api_key_id,
         key_type=key_type,
         client_reference=form.get('reference', None),
         simulated=simulated,
@@ -266,23 +271,26 @@ def save_email_to_queue(
     notification_id,
     form,
     notification_type,
-    api_key,
-    template,
+    api_key_id,
+    key_type,
+    template_id,
+    template_version,
     service_id,
     personalisation,
     document_download_count,
     reply_to_text=None
 ):
+    db.session.commit()
     data = {
         "id": notification_id,
-        "template_id": str(template.id),
-        "template_version": template._template['version'],
+        "template_id": str(template_id),
+        "template_version": template_version,
         "to": form['email_address'],
         "service_id": str(service_id),
         "personalisation": personalisation,
         "notification_type": notification_type,
-        "api_key_id": str(api_key.id),
-        "key_type": api_key.key_type,
+        "api_key_id": str(api_key_id),
+        "key_type": key_type,
         "client_reference": form.get('reference', None),
         "reply_to_text": reply_to_text,
         "document_download_count": document_download_count,
@@ -347,6 +355,7 @@ def process_letter_notification(
     test_key = api_key.key_type == KEY_TYPE_TEST
 
     status = NOTIFICATION_CREATED
+    updated_at = None
     if test_key:
         # if we don't want to actually send the letter, then start it off in SENDING so we don't pick it up
         if current_app.config['NOTIFY_ENVIRONMENT'] in ['preview', 'development']:
@@ -354,6 +363,7 @@ def process_letter_notification(
         # mark test letter as delivered and do not create a fake response later
         else:
             status = NOTIFICATION_DELIVERED
+            updated_at = datetime.utcnow()
 
     queue = QueueNames.CREATE_LETTERS_PDF if not test_key else QueueNames.RESEARCH_MODE
 
@@ -361,7 +371,9 @@ def process_letter_notification(
                                               template=template,
                                               api_key=api_key,
                                               status=status,
-                                              reply_to_text=reply_to_text)
+                                              reply_to_text=reply_to_text,
+                                              updated_at=updated_at
+                                              )
 
     get_pdf_for_templated_letter.apply_async(
         [str(notification.id)],

requirements-app.txt

@@ -26,7 +26,7 @@ notifications-python-client==5.5.1
 # PaaS
 awscli-cwlogs>=1.4,<1.5
 
-git+https://github.com/alphagov/notifications-utils.git@39.4.4#egg=notifications-utils==39.4.4
+git+https://github.com/alphagov/notifications-utils.git@39.6.0#egg=notifications-utils==39.6.0
 
 # gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains
 prometheus-client==0.7.1

requirements.txt

@@ -28,7 +28,7 @@ notifications-python-client==5.5.1
 # PaaS
 awscli-cwlogs>=1.4,<1.5
 
-git+https://github.com/alphagov/notifications-utils.git@39.4.4#egg=notifications-utils==39.4.4
+git+https://github.com/alphagov/notifications-utils.git@39.6.0#egg=notifications-utils==39.6.0
 
 # gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains
 prometheus-client==0.7.1
@@ -39,14 +39,15 @@ alembic==1.4.2
 amqp==1.4.9
 anyjson==0.3.3
 attrs==19.3.0
-awscli==1.18.75
+awscli==1.18.82
 bcrypt==3.1.7
 billiard==3.3.0.23
 bleach==3.1.4
 blinker==1.4
 boto==2.49.0
 boto3==1.10.38
-botocore==1.16.25
+botocore==1.17.5
+cachetools==4.1.0
 certifi==2020.4.5.2
 chardet==3.0.4
 click==7.1.2
@@ -78,7 +79,7 @@ python-json-logger==0.1.11
 pytz==2020.1
 PyYAML==5.3.1
 redis==3.5.3
-requests==2.23.0
+requests==2.24.0
 rsa==3.4.2
 s3transfer==0.3.3
 six==1.15.0

tests/app/v2/notifications/test_post_letter_notifications.py

@@ -245,6 +245,7 @@ def test_post_letter_notification_with_test_key_creates_pdf_and_sets_status_to_d
     fake_create_letter_task.assert_called_once_with([str(notification.id)], queue='research-mode-tasks')
     assert not fake_create_dvla_response_task.called
     assert notification.status == NOTIFICATION_DELIVERED
+    assert notification.updated_at is not None
 
 
 @pytest.mark.parametrize('env', [

tests/app/v2/notifications/test_post_notifications.py

@@ -1006,11 +1006,14 @@ def test_post_notifications_saves_email_normally_if_save_email_to_queue_fails(cl
         "template_id": template.id,
         "personalisation": {"message": "Dear citizen, have a nice day"}
     }
+
+    print("******** Start")
     response = client.post(
         path='/v2/notifications/email',
         data=json.dumps(data),
         headers=[('Content-Type', 'application/json'), create_authorization_header(service_id=service.id)]
     )
+    print("********** End")
 
     json_resp = response.get_json()