From 01cf3dfb9d1c032916fdfe362ec3ac07cf9fa5ab Mon Sep 17 00:00:00 2001 From: Ryan Ahearn Date: Wed, 30 Nov 2022 11:08:55 -0500 Subject: [PATCH] Completely remove code to switch sms providers on slow delivery --- app/celery/scheduled_tasks.py | 24 ---------- app/config.py | 5 --- tests/app/celery/test_scheduled_tasks.py | 56 ------------------------ 3 files changed, 85 deletions(-) diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 15e5cff3d..28298aa75 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -32,13 +32,11 @@ from app.dao.jobs_dao import ( from app.dao.notifications_dao import ( dao_old_letters_with_created_status, dao_precompiled_letters_still_pending_virus_check, - is_delivery_slow_for_providers, letters_missing_from_sending_bucket, notifications_not_yet_sent, ) from app.dao.provider_details_dao import ( dao_adjust_provider_priority_back_to_resting_points, - dao_reduce_sms_provider_priority, ) from app.dao.services_dao import ( dao_find_services_sending_to_tv_numbers, @@ -95,28 +93,6 @@ def delete_invitations(): raise -@notify_celery.task(name='switch-current-sms-provider-on-slow-delivery') -def switch_current_sms_provider_on_slow_delivery(): - """ - Reduce provider's priority if at least 30% of notifications took more than four minutes to be delivered - in the last ten minutes. If both providers are slow, don't do anything. If we changed the providers in the - last ten minutes, then don't update them again either. - """ - slow_delivery_notifications = is_delivery_slow_for_providers( - threshold=0.3, - created_at=datetime.utcnow() - timedelta(minutes=10), - delivery_time=timedelta(minutes=4), - ) - - # only adjust if some values are true and some are false - ie, don't adjust if all providers are fast or - # all providers are slow - if len(set(slow_delivery_notifications.values())) != 1: - for provider_name, is_slow in slow_delivery_notifications.items(): - if is_slow: - current_app.logger.warning('Slow delivery notifications detected for provider {}'.format(provider_name)) - dao_reduce_sms_provider_priority(provider_name, time_threshold=timedelta(minutes=10)) - - @notify_celery.task(name='tend-providers-back-to-middle') def tend_providers_back_to_middle(): dao_adjust_provider_priority_back_to_resting_points() diff --git a/app/config.py b/app/config.py index 9b294938d..d1085e2f0 100644 --- a/app/config.py +++ b/app/config.py @@ -215,11 +215,6 @@ class Config(object): 'schedule': timedelta(minutes=66), 'options': {'queue': QueueNames.PERIODIC} }, - # 'switch-current-sms-provider-on-slow-delivery': { - # 'task': 'switch-current-sms-provider-on-slow-delivery', - # 'schedule': crontab(), # Every minute - # 'options': {'queue': QueueNames.PERIODIC} - # }, 'check-job-status': { 'task': 'check-job-status', 'schedule': crontab(), diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 3b5c51ce8..55298af2c 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -20,11 +20,9 @@ from app.celery.scheduled_tasks import ( delete_verify_codes, replay_created_notifications, run_scheduled_jobs, - switch_current_sms_provider_on_slow_delivery, ) from app.config import QueueNames, TaskNames, Test from app.dao.jobs_dao import dao_get_job_by_id -from app.dao.provider_details_dao import get_provider_details_by_identifier from app.models import ( JOB_STATUS_ERROR, JOB_STATUS_FINISHED, @@ -37,19 +35,6 @@ from tests.app import load_example_csv from tests.app.db import create_job, create_notification, create_template -def _create_slow_delivery_notification(template, provider='mmg'): - now = datetime.utcnow() - five_minutes_from_now = now + timedelta(minutes=5) - - create_notification( - template=template, - status='delivered', - sent_by=provider, - updated_at=five_minutes_from_now, - sent_at=now, - ) - - def test_should_call_delete_codes_on_delete_verify_codes_task(notify_db_session, mocker): mocker.patch('app.celery.scheduled_tasks.delete_codes_older_created_more_than_a_day_ago') delete_verify_codes() @@ -98,47 +83,6 @@ def test_should_update_all_scheduled_jobs_and_put_on_queue(sample_template, mock ]) -@freeze_time('2017-05-01 14:00:00') -def test_switch_current_sms_provider_on_slow_delivery_switches_when_one_provider_is_slow( - mocker, - restore_provider_details, -): - is_slow_dict = {'mmg': False, 'firetext': True} - mock_is_slow = mocker.patch('app.celery.scheduled_tasks.is_delivery_slow_for_providers', return_value=is_slow_dict) - mock_reduce = mocker.patch('app.celery.scheduled_tasks.dao_reduce_sms_provider_priority') - # updated_at times are older than the 10 minute window - get_provider_details_by_identifier('mmg').updated_at = datetime(2017, 5, 1, 13, 49) - get_provider_details_by_identifier('firetext').updated_at = None - - switch_current_sms_provider_on_slow_delivery() - - mock_is_slow.assert_called_once_with( - threshold=0.3, - created_at=datetime(2017, 5, 1, 13, 50), - delivery_time=timedelta(minutes=4) - ) - mock_reduce.assert_called_once_with('firetext', time_threshold=timedelta(minutes=10)) - - -@freeze_time('2017-05-01 14:00:00') -@pytest.mark.parametrize('is_slow_dict', [ - {'mmg': False, 'firetext': False}, - {'mmg': True, 'firetext': True}, -]) -def test_switch_current_sms_provider_on_slow_delivery_does_nothing_if_no_need( - mocker, - restore_provider_details, - is_slow_dict -): - mocker.patch('app.celery.scheduled_tasks.is_delivery_slow_for_providers', return_value=is_slow_dict) - mock_reduce = mocker.patch('app.celery.scheduled_tasks.dao_reduce_sms_provider_priority') - get_provider_details_by_identifier('mmg').updated_at = datetime(2017, 5, 1, 13, 51) - - switch_current_sms_provider_on_slow_delivery() - - assert mock_reduce.called is False - - def test_check_job_status_task_calls_process_incomplete_jobs(mocker, sample_template): mock_celery = mocker.patch('app.celery.tasks.process_incomplete_jobs.apply_async') job = create_job(template=sample_template, notification_count=3,