From e85b621cbc81f794428ef35912cec5e2b87029cc Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 24 Aug 2017 17:08:39 +0100 Subject: [PATCH] make perf platform client handle more stuff sensibly specifically, all of the performance platform specific data layout now happens in performance_platform_client.py - stuff like setting the _timestamp, period etc, and the perf platform-specific nomenclature is all handled there. --- app/celery/scheduled_tasks.py | 20 +++++----- .../performance_platform_client.py | 38 ++++++++++++++++--- .../total_sent_notifications.py | 24 +++++------- manifest-api-base.yml | 2 +- manifest-delivery-base.yml | 1 + tests/app/celery/test_scheduled_tasks.py | 6 +-- .../app/clients/test_performance_platform.py | 12 +++--- .../test_total_sent_notifications.py | 8 ++-- 8 files changed, 64 insertions(+), 47 deletions(-) diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index c7bda0f74..68295ff7d 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -87,7 +87,7 @@ def delete_verify_codes(): current_app.logger.info( "Delete job started {} finished {} deleted {} verify codes".format(start, datetime.utcnow(), deleted) ) - except SQLAlchemyError as e: + except SQLAlchemyError: current_app.logger.exception("Failed to delete verify codes") raise @@ -106,7 +106,7 @@ def delete_sms_notifications_older_than_seven_days(): deleted ) ) - except SQLAlchemyError as e: + except SQLAlchemyError: current_app.logger.exception("Failed to delete sms notifications") raise @@ -125,7 +125,7 @@ def delete_email_notifications_older_than_seven_days(): deleted ) ) - except SQLAlchemyError as e: + except SQLAlchemyError: current_app.logger.exception("Failed to delete sms notifications") raise @@ -144,7 +144,7 @@ def delete_letter_notifications_older_than_seven_days(): deleted ) ) - except SQLAlchemyError as e: + except SQLAlchemyError: current_app.logger.exception("Failed to delete sms notifications") raise @@ -158,7 +158,7 @@ def delete_invitations(): current_app.logger.info( "Delete job started {} finished {} deleted {} invitations".format(start, datetime.utcnow(), deleted) ) - except SQLAlchemyError as e: + except SQLAlchemyError: current_app.logger.exception("Failed to delete invitations") raise @@ -189,15 +189,13 @@ def send_daily_performance_platform_stats(): total_sent_notifications.send_total_notifications_sent_for_day_stats( start_date, 'sms', - sms_sent_count, - 'day' + sms_sent_count ) total_sent_notifications.send_total_notifications_sent_for_day_stats( start_date, 'email', - email_sent_count, - 'day' + email_sent_count ) @@ -254,7 +252,7 @@ def delete_inbound_sms_older_than_seven_days(): deleted ) ) - except SQLAlchemyError as e: + except SQLAlchemyError: current_app.logger.exception("Failed to delete inbound sms notifications") raise @@ -289,7 +287,7 @@ def delete_dvla_response_files_older_than_seven_days(): len(older_than_seven_days) ) ) - except SQLAlchemyError as e: + except SQLAlchemyError: current_app.logger.exception("Failed to delete dvla response files") raise diff --git a/app/clients/performance_platform/performance_platform_client.py b/app/clients/performance_platform/performance_platform_client.py index b0f400a8c..d04b22659 100644 --- a/app/clients/performance_platform/performance_platform_client.py +++ b/app/clients/performance_platform/performance_platform_client.py @@ -4,6 +4,8 @@ import json from flask import current_app import requests +from app.utils import convert_utc_to_bst + class PerformancePlatformClient: @@ -17,15 +19,15 @@ class PerformancePlatformClient: self.performance_platform_url = app.config.get('PERFORMANCE_PLATFORM_URL') self.performance_platform_endpoints = app.config.get('PERFORMANCE_PLATFORM_ENDPOINTS') - def send_stats_to_performance_platform(self, dataset, payload): + def send_stats_to_performance_platform(self, payload): if self.active: - bearer_token = self.performance_platform_endpoints[dataset] + bearer_token = self.performance_platform_endpoints[payload['dataType']] headers = { 'Content-Type': "application/json", 'Authorization': 'Bearer {}'.format(bearer_token) } resp = requests.post( - self.performance_platform_url + dataset, + self.performance_platform_url + payload['dataType'], json=payload, headers=headers ) @@ -44,13 +46,37 @@ class PerformancePlatformClient: resp.raise_for_status() @staticmethod - def add_id_to_payload(payload): + def format_payload(*, dataset, date, group_name, group_value, count, period='day'): + """ + :param dataset - the name of the overall graph, as referred to in the endpoint. + :param date - the date we're sending stats for + :param group_name - the name of the individual groups of data, eg "channel" or "status" + :param group_value - the value of the group, eg "sms" or "email" for group_name=channel + :param count - the actual numeric value to send + :param period - the period that this data covers - "day", "week", "month", "quarter". + """ + payload = { + '_timestamp': convert_utc_to_bst(date).isoformat(), + 'service': 'govuk-notify', + 'dataType': dataset, + 'period': period, + 'count': count, + group_name: group_value, + } + payload['_id'] = PerformancePlatformClient.generate_payload_id(payload, group_name) + return payload + + @staticmethod + def generate_payload_id(payload, group_name): + """ + group_name is the name of the group - eg "channel" or "status" + """ payload_string = '{}{}{}{}{}'.format( payload['_timestamp'], payload['service'], - payload['channel'], + payload[group_name], payload['dataType'], payload['period'] ) _id = base64.b64encode(payload_string.encode('utf-8')) - payload.update({'_id': _id.decode('utf-8')}) + return _id.decode('utf-8') diff --git a/app/performance_platform/total_sent_notifications.py b/app/performance_platform/total_sent_notifications.py index 9f455f113..4ad57171e 100644 --- a/app/performance_platform/total_sent_notifications.py +++ b/app/performance_platform/total_sent_notifications.py @@ -4,27 +4,21 @@ from app import performance_platform_client from app.dao.notifications_dao import get_total_sent_notifications_in_date_range from app.utils import ( get_london_midnight_in_utc, - get_midnight_for_day_before, - convert_utc_to_bst, + get_midnight_for_day_before ) -def send_total_notifications_sent_for_day_stats(date, channel, count, period): - payload = { - '_timestamp': convert_utc_to_bst(date).isoformat(), - 'service': 'govuk-notify', - 'channel': channel, - 'count': count, - 'dataType': 'notifications', - 'period': period - } - performance_platform_client.add_id_to_payload(payload) - - performance_platform_client.send_stats_to_performance_platform( +def send_total_notifications_sent_for_day_stats(date, notification_type, count): + payload = performance_platform_client.format_payload( dataset='notifications', - payload=payload + date=date, + group_name='channel', + group_value=notification_type, + count=count ) + performance_platform_client.send_stats_to_performance_platform(payload) + def get_total_sent_notifications_yesterday(): today = datetime.utcnow() diff --git a/manifest-api-base.yml b/manifest-api-base.yml index 9eb4ceb70..19c198d21 100644 --- a/manifest-api-base.yml +++ b/manifest-api-base.yml @@ -1,4 +1,4 @@ -#--- +--- buildpack: python_buildpack command: scripts/run_app_paas.sh gunicorn -w 5 -b 0.0.0.0:$PORT wsgi diff --git a/manifest-delivery-base.yml b/manifest-delivery-base.yml index 78053a729..6ceeaf201 100644 --- a/manifest-delivery-base.yml +++ b/manifest-delivery-base.yml @@ -11,6 +11,7 @@ services: - firetext - hosted-graphite - redis + - performance-platform instances: 1 memory: 1G diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 6064f1c79..2a8f2160e 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -270,7 +270,7 @@ def test_will_remove_csv_files_for_jobs_older_than_seven_days( ] -def test_send_daily_performance_stats_calls_does_not_send_if_inactive(mocker): +def test_send_daily_performance_stats_calls_does_not_send_if_inactive(client, mocker): send_mock = mocker.patch('app.celery.scheduled_tasks.total_sent_notifications.send_total_notifications_sent_for_day_stats') # noqa with patch.object( @@ -322,8 +322,8 @@ def test_send_daily_performance_stats_calls_with_correct_totals( send_daily_performance_platform_stats() perf_mock.assert_has_calls([ - call(get_london_midnight_in_utc(yesterday), 'sms', 2, 'day'), - call(get_london_midnight_in_utc(yesterday), 'email', 3, 'day') + call(get_london_midnight_in_utc(yesterday), 'sms', 2), + call(get_london_midnight_in_utc(yesterday), 'email', 3) ]) diff --git a/tests/app/clients/test_performance_platform.py b/tests/app/clients/test_performance_platform.py index aba87b9e7..c55a004cd 100644 --- a/tests/app/clients/test_performance_platform.py +++ b/tests/app/clients/test_performance_platform.py @@ -6,7 +6,7 @@ from app.clients.performance_platform.performance_platform_client import Perform @pytest.fixture(scope='function') -def perf_client(mocker): +def perf_client(client, mocker): perf_client = PerformancePlatformClient() current_app = mocker.Mock(config={ 'PERFORMANCE_PLATFORM_ENABLED': True, @@ -24,15 +24,15 @@ def test_should_not_call_if_not_enabled(perf_client): with requests_mock.Mocker() as request_mock: request_mock.post('https://performance-platform-url/foo', json={}, status_code=200) perf_client._active = False - perf_client.send_stats_to_performance_platform(dataset='foo', payload={}) + perf_client.send_stats_to_performance_platform({'dataType': 'foo'}) assert request_mock.called is False -def test_should_call_if_enabled(perf_client): +def test_should_call_datatype_endpoint_if_enabled(perf_client): with requests_mock.Mocker() as request_mock: request_mock.post('https://performance-platform-url/foo', json={}, status_code=200) - perf_client.send_stats_to_performance_platform(dataset='foo', payload={}) + perf_client.send_stats_to_performance_platform({'dataType': 'foo'}) assert request_mock.call_count == 1 assert request_mock.last_request.method == 'POST' @@ -46,7 +46,7 @@ def test_should_use_correct_token(perf_client, dataset, token): with requests_mock.Mocker() as request_mock: request_mock.post('https://performance-platform-url/foo', json={}, status_code=200) request_mock.post('https://performance-platform-url/bar', json={}, status_code=200) - perf_client.send_stats_to_performance_platform(dataset=dataset, payload={}) + perf_client.send_stats_to_performance_platform({'dataType': dataset}) assert request_mock.call_count == 1 assert request_mock.last_request.headers.get('authorization') == 'Bearer {}'.format(token) @@ -55,4 +55,4 @@ def test_should_use_correct_token(perf_client, dataset, token): def test_should_raise_for_status(perf_client): with pytest.raises(requests.HTTPError), requests_mock.Mocker() as request_mock: request_mock.post('https://performance-platform-url/foo', json={}, status_code=403) - perf_client.send_stats_to_performance_platform(dataset='foo', payload={}) + perf_client.send_stats_to_performance_platform({'dataType': 'foo'}) diff --git a/tests/app/performance_platform/test_total_sent_notifications.py b/tests/app/performance_platform/test_total_sent_notifications.py index 12125d9c0..e4fe597d0 100644 --- a/tests/app/performance_platform/test_total_sent_notifications.py +++ b/tests/app/performance_platform/test_total_sent_notifications.py @@ -19,15 +19,13 @@ def test_send_total_notifications_sent_for_day_stats_stats_creates_correct_call( send_total_notifications_sent_for_day_stats( date=datetime(2016, 10, 15, 23, 0, 0), - channel='sms', - count=142, - period='day' + notification_type='sms', + count=142 ) assert send_stats.call_count == 1 - assert send_stats.call_args[1]['dataset'] == 'notifications' - request_args = send_stats.call_args[1]['payload'] + request_args = send_stats.call_args[0][0] assert request_args['dataType'] == 'notifications' assert request_args['service'] == 'govuk-notify' assert request_args['period'] == 'day'