clean up usage of dates/datetimes in performance platform tasks

* call variables unambiguous things like `start_time` or `bst_date` to
  reduce risk of passing in the wrong thing
* simplify the count_dict object - remove nested dict and start_date
  fields as superfluous
* use static datetime objects in tests rather than calculating them
  each time
This commit is contained in:
Leo Hemsted
2019-04-02 11:49:20 +01:00
parent 4abbb7137a
commit 3739d9055d
7 changed files with 64 additions and 79 deletions

View File

@@ -37,6 +37,7 @@ from app.models import (
) )
from app.performance_platform import total_sent_notifications, processing_time from app.performance_platform import total_sent_notifications, processing_time
from app.cronitor import cronitor from app.cronitor import cronitor
from app.utils import get_london_midnight_in_utc
@notify_celery.task(name="remove_sms_email_jobs") @notify_celery.task(name="remove_sms_email_jobs")
@@ -152,36 +153,37 @@ def timeout_notifications():
def send_daily_performance_platform_stats(): def send_daily_performance_platform_stats():
if performance_platform_client.active: if performance_platform_client.active:
yesterday = datetime.utcnow() - timedelta(days=1) yesterday = datetime.utcnow() - timedelta(days=1)
send_total_sent_notifications_to_performance_platform(yesterday) send_total_sent_notifications_to_performance_platform(bst_date=yesterday.date())
processing_time.send_processing_time_to_performance_platform() processing_time.send_processing_time_to_performance_platform()
def send_total_sent_notifications_to_performance_platform(day): def send_total_sent_notifications_to_performance_platform(bst_date):
count_dict = total_sent_notifications.get_total_sent_notifications_for_day(day.date()) count_dict = total_sent_notifications.get_total_sent_notifications_for_day(bst_date)
email_sent_count = count_dict.get('email').get('count') start_time = get_london_midnight_in_utc(bst_date)
sms_sent_count = count_dict.get('sms').get('count')
letter_sent_count = count_dict.get('letter').get('count') email_sent_count = count_dict['email']
start_date = count_dict.get('start_date') sms_sent_count = count_dict['sms']
letter_sent_count = count_dict['letter']
current_app.logger.info( current_app.logger.info(
"Attempting to update Performance Platform for {} with {} emails, {} text messages and {} letters" "Attempting to update Performance Platform for {} with {} emails, {} text messages and {} letters"
.format(start_date, email_sent_count, sms_sent_count, letter_sent_count) .format(bst_date, email_sent_count, sms_sent_count, letter_sent_count)
) )
total_sent_notifications.send_total_notifications_sent_for_day_stats( total_sent_notifications.send_total_notifications_sent_for_day_stats(
start_date, start_time,
'sms', 'sms',
sms_sent_count sms_sent_count
) )
total_sent_notifications.send_total_notifications_sent_for_day_stats( total_sent_notifications.send_total_notifications_sent_for_day_stats(
start_date, start_time,
'email', 'email',
email_sent_count email_sent_count
) )
total_sent_notifications.send_total_notifications_sent_for_day_stats( total_sent_notifications.send_total_notifications_sent_for_day_stats(
start_date, start_time,
'letter', 'letter',
letter_sent_count letter_sent_count
) )

View File

