From 8f5a5f8105aa35226df908d0fd0ae62b42562b96 Mon Sep 17 00:00:00 2001 From: venusbb Date: Thu, 18 Jan 2018 10:44:36 +0000 Subject: [PATCH 1/3] Parse acknowledgement files against .ZIP.TXT created by ftp app. - Also convert the files info to upper() for comparison rather than lower because original file names are in upper case. The unit tests contain examples of the returned lists. --- app/aws/s3.py | 4 ++-- app/celery/scheduled_tasks.py | 9 ++++---- tests/app/celery/test_scheduled_tasks.py | 27 ++++++++++++++++++------ 3 files changed, 26 insertions(+), 14 deletions(-) diff --git a/app/aws/s3.py b/app/aws/s3.py index e8ef69edc..8020c18a7 100644 --- a/app/aws/s3.py +++ b/app/aws/s3.py @@ -120,7 +120,7 @@ def get_list_of_files_by_suffix(bucket_name, subfolder='', suffix='', last_modif for page in page_iterator: for obj in page.get('Contents', []): - key = obj['Key'].lower() - if key.endswith(suffix.lower()): + key = obj['Key'] + if key.lower().endswith(suffix.lower()): if not last_modified or obj['LastModified'] >= last_modified: yield key diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 6ebaec87a..81f38ca29 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -490,9 +490,9 @@ def letter_raise_alert_if_no_ack_file_for_zip(): zip_file_list = [] for key in s3.get_list_of_files_by_suffix(bucket_name=current_app.config['LETTERS_PDF_BUCKET_NAME'], - subfolder=datetime.utcnow().strftime('%Y-%m-%d'), - suffix='.zip'): - zip_file_list.append(key) + subfolder=datetime.utcnow().strftime('%Y-%m-%d') + '/zips_sent', + suffix='.TXT'): + zip_file_list.append(key.upper().rstrip('.TXT')) # get acknowledgement file ack_file_list = [] @@ -511,13 +511,12 @@ def letter_raise_alert_if_no_ack_file_for_zip(): for zip_file in content.split('\n'): # each line s = zip_file.split('|') for zf in zip_file_list: - if s[0].lower() in zf.lower(): + if s[0].upper() in zf: zip_file_list.remove(zf) else: zip_not_today.append(s[0]) if zip_file_list: - raise NoAckFileReceived(message=zip_file_list) if zip_not_today: diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 0eb25f2a9..ac8b65d83 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -1109,16 +1109,16 @@ def test_dao_fetch_monthly_historical_stats_by_template_null_template_id_not_cou def mock_s3_get_list_match(bucket_name, subfolder='', suffix='', last_modified=None): - if subfolder == '2018-01-11': - return ['NOTIFY.20180111175007.ZIP', 'NOTIFY.20180111175008.ZIP'] + if subfolder == '2018-01-11/zips_sent': + return ['NOTIFY.20180111175007.ZIP.TXT', 'NOTIFY.20180111175008.ZIP.TXT'] if subfolder == 'root/dispatch': return ['root/dispatch/NOTIFY.20180111175733.ACK.txt'] def mock_s3_get_list_diff(bucket_name, subfolder='', suffix='', last_modified=None): - if subfolder == '2018-01-11': - return ['NOTIFY.20180111175007.ZIP', 'NOTIFY.20180111175008.ZIP', 'NOTIFY.20180111175009.ZIP', - 'NOTIFY.20180111175010.ZIP'] + if subfolder == '2018-01-11/zips_sent': + return ['NOTIFY.20180111175007.ZIP.TXT', 'NOTIFY.20180111175008.ZIP.TXT', 'NOTIFY.20180111175009.ZIP.TXT', + 'NOTIFY.20180111175010.ZIP.TXT'] if subfolder == 'root/dispatch': return ['root/dispatch/NOTIFY.20180111175733.ACK.txt'] @@ -1133,10 +1133,10 @@ def test_letter_not_raise_alert_if_ack_files_match_zip_list(mocker, notify_db): letter_raise_alert_if_no_ack_file_for_zip() yesterday = datetime.utcnow() - timedelta(days=1) - subfoldername = datetime.utcnow().strftime('%Y-%m-%d') + subfoldername = datetime.utcnow().strftime('%Y-%m-%d') + '/zips_sent' assert mock_file_list.call_count == 2 assert mock_file_list.call_args_list == [ - call(bucket_name=current_app.config['LETTERS_PDF_BUCKET_NAME'], subfolder=subfoldername, suffix='.zip'), + call(bucket_name=current_app.config['LETTERS_PDF_BUCKET_NAME'], subfolder=subfoldername, suffix='.TXT'), call(bucket_name=current_app.config['DVLA_RESPONSE_BUCKET_NAME'], subfolder='root/dispatch', suffix='.ACK.txt', last_modified=yesterday), ] @@ -1155,3 +1155,16 @@ def test_letter_not_raise_alert_if_ack_files_not_match_zip_list(mocker, notify_d assert e.value.message == ['NOTIFY.20180111175009.ZIP', 'NOTIFY.20180111175010.ZIP'] assert mock_file_list.call_count == 2 assert mock_get_file.call_count == 1 + + +@freeze_time('2018-01-11T23:00:00') +def test_letter_not_raise_alert_if_no_files_do_not_cause_error(mocker, notify_db): + mock_file_list = mocker.patch("app.aws.s3.get_list_of_files_by_suffix", side_effect=None) + mock_get_file = mocker.patch("app.aws.s3.get_s3_file", + return_value='NOTIFY.20180111175007.ZIP|20180111175733\n' + 'NOTIFY.20180111175008.ZIP|20180111175734') + + letter_raise_alert_if_no_ack_file_for_zip() + + assert mock_file_list.call_count == 2 + assert mock_get_file.call_count == 0 From 357ec3a7d5b008d53e775efd856e930725aece8a Mon Sep 17 00:00:00 2001 From: venusbb Date: Thu, 18 Jan 2018 11:06:07 +0000 Subject: [PATCH 2/3] Call Deskpro ticket when there is an error --- app/celery/scheduled_tasks.py | 7 +++++++ tests/app/celery/test_scheduled_tasks.py | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 81f38ca29..7ce16e01f 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -517,6 +517,13 @@ def letter_raise_alert_if_no_ack_file_for_zip(): zip_not_today.append(s[0]) if zip_file_list: + deskpro_client.create_ticket( + subject="Letter acknowledge error", + message="Letter acknowledgement file do not contains all zip files sent: {}".format(datetime.utcnow() + .strftime('%Y-%m-%d')), + ticket_type='alert' + ) + raise NoAckFileReceived(message=zip_file_list) if zip_not_today: diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index ac8b65d83..bbea1ad5d 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -1149,12 +1149,19 @@ def test_letter_not_raise_alert_if_ack_files_not_match_zip_list(mocker, notify_d mock_get_file = mocker.patch("app.aws.s3.get_s3_file", return_value='NOTIFY.20180111175007.ZIP|20180111175733\n' 'NOTIFY.20180111175008.ZIP|20180111175734') + mock_deskpro = mocker.patch("app.celery.scheduled_tasks.deskpro_client.create_ticket") + with pytest.raises(expected_exception=NoAckFileReceived) as e: letter_raise_alert_if_no_ack_file_for_zip() assert e.value.message == ['NOTIFY.20180111175009.ZIP', 'NOTIFY.20180111175010.ZIP'] assert mock_file_list.call_count == 2 assert mock_get_file.call_count == 1 + mock_deskpro.assert_called_once_with( + subject="Letter acknowledge error", + message="Letter acknowledgement file do not contains all zip files sent: 2018-01-11", + ticket_type='alert' + ) @freeze_time('2018-01-11T23:00:00') From 99dda9989052680a584b4562935852cbe7ba939b Mon Sep 17 00:00:00 2001 From: venusbb Date: Thu, 18 Jan 2018 14:44:23 +0000 Subject: [PATCH 3/3] Use set rather than list to compare ack file and zip files difference --- app/celery/scheduled_tasks.py | 26 ++++++++++-------------- tests/app/celery/test_scheduled_tasks.py | 2 +- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 7ce16e01f..5cea53966 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -487,36 +487,32 @@ def daily_stats_template_usage_by_month(): @statsd(namespace="tasks") def letter_raise_alert_if_no_ack_file_for_zip(): # get a list of zip files since yesterday - zip_file_list = [] + zip_file_set = set() for key in s3.get_list_of_files_by_suffix(bucket_name=current_app.config['LETTERS_PDF_BUCKET_NAME'], subfolder=datetime.utcnow().strftime('%Y-%m-%d') + '/zips_sent', suffix='.TXT'): - zip_file_list.append(key.upper().rstrip('.TXT')) + zip_file_set.add(key.upper().rstrip('.TXT')) # get acknowledgement file - ack_file_list = [] + ack_file_set = set() # yesterday = datetime.now(tz=pytz.utc) - timedelta(days=1) yesterday = datetime.utcnow() - timedelta(days=1) for key in s3.get_list_of_files_by_suffix(bucket_name=current_app.config['DVLA_RESPONSE_BUCKET_NAME'], subfolder='root/dispatch', suffix='.ACK.txt', last_modified=yesterday): - ack_file_list.append(key) + ack_file_set.add(key) today_str = datetime.utcnow().strftime('%Y%m%d') - zip_not_today = [] - for key in ack_file_list: + ack_content_set = set() + for key in ack_file_set: if today_str in key: content = s3.get_s3_file(current_app.config['DVLA_RESPONSE_BUCKET_NAME'], key) for zip_file in content.split('\n'): # each line s = zip_file.split('|') - for zf in zip_file_list: - if s[0].upper() in zf: - zip_file_list.remove(zf) - else: - zip_not_today.append(s[0]) + ack_content_set.add(s[0].upper()) - if zip_file_list: + if len(zip_file_set - ack_content_set) > 0: deskpro_client.create_ticket( subject="Letter acknowledge error", message="Letter acknowledgement file do not contains all zip files sent: {}".format(datetime.utcnow() @@ -524,9 +520,9 @@ def letter_raise_alert_if_no_ack_file_for_zip(): ticket_type='alert' ) - raise NoAckFileReceived(message=zip_file_list) + raise NoAckFileReceived(message=str(zip_file_set - ack_content_set)) - if zip_not_today: + if len(ack_content_set - zip_file_set) > 0: current_app.logger.info( - "letter ack contains zip that is not for today {} ".format(zip_not_today) + "letter ack contains zip that is not for today: {}".format(ack_content_set - zip_file_set) ) diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index bbea1ad5d..be597b8fe 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -1154,7 +1154,7 @@ def test_letter_not_raise_alert_if_ack_files_not_match_zip_list(mocker, notify_d with pytest.raises(expected_exception=NoAckFileReceived) as e: letter_raise_alert_if_no_ack_file_for_zip() - assert e.value.message == ['NOTIFY.20180111175009.ZIP', 'NOTIFY.20180111175010.ZIP'] + assert e.value.message == str(set(['NOTIFY.20180111175009.ZIP', 'NOTIFY.20180111175010.ZIP'])) assert mock_file_list.call_count == 2 assert mock_get_file.call_count == 1 mock_deskpro.assert_called_once_with(