From a997fe94a8bb4dae699dbde60528b4cb4fa77c84 Mon Sep 17 00:00:00 2001 From: pyup-bot Date: Thu, 11 Jan 2018 20:27:25 +0000 Subject: [PATCH 1/4] Update pytest-xdist from 1.21.0 to 1.22.0 --- requirements_for_test.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements_for_test.txt b/requirements_for_test.txt index eda5d1c28..27ddd354a 100644 --- a/requirements_for_test.txt +++ b/requirements_for_test.txt @@ -3,7 +3,7 @@ flake8==3.5.0 pytest==3.3.2 pytest-mock==1.6.3 pytest-cov==2.5.1 -pytest-xdist==1.21.0 +pytest-xdist==1.22.0 coveralls==1.2.0 freezegun==0.3.9 requests-mock==1.4.0 From 218fc5e14da3fb2e489fc2f7342a43ccea4a4e28 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 15 Jan 2018 17:00:00 +0000 Subject: [PATCH 2/4] only send letters in created state to ftp app for zipping this means if we end up with some notifications sending and others not, due to problems with the ftp connectivity for example, we don't re-send those that worked. As a reminder, letter pdf notifications start as created and stay that way until we have sent the zip file to DVLA, at which point they are updated to sending # --- app/celery/letters_pdf_tasks.py | 22 +++++++++- tests/app/celery/test_letters_pdf_tasks.py | 47 ++++++++++++++++++---- 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/app/celery/letters_pdf_tasks.py b/app/celery/letters_pdf_tasks.py index 7096f281a..8de14f28b 100644 --- a/app/celery/letters_pdf_tasks.py +++ b/app/celery/letters_pdf_tasks.py @@ -13,8 +13,10 @@ from app.config import QueueNames, TaskNames from app.dao.notifications_dao import ( get_notification_by_id, update_notification_status_by_id, - dao_update_notification + dao_update_notification, + dao_get_notifications_by_references, ) +from app.models import NOTIFICATION_CREATED from app.statsd_decorators import statsd @@ -112,7 +114,7 @@ def group_letters(letter_pdfs): running_filesize = 0 list_of_files = [] for letter in letter_pdfs: - if letter['Key'].lower().endswith('.pdf'): + if letter['Key'].lower().endswith('.pdf') and letter_in_created_state(letter['Key']): if ( running_filesize + letter['Size'] > current_app.config['MAX_LETTER_PDF_ZIP_FILESIZE'] or len(list_of_files) >= current_app.config['MAX_LETTER_PDF_COUNT_PER_ZIP'] @@ -126,3 +128,19 @@ def group_letters(letter_pdfs): if list_of_files: yield list_of_files + + +def letter_in_created_state(filename): + # filename looks like '2018-01-13/NOTIFY.ABCDEF1234567890.D.2.C.C.20180113120000.PDF' + subfolder = filename.split('/')[0] + ref = filename.split('.')[1] + notifications = dao_get_notifications_by_references([ref]) + if notifications: + if notifications[0].status == NOTIFICATION_CREATED: + return True + current_app.logger.info('Collating letters for {} but notification with reference {} already in {}'.format( + subfolder, + ref, + notifications[0].status + )) + return False diff --git a/tests/app/celery/test_letters_pdf_tasks.py b/tests/app/celery/test_letters_pdf_tasks.py index 5c2532934..95942712c 100644 --- a/tests/app/celery/test_letters_pdf_tasks.py +++ b/tests/app/celery/test_letters_pdf_tasks.py @@ -11,9 +11,10 @@ from app.celery.letters_pdf_tasks import ( create_letters_pdf, get_letters_pdf, collate_letter_pdfs_for_day, - group_letters + group_letters, + letter_in_created_state, ) -from app.models import Notification +from app.models import Notification, NOTIFICATION_SENDING from tests.conftest import set_config_values @@ -166,7 +167,8 @@ def test_collate_letter_pdfs_for_day(notify_api, mocker): ) -def test_group_letters_splits_on_file_size(notify_api): +def test_group_letters_splits_on_file_size(notify_api, mocker): + mocker.patch('app.celery.letters_pdf_tasks.letter_in_created_state', return_value=True) letters = [ # ends under max but next one is too big {'Key': 'A.pdf', 'Size': 1}, {'Key': 'B.pdf', 'Size': 2}, @@ -192,7 +194,8 @@ def test_group_letters_splits_on_file_size(notify_api): assert next(x, None) is None -def test_group_letters_splits_on_file_count(notify_api): +def test_group_letters_splits_on_file_count(notify_api, mocker): + mocker.patch('app.celery.letters_pdf_tasks.letter_in_created_state', return_value=True) letters = [ {'Key': 'A.pdf', 'Size': 1}, {'Key': 'B.pdf', 'Size': 2}, @@ -215,7 +218,8 @@ def test_group_letters_splits_on_file_count(notify_api): assert next(x, None) is None -def test_group_letters_splits_on_file_size_and_file_count(notify_api): +def test_group_letters_splits_on_file_size_and_file_count(notify_api, mocker): + mocker.patch('app.celery.letters_pdf_tasks.letter_in_created_state', return_value=True) letters = [ # ends under max file size but next file is too big {'Key': 'A.pdf', 'Size': 1}, @@ -249,10 +253,39 @@ def test_group_letters_splits_on_file_size_and_file_count(notify_api): assert next(x, None) is None -def test_group_letters_ignores_non_pdfs(notify_api): +def test_group_letters_ignores_non_pdfs(notify_api, mocker): + mocker.patch('app.celery.letters_pdf_tasks.letter_in_created_state', return_value=True) letters = [{'Key': 'A.zip'}] assert list(group_letters(letters)) == [] -def test_group_letters_with_no_letters(notify_api): +def test_group_letters_ignores_notifications_already_sent(notify_api, mocker): + mock = mocker.patch('app.celery.letters_pdf_tasks.letter_in_created_state', return_value=False) + letters = [{'Key': 'A.pdf'}] + assert list(group_letters(letters)) == [] + mock.assert_called_once_with('A.pdf') + + +def test_group_letters_with_no_letters(notify_api, mocker): + mocker.patch('app.celery.letters_pdf_tasks.letter_in_created_state', return_value=True) assert list(group_letters([])) == [] + + +def test_letter_in_created_state(sample_notification): + sample_notification.reference = 'ABCDEF1234567890' + filename = '2018-01-13/NOTIFY.ABCDEF1234567890.D.2.C.C.20180113120000.PDF' + + assert letter_in_created_state(filename) is True + + +def test_letter_in_created_state_fails_if_notification_not_in_created(sample_notification): + sample_notification.reference = 'ABCDEF1234567890' + sample_notification.status = NOTIFICATION_SENDING + filename = '2018-01-13/NOTIFY.ABCDEF1234567890.D.2.C.C.20180113120000.PDF' + assert letter_in_created_state(filename) is False + + +def test_letter_in_created_state_fails_if_notification_doesnt_exist(sample_notification): + sample_notification.reference = 'QWERTY1234567890' + filename = '2018-01-13/NOTIFY.ABCDEF1234567890.D.2.C.C.20180113120000.PDF' + assert letter_in_created_state(filename) is False From afcbfda566cd53a127a9ad09da85d5fc37bfbdc0 Mon Sep 17 00:00:00 2001 From: pyup-bot Date: Tue, 16 Jan 2018 02:54:27 +0000 Subject: [PATCH 3/4] Update boto3 from 1.5.6 to 1.5.15 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 21a7b9ea1..c5f19084a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -boto3==1.5.6 +boto3==1.5.15 cffi==1.11.0 # pyup: != 1.11.1, != 1.11.2 # These versions are missing .whl celery==3.1.25 # pyup: <4 docopt==0.6.2 From a2cf254a17d0dae2133ef6f9c561438866eb00f2 Mon Sep 17 00:00:00 2001 From: pyup-bot Date: Tue, 16 Jan 2018 02:54:36 +0000 Subject: [PATCH 4/4] Update awscli from 1.14.16 to 1.14.25 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 21a7b9ea1..9cc784c82 100644 --- a/requirements.txt +++ b/requirements.txt @@ -22,7 +22,7 @@ SQLAlchemy==1.2.0 notifications-python-client==4.7.1 # PaaS -awscli==1.14.16 +awscli==1.14.25 awscli-cwlogs>=1.4,<1.5 git+https://github.com/alphagov/notifications-utils.git@23.4.1#egg=notifications-utils==23.4.1