randomly choose from providers based on priority

todo: make sure if they don't add up to 100 we do something sensible,
especially if they're both 0.

app/celery/scheduled_tasks.py

@@ -116,14 +116,13 @@ def switch_current_sms_provider_on_slow_delivery():
     if current_provider.updated_at > datetime.utcnow() - timedelta(minutes=10):
         current_app.logger.info("Slow delivery notifications provider switched less than 10 minutes ago.")
         return
-    slow_delivery_notifications = is_delivery_slow_for_provider(
+    slow_delivery_notifications = is_delivery_slow_for_providers(
-        provider=current_provider.identifier,
         threshold=0.3,
         created_at=datetime.utcnow() - timedelta(minutes=10),
         delivery_time=timedelta(minutes=4),
     )
 
-    if slow_delivery_notifications:
+    if slow_delivery_notifications[current_provider]:
         current_app.logger.warning(
             'Slow delivery notifications detected for provider {}'.format(
                 current_provider.identifier

app/dao/notifications_dao.py

@@ -1,5 +1,7 @@
 import functools
 import string
+from itertools import groupby
+from operator import attrgetter
 from datetime import (
     datetime,
     timedelta,
@@ -15,7 +17,7 @@ from notifications_utils.recipients import (
 )
 from notifications_utils.statsd_decorators import statsd
 from notifications_utils.timezones import convert_bst_to_utc, convert_utc_to_bst
-from sqlalchemy import (desc, func, asc)
+from sqlalchemy import (desc, func, asc, and_)
 from sqlalchemy.orm import joinedload
 from sqlalchemy.orm.exc import NoResultFound
 from sqlalchemy.sql import functions
@@ -31,6 +33,7 @@ from app.letters.utils import get_letter_pdf_filename
 from app.models import (
     Notification,
     NotificationHistory,
+    ProviderDetails,
     ScheduledNotification,
     KEY_TYPE_NORMAL,
     KEY_TYPE_TEST,
@@ -486,40 +489,63 @@ def dao_timeout_notifications(timeout_period_in_seconds):
     return technical_failure_notifications, temporary_failure_notifications
 
 
-def is_delivery_slow_for_provider(
+def is_delivery_slow_for_providers(
         created_at,
-        provider,
         threshold,
         delivery_time,
 ):
-    count = db.session.query(
+    """
+    Returns a dict of providers and whether they are currently slow or not. eg:
+    {
+        'mmg': True,
+        'firetext': False
+    }
+    """
+    slow_notification_counts = db.session.query(
+        ProviderDetails.identifier,
         case(
             [(
                 Notification.status == NOTIFICATION_DELIVERED,
                 (Notification.updated_at - Notification.sent_at) >= delivery_time
             )],
             else_=(datetime.utcnow() - Notification.sent_at) >= delivery_time
-        ).label("slow"), func.count()
+        ).label("slow"),
-
+        func.count().label('count')
+    ).select_from(
+        ProviderDetails
+    ).outerjoin(
+        Notification, and_(
+            Notification.sent_by == ProviderDetails.identifier,
+            Notification.created_at >= created_at,
+            Notification.sent_at.isnot(None),
+            Notification.status.in_([NOTIFICATION_DELIVERED, NOTIFICATION_PENDING, NOTIFICATION_SENDING]),
+            Notification.key_type != KEY_TYPE_TEST
+        )
     ).filter(
-        Notification.created_at >= created_at,
+        ProviderDetails.notification_type == 'sms',
-        Notification.sent_at.isnot(None),
+        ProviderDetails.active
-        Notification.status.in_([NOTIFICATION_DELIVERED, NOTIFICATION_PENDING, NOTIFICATION_SENDING]),
+    ).order_by(
-        Notification.sent_by == provider,
+        ProviderDetails.identifier
-        Notification.key_type != KEY_TYPE_TEST
+    ).group_by(
-    ).group_by("slow").all()
+        ProviderDetails.identifier,
+        "slow"
+    )
+    print(slow_notification_counts)
+    print([x for x in slow_notification_counts])
 
-    counts = {c[0]: c[1] for c in count}
+    slow_providers = {}
-    total_notifications = sum(counts.values())
+    for provider, rows in groupby(slow_notification_counts, key=attrgetter('identifier')):
-    slow_notifications = counts.get(True, 0)
+        rows = list(rows)
+        total_notifications = sum(row.count for row in rows)
+        slow_notifications = sum(row.count for row in rows if row.slow)
+
+        slow_providers[provider] = (slow_notifications / total_notifications >= threshold)
 
-    if total_notifications:
         current_app.logger.info("Slow delivery notifications count for provider {}: {} out of {}. Ratio {}".format(
             provider, slow_notifications, total_notifications, slow_notifications / total_notifications
         ))
-        return slow_notifications / total_notifications >= threshold
+
-    else:
+    return slow_providers
-        return False
 
 
 @statsd(namespace="dao")

app/delivery/send_to_providers.py

@@ -1,3 +1,4 @@
+import random
 from urllib import parse
 from datetime import datetime
 
@@ -39,7 +40,7 @@ def send_sms_to_provider(notification):
         return
 
     if notification.status == 'created':
-        provider = provider_to_use(SMS_TYPE, notification.id, notification.international)
+        provider = provider_to_use(SMS_TYPE, notification.international)
 
         template_model = dao_get_template_by_id(notification.template_id, notification.template_version)
 
@@ -81,7 +82,7 @@ def send_email_to_provider(notification):
         technical_failure(notification=notification)
         return
     if notification.status == 'created':
-        provider = provider_to_use(EMAIL_TYPE, notification.id)
+        provider = provider_to_use(EMAIL_TYPE)
 
         template_dict = dao_get_template_by_id(notification.template_id, notification.template_version).__dict__
 
@@ -128,18 +129,20 @@ def update_notification_to_sending(notification, provider):
     dao_update_notification(notification)
 
 
-def provider_to_use(notification_type, notification_id, international=False):
+def provider_to_use(notification_type, international=False):
-    active_providers_in_order = [
+    active_providers = [
         p for p in get_provider_details_by_notification_type(notification_type, international) if p.active
     ]
 
-    if not active_providers_in_order:
+    if not active_providers:
         current_app.logger.error(
-            "{} {} failed as no active providers".format(notification_type, notification_id)
+            "{} failed as no active providers".format(notification_type)
         )
         raise Exception("No active {} providers".format(notification_type))
 
-    return clients.get_client_by_name_and_type(active_providers_in_order[0].identifier, notification_type)
+    chosen_provider = random.choices(active_providers, weights=[p.priority for p in active_providers])[0]
+
+    return clients.get_client_by_name_and_type(chosen_provider.identifier, notification_type)
 
 
 def get_logo_url(base_url, logo_file):

tests/app/dao/notification_dao/test_notification_dao.py

@@ -26,7 +26,7 @@ from app.dao.notifications_dao import (
     get_notification_with_personalisation,
     get_notifications_for_job,
     get_notifications_for_service,
-    is_delivery_slow_for_provider,
+    is_delivery_slow_for_providers,
     set_scheduled_notification_to_processed,
     update_notification_status_by_id,
     update_notification_status_by_reference,
@@ -957,7 +957,7 @@ def test_should_exclude_test_key_notifications_by_default(
     ]
 )
 @freeze_time("2018-12-04 12:00:00.000000")
-def test_is_delivery_slow_for_provider(
+def test_is_delivery_slow_for_providers(
     notify_db_session,
     sample_template,
     normal_sending,
@@ -992,7 +992,11 @@ def test_is_delivery_slow_for_provider(
     for _ in range(slow_delivered):
         slow_notification(status='delivered')
 
-    assert is_delivery_slow_for_provider(datetime.utcnow(), "mmg", threshold, timedelta(minutes=4)) is expected_result
+    result = is_delivery_slow_for_providers(datetime.utcnow(), threshold, timedelta(minutes=4))
+    assert result == {
+        'firetext': False,
+        'mmg': expected_result
+    }
 
 
 @pytest.mark.parametrize("options,expected_result", [
@@ -1008,22 +1012,23 @@ def test_is_delivery_slow_for_provider(
 
 ])
 @freeze_time("2018-12-04 12:00:00.000000")
-def test_delivery_is_delivery_slow_for_provider_filters_out_notifications_it_should_not_count(
+def test_delivery_is_delivery_slow_for_providers_filters_out_notifications_it_should_not_count(
     notify_db_session,
     sample_template,
     options,
     expected_result
 ):
-    create_notification_with = {
+    create_slow_notification_with = {
         "template": sample_template,
         "sent_at": datetime.now() - timedelta(minutes=5),
         "updated_at": datetime.now(),
     }
-    create_notification_with.update(options)
+    create_slow_notification_with.update(options)
     create_notification(
-        **create_notification_with
+        **create_slow_notification_with
     )
-    assert is_delivery_slow_for_provider(datetime.utcnow(), "mmg", 0.1, timedelta(minutes=4)) is expected_result
+    result = is_delivery_slow_for_providers(datetime.utcnow(), 0.1, timedelta(minutes=4))
+    assert result['mmg'] == expected_result
 
 
 def test_dao_get_notifications_by_to_field(sample_template):

tests/app/delivery/test_send_to_providers.py

@@ -40,7 +40,7 @@ def test_should_return_highest_priority_active_provider(restore_provider_details
     first = providers[0]
     second = providers[1]
 
-    assert send_to_providers.provider_to_use('sms', '1234').name == first.identifier
+    assert send_to_providers.provider_to_use('sms').name == first.identifier
 
     first.priority = 20
     second.priority = 10
@@ -48,7 +48,7 @@ def test_should_return_highest_priority_active_provider(restore_provider_details
     provider_details_dao.dao_update_provider_details(first)
     provider_details_dao.dao_update_provider_details(second)
 
-    assert send_to_providers.provider_to_use('sms', '1234').name == second.identifier
+    assert send_to_providers.provider_to_use('sms').name == second.identifier
 
     first.priority = 10
     first.active = False
@@ -57,12 +57,12 @@ def test_should_return_highest_priority_active_provider(restore_provider_details
     provider_details_dao.dao_update_provider_details(first)
     provider_details_dao.dao_update_provider_details(second)
 
-    assert send_to_providers.provider_to_use('sms', '1234').name == second.identifier
+    assert send_to_providers.provider_to_use('sms').name == second.identifier
 
     first.active = True
     provider_details_dao.dao_update_provider_details(first)
 
-    assert send_to_providers.provider_to_use('sms', '1234').name == first.identifier
+    assert send_to_providers.provider_to_use('sms').name == first.identifier
 
 
 def test_should_send_personalised_template_to_correct_sms_provider_and_persist(