From ed182c2a221c403bafeb15217b6fb4a9d3041427 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Fri, 23 Oct 2020 20:01:16 +0100 Subject: [PATCH] return just the columns we need for collating letters previously we were returning the entire ORM object. Returning columns has a couple of benefits: * Means we can join on to services there and then, avoiding second queries to get the crown status of the service later in the collate flow. * Massively reduces the amount of data we return - particularly free text fields like personalisation that could be potentially quite big. 5 columns rather than 26 columns. * Minor thing, but will skip some CPU cycles as sqlalchemy will no longer construct an ORM object and try and keep track of changes. We know this function doesn't change any of the values to persist them back, so this is an unnecessary step from sqlalchemy. Disadvantages are: * The dao_get_letters_to_be_printed return interface is now much more tightly coupled to the get_key_and_size_of_letters_to_be_sent_to_print function that calls it. --- app/celery/letters_pdf_tasks.py | 6 +++--- app/dao/notifications_dao.py | 10 +++++++++- .../app/dao/notification_dao/test_notification_dao.py | 2 +- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/app/celery/letters_pdf_tasks.py b/app/celery/letters_pdf_tasks.py index 5f715c97d..75c2e5c62 100644 --- a/app/celery/letters_pdf_tasks.py +++ b/app/celery/letters_pdf_tasks.py @@ -182,15 +182,15 @@ def get_key_and_size_of_letters_to_be_sent_to_print(print_run_deadline, postage) try: letter_file_name = get_letter_pdf_filename( reference=letter.reference, - crown=letter.service.crown, + crown=letter.crown, created_at=letter.created_at, - postage=letter.postage + postage=postage ) letter_head = s3.head_s3_object(current_app.config['LETTERS_PDF_BUCKET_NAME'], letter_file_name) yield { "Key": letter_file_name, "Size": letter_head['ContentLength'], - "ServiceId": str(letter.service.id) + "ServiceId": str(letter.service_id) } except BotoClientError as e: current_app.logger.exception( diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 7c8564489..5346ed63a 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -721,7 +721,15 @@ def dao_get_letters_to_be_printed(print_run_deadline, postage): """ Return all letters created before the print run deadline that have not yet been sent """ - notifications = Notification.query.filter( + notifications = db.session.query( + Notification.id, + Notification.created_at, + Notification.reference, + Notification.service_id, + Service.crown, + ).join( + Notification.service + ).filter( Notification.created_at < convert_bst_to_utc(print_run_deadline), Notification.notification_type == LETTER_TYPE, Notification.status == NOTIFICATION_CREATED, diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 8b8717740..3e74ca0e9 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -1694,4 +1694,4 @@ def test_letters_to_be_printed_sort_by_service(notify_db_session): results = list(dao_get_letters_to_be_printed(print_run_deadline=datetime(2020, 12, 1, 17, 30), postage='second')) assert len(results) == 3 - assert results == [notification_1, notification_2, notification_3] + assert [x.id for x in results] == [notification_1.id, notification_2.id, notification_3.id]