From a14d5f0225ba89fe9ab39875a5cf36e0566b2c30 Mon Sep 17 00:00:00 2001 From: David McDonald Date: Thu, 6 Feb 2020 10:24:39 +0000 Subject: [PATCH] Remove task that no longer runs We no longer puts files in these s3 buckets (and have in fact deleted the buckets) therefore this task is redundant and can be removed. --- app/aws/s3.py | 7 --- app/celery/nightly_tasks.py | 10 ----- app/config.py | 12 +----- tests/app/aws/test_s3.py | 14 ------ tests/app/celery/test_nightly_tasks.py | 59 -------------------------- 5 files changed, 1 insertion(+), 101 deletions(-) diff --git a/app/aws/s3.py b/app/aws/s3.py index cb4fef679..01fa04ac3 100644 --- a/app/aws/s3.py +++ b/app/aws/s3.py @@ -97,13 +97,6 @@ def remove_s3_object(bucket_name, object_key): return obj.delete() -def remove_transformed_dvla_file(job_id): - bucket_name = current_app.config['DVLA_BUCKETS']['job'] - file_location = '{}-dvla-job.text'.format(job_id) - obj = get_s3_object(bucket_name, file_location) - return obj.delete() - - def get_list_of_files_by_suffix(bucket_name, subfolder='', suffix='', last_modified=None): s3_client = client('s3', current_app.config['AWS_REGION']) paginator = s3_client.get_paginator('list_objects_v2') diff --git a/app/celery/nightly_tasks.py b/app/celery/nightly_tasks.py index 060834550..154fec694 100644 --- a/app/celery/nightly_tasks.py +++ b/app/celery/nightly_tasks.py @@ -214,16 +214,6 @@ def delete_inbound_sms(): raise -@notify_celery.task(name="remove_transformed_dvla_files") -@cronitor("remove_transformed_dvla_files") -@statsd(namespace="tasks") -def remove_transformed_dvla_files(): - jobs = dao_get_jobs_older_than_data_retention(notification_types=[LETTER_TYPE]) - for job in jobs: - s3.remove_transformed_dvla_file(job.id) - current_app.logger.info("Transformed dvla file for job {} has been removed from s3.".format(job.id)) - - # TODO: remove me, i'm not being run by anything @notify_celery.task(name="delete_dvla_response_files") @statsd(namespace="tasks") diff --git a/app/config.py b/app/config.py index c63e3ce1a..c53ef01e7 100644 --- a/app/config.py +++ b/app/config.py @@ -263,11 +263,6 @@ class Config(object): 'schedule': crontab(hour=2, minute=0), 'options': {'queue': QueueNames.PERIODIC} }, - 'remove_transformed_dvla_files': { - 'task': 'remove_transformed_dvla_files', - 'schedule': crontab(hour=3, minute=40), - 'options': {'queue': QueueNames.PERIODIC} - }, 'remove_sms_email_jobs': { 'task': 'remove_sms_email_jobs', 'schedule': crontab(hour=4, minute=0), @@ -275,7 +270,7 @@ class Config(object): }, 'remove_letter_jobs': { 'task': 'remove_letter_jobs', - 'schedule': crontab(hour=4, minute=20), # this has to run AFTER remove_transformed_dvla_files + 'schedule': crontab(hour=4, minute=20), # since we mark jobs as archived 'options': {'queue': QueueNames.PERIODIC}, }, @@ -330,11 +325,6 @@ class Config(object): SIMULATED_SMS_NUMBERS = ('+447700900000', '+447700900111', '+447700900222') - DVLA_BUCKETS = { - 'job': '{}-dvla-file-per-job'.format(os.getenv('NOTIFY_ENVIRONMENT')), - 'notification': '{}-dvla-letter-api-files'.format(os.getenv('NOTIFY_ENVIRONMENT')) - } - FREE_SMS_TIER_FRAGMENT_COUNT = 250000 SMS_INBOUND_WHITELIST = json.loads(os.environ.get('SMS_INBOUND_WHITELIST', '[]')) diff --git a/tests/app/aws/test_s3.py b/tests/app/aws/test_s3.py index f122ebdc5..a15474eaa 100644 --- a/tests/app/aws/test_s3.py +++ b/tests/app/aws/test_s3.py @@ -2,7 +2,6 @@ from unittest.mock import call from datetime import datetime, timedelta import pytest import pytz -from flask import current_app from freezegun import freeze_time @@ -10,7 +9,6 @@ from app.aws.s3 import ( get_s3_bucket_objects, get_s3_file, filter_s3_bucket_objects_within_date_range, - remove_transformed_dvla_file, get_list_of_files_by_suffix, ) from tests.app.conftest import datetime_in_past @@ -34,18 +32,6 @@ def test_get_s3_file_makes_correct_call(notify_api, mocker): ) -def test_remove_transformed_dvla_file_makes_correct_call(notify_api, mocker): - s3_mock = mocker.patch('app.aws.s3.get_s3_object') - fake_uuid = '5fbf9799-6b9b-4dbb-9a4e-74a939f3bb49' - - remove_transformed_dvla_file(fake_uuid) - - s3_mock.assert_has_calls([ - call(current_app.config['DVLA_BUCKETS']['job'], '{}-dvla-job.text'.format(fake_uuid)), - call().delete() - ]) - - def test_get_s3_bucket_objects_make_correct_pagination_call(notify_api, mocker): paginator_mock = mocker.patch('app.aws.s3.client') diff --git a/tests/app/celery/test_nightly_tasks.py b/tests/app/celery/test_nightly_tasks.py index e1c86c1ba..1713f8f49 100644 --- a/tests/app/celery/test_nightly_tasks.py +++ b/tests/app/celery/test_nightly_tasks.py @@ -1,5 +1,4 @@ from datetime import datetime, timedelta, date -from functools import partial from unittest.mock import call, patch, PropertyMock import pytest @@ -18,7 +17,6 @@ from app.celery.nightly_tasks import ( raise_alert_if_letter_notifications_still_sending, remove_letter_csv_files, remove_sms_email_csv_files, - remove_transformed_dvla_files, s3, send_daily_performance_platform_stats, send_total_sent_notifications_to_performance_platform, @@ -291,63 +289,6 @@ def test_should_call_delete_inbound_sms(notify_api, mocker): assert nightly_tasks.delete_inbound_sms_older_than_retention.call_count == 1 -@freeze_time('2017-01-01 10:00:00') -def test_remove_dvla_transformed_files_removes_expected_files(mocker, sample_service): - mocker.patch('app.celery.nightly_tasks.s3.remove_transformed_dvla_file') - - letter_template = create_template(service=sample_service, template_type=LETTER_TYPE) - - job = partial(create_job, template=letter_template) - - seven_days_ago = datetime.utcnow() - timedelta(days=7) - just_under_seven_days = seven_days_ago + timedelta(seconds=1) - just_over_seven_days = seven_days_ago - timedelta(seconds=1) - eight_days_ago = seven_days_ago - timedelta(days=1) - nine_days_ago = eight_days_ago - timedelta(days=1) - ten_days_ago = nine_days_ago - timedelta(days=1) - just_under_nine_days = nine_days_ago + timedelta(seconds=1) - just_over_nine_days = nine_days_ago - timedelta(seconds=1) - just_over_ten_days = ten_days_ago - timedelta(seconds=1) - - job(created_at=just_under_seven_days) - job(created_at=just_over_seven_days) - job_to_delete_1 = job(created_at=eight_days_ago) - job_to_delete_2 = job(created_at=nine_days_ago) - job_to_delete_3 = job(created_at=just_under_nine_days) - job_to_delete_4 = job(created_at=just_over_nine_days) - job(created_at=just_over_ten_days) - remove_transformed_dvla_files() - - s3.remove_transformed_dvla_file.assert_has_calls([ - call(job_to_delete_1.id), - call(job_to_delete_2.id), - call(job_to_delete_3.id), - call(job_to_delete_4.id), - ], any_order=True) - - -def test_remove_dvla_transformed_files_does_not_remove_files(mocker, sample_service): - mocker.patch('app.celery.nightly_tasks.s3.remove_transformed_dvla_file') - - letter_template = create_template(service=sample_service, template_type=LETTER_TYPE) - - job = partial(create_job, template=letter_template) - - yesterday = datetime.utcnow() - timedelta(days=1) - six_days_ago = datetime.utcnow() - timedelta(days=6) - seven_days_ago = six_days_ago - timedelta(days=1) - just_over_nine_days = seven_days_ago - timedelta(days=2, seconds=1) - - job(created_at=yesterday) - job(created_at=six_days_ago) - job(created_at=seven_days_ago) - job(created_at=just_over_nine_days) - - remove_transformed_dvla_files() - - s3.remove_transformed_dvla_file.assert_has_calls([]) - - @freeze_time("2016-01-01 11:00:00") def test_delete_dvla_response_files_older_than_seven_days_removes_old_files(notify_api, mocker): AFTER_SEVEN_DAYS = datetime_in_past(days=8)