@@ -46,17 +46,17 @@ class PerformancePlatformClient:
resp.raise_for_status() resp.raise_for_status()
@staticmethod @staticmethod
def format_payload(*, dataset, date, group_name, group_value, count, period='day'): def format_payload(*, dataset, start_time, group_name, group_value, count, period='day'):
""" """
:param dataset - the name of the overall graph, as referred to in the endpoint. :param dataset - the name of the overall graph, as referred to in the endpoint.
:param date - the date we're sending stats for :param start_time - UTC midnight of the day we're sending stats for
:param group_name - the name of the individual groups of data, eg "channel" or "status" :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 group_value - the value of the group, eg "sms" or "email" for group_name=channel
:param count - the actual numeric value to send :param count - the actual numeric value to send
:param period - the period that this data covers - "day", "week", "month", "quarter". :param period - the period that this data covers - "day", "week", "month", "quarter".
""" """
payload = { payload = {
'_timestamp': convert_utc_to_bst(date).isoformat(), '_timestamp': convert_utc_to_bst(start_time).isoformat(),
'service': 'govuk-notify', 'service': 'govuk-notify',
'dataType': dataset, 'dataType': dataset,
'period': period, 'period': period,

View File

@@ -9,29 +9,29 @@ from app import performance_platform_client
def send_processing_time_to_performance_platform(): def send_processing_time_to_performance_platform():
today = datetime.utcnow() today = datetime.utcnow()
start_date = get_midnight_for_day_before(today) start_time = get_midnight_for_day_before(today)
end_date = get_london_midnight_in_utc(today) end_time = get_london_midnight_in_utc(today)
send_processing_time_for_start_and_end(start_date, end_date) send_processing_time_for_start_and_end(start_time, end_time)
def send_processing_time_for_start_and_end(start_date, end_date): def send_processing_time_for_start_and_end(start_time, end_time):
result = dao_get_total_notifications_sent_per_day_for_performance_platform(start_date, end_date) result = dao_get_total_notifications_sent_per_day_for_performance_platform(start_time, end_time)
current_app.logger.info( current_app.logger.info(
'Sending processing-time to performance platform for date {}. Total: {}, under 10 secs {}'.format( 'Sending processing-time to performance platform for date {}. Total: {}, under 10 secs {}'.format(
start_date, result.messages_total, result.messages_within_10_secs start_time, result.messages_total, result.messages_within_10_secs
) )
) )
send_processing_time_data(start_date, 'messages-total', result.messages_total) send_processing_time_data(start_time, 'messages-total', result.messages_total)
send_processing_time_data(start_date, 'messages-within-10-secs', result.messages_within_10_secs) send_processing_time_data(start_time, 'messages-within-10-secs', result.messages_within_10_secs)
def send_processing_time_data(date, status, count): def send_processing_time_data(start_time, status, count):
payload = performance_platform_client.format_payload( payload = performance_platform_client.format_payload(
dataset='processing-time', dataset='processing-time',
date=date, start_time=start_time,
group_name='status', group_name='status',
group_value=status, group_value=status,
count=count count=count

View File

@@ -1,12 +1,11 @@
from app import performance_platform_client from app import performance_platform_client
from app.dao.fact_notification_status_dao import get_total_sent_notifications_for_day_and_type from app.dao.fact_notification_status_dao import get_total_sent_notifications_for_day_and_type
from app.utils import get_london_midnight_in_utc
def send_total_notifications_sent_for_day_stats(date, notification_type, count): def send_total_notifications_sent_for_day_stats(start_time, notification_type, count):
payload = performance_platform_client.format_payload( payload = performance_platform_client.format_payload(
dataset='notifications', dataset='notifications',
date=date, start_time=start_time,
group_name='channel', group_name='channel',
group_value=notification_type, group_value=notification_type,
count=count count=count
@@ -20,17 +19,8 @@ def get_total_sent_notifications_for_day(day):
sms_count = get_total_sent_notifications_for_day_and_type(day, 'sms') sms_count = get_total_sent_notifications_for_day_and_type(day, 'sms')
letter_count = get_total_sent_notifications_for_day_and_type(day, 'letter') letter_count = get_total_sent_notifications_for_day_and_type(day, 'letter')
start_date = get_london_midnight_in_utc(day)
return { return {
"start_date": start_date, "email": email_count,
"email": { "sms": sms_count,
"count": email_count "letter": letter_count,
},
"sms": {
"count": sms_count
},
"letter": {
"count": letter_count
},
} }

View File

@@ -1,4 +1,4 @@
from datetime import datetime, timedelta from datetime import datetime, timedelta, date
from functools import partial from functools import partial
from unittest.mock import call, patch, PropertyMock from unittest.mock import call, patch, PropertyMock
@@ -251,7 +251,7 @@ def test_send_daily_performance_stats_calls_does_not_send_if_inactive(client, mo
assert send_mock.call_count == 0 assert send_mock.call_count == 0
@freeze_time("2016-01-11 12:30:00") @freeze_time("2016-06-11 02:00:00")
def test_send_total_sent_notifications_to_performance_platform_calls_with_correct_totals( def test_send_total_sent_notifications_to_performance_platform_calls_with_correct_totals(
notify_db_session, notify_db_session,
sample_template, sample_template,
@@ -261,18 +261,14 @@ def test_send_total_sent_notifications_to_performance_platform_calls_with_correc
perf_mock = mocker.patch( perf_mock = mocker.patch(
'app.celery.nightly_tasks.total_sent_notifications.send_total_notifications_sent_for_day_stats') # noqa 'app.celery.nightly_tasks.total_sent_notifications.send_total_notifications_sent_for_day_stats') # noqa
today = datetime.utcnow() today = date(2016, 6, 11)
create_ft_notification_status(bst_date=today, notification_type='sms', service=sample_template.service, create_ft_notification_status(bst_date=today, template=sample_template)
template=sample_template) create_ft_notification_status(bst_date=today, template=sample_email_template)
create_ft_notification_status(bst_date=today, notification_type='email', service=sample_email_template.service,
template=sample_email_template)
# Create some notifications for the day before # Create some notifications for the day before
yesterday = today - timedelta(days=1) yesterday = date(2016, 6, 10)
create_ft_notification_status(bst_date=yesterday, notification_type='sms', service=sample_template.service, create_ft_notification_status(bst_date=yesterday, template=sample_template, count=2)
template=sample_template, count=2) create_ft_notification_status(bst_date=yesterday, template=sample_email_template, count=3)
create_ft_notification_status(bst_date=yesterday, notification_type='email', service=sample_email_template.service,
template=sample_email_template, count=3)
with patch.object( with patch.object(
PerformancePlatformClient, PerformancePlatformClient,
@@ -283,9 +279,9 @@ def test_send_total_sent_notifications_to_performance_platform_calls_with_correc
send_total_sent_notifications_to_performance_platform(yesterday) send_total_sent_notifications_to_performance_platform(yesterday)
perf_mock.assert_has_calls([ perf_mock.assert_has_calls([
call(yesterday.date(), 'sms', 2), call(datetime(2016, 6, 9, 23, 0), 'sms', 2),
call(yesterday.date(), 'email', 3), call(datetime(2016, 6, 9, 23, 0), 'email', 3),
call(yesterday.date(), 'letter', 0) call(datetime(2016, 6, 9, 23, 0), 'letter', 0)
]) ])

View File

@@ -29,7 +29,7 @@ def test_send_processing_time_to_performance_platform_creates_correct_call_to_pe
send_stats = mocker.patch('app.performance_platform.total_sent_notifications.performance_platform_client.send_stats_to_performance_platform') # noqa send_stats = mocker.patch('app.performance_platform.total_sent_notifications.performance_platform_client.send_stats_to_performance_platform') # noqa
send_processing_time_data( send_processing_time_data(
date=datetime(2016, 10, 15, 23, 0, 0), start_time=datetime(2016, 10, 15, 23, 0, 0),
status='foo', status='foo',
count=142 count=142
) )

View File

@@ -1,4 +1,6 @@
from datetime import datetime, timedelta from datetime import datetime, date
from freezegun import freeze_time
from app.performance_platform.total_sent_notifications import ( from app.performance_platform.total_sent_notifications import (
send_total_notifications_sent_for_day_stats, send_total_notifications_sent_for_day_stats,
@@ -12,7 +14,7 @@ def test_send_total_notifications_sent_for_day_stats_stats_creates_correct_call(
send_stats = mocker.patch('app.performance_platform.total_sent_notifications.performance_platform_client.send_stats_to_performance_platform') # noqa send_stats = mocker.patch('app.performance_platform.total_sent_notifications.performance_platform_client.send_stats_to_performance_platform') # noqa
send_total_notifications_sent_for_day_stats( send_total_notifications_sent_for_day_stats(
date=datetime(2016, 10, 15, 23, 0, 0), start_time=datetime(2016, 10, 15, 23, 0, 0),
notification_type='sms', notification_type='sms',
count=142 count=142
) )
@@ -30,34 +32,29 @@ def test_send_total_notifications_sent_for_day_stats_stats_creates_correct_call(
assert request_args['_id'] == expected_base64_id assert request_args['_id'] == expected_base64_id
@freeze_time('2018-06-10 01:00')
def test_get_total_sent_notifications_yesterday_returns_expected_totals_dict(sample_service): def test_get_total_sent_notifications_yesterday_returns_expected_totals_dict(sample_service):
sms = create_template(sample_service, template_type='sms') sms = create_template(sample_service, template_type='sms')
email = create_template(sample_service, template_type='email') email = create_template(sample_service, template_type='email')
letter = create_template(sample_service, template_type='letter') letter = create_template(sample_service, template_type='letter')
today = datetime.utcnow().date() today = date(2018, 6, 10)
yesterday = today - timedelta(days=1) yesterday = date(2018, 6, 9)
create_ft_notification_status(bst_date=today, notification_type='sms',
service=sms.service, template=sms)
create_ft_notification_status(bst_date=today, notification_type='email',
service=email.service, template=email)
create_ft_notification_status(bst_date=today, notification_type='letter',
service=letter.service, template=letter)
create_ft_notification_status(bst_date=yesterday, notification_type='sms', # todays is excluded
service=sms.service, template=sms, count=2) create_ft_notification_status(bst_date=today, template=sms)
create_ft_notification_status(bst_date=yesterday, notification_type='email', create_ft_notification_status(bst_date=today, template=email)
service=email.service, template=email, count=3) create_ft_notification_status(bst_date=today, template=letter)
create_ft_notification_status(bst_date=yesterday, notification_type='letter',
service=letter.service, template=letter, count=1) # yesterdays is included
create_ft_notification_status(bst_date=yesterday, template=sms, count=2)
create_ft_notification_status(bst_date=yesterday, template=email, count=3)
create_ft_notification_status(bst_date=yesterday, template=letter, count=1)
total_count_dict = get_total_sent_notifications_for_day(yesterday) total_count_dict = get_total_sent_notifications_for_day(yesterday)
assert total_count_dict["email"] == {"count": 3} assert total_count_dict == {
assert total_count_dict["sms"] == {"count": 2} "email": 3,
assert total_count_dict["letter"] == {"count": 1} "sms": 2,
"letter": 1
# Should return a time around midnight depending on timezones }
expected_start = datetime.combine(yesterday, datetime.min.time())
time_diff = abs(expected_start - total_count_dict["start_date"])
assert time_diff <= timedelta(minutes=60)