From b9db5d9c48071e7f7386622c0198faff6ebf39ea Mon Sep 17 00:00:00 2001 From: Jumana B Date: Mon, 6 Jan 2025 09:04:41 -0500 Subject: [PATCH 1/3] Revert "Task/fix stats (#2401)" (#2402) This reverts commit 59edacbedc832823b6e778b40876e2d06d786c22. --- app/service/rest.py | 4 ++-- tests/app/service/test_statistics_rest.py | 8 -------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/app/service/rest.py b/app/service/rest.py index 197194691b..0156490291 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -649,13 +649,13 @@ def get_monthly_notification_stats(service_id): data = statistics.create_empty_monthly_notification_status_stats_dict(year) stats = fetch_notification_status_for_service_by_month(start_date, end_date, service_id) - # statistics.add_monthly_notification_status_stats(data, stats) + statistics.add_monthly_notification_status_stats(data, stats) now = datetime.utcnow() # TODO FF_ANNUAL_LIMIT removal if not current_app.config["FF_ANNUAL_LIMIT"] and end_date > now: todays_deltas = fetch_notification_status_for_service_for_day(convert_utc_to_local_timezone(now), service_id=service_id) - # statistics.add_monthly_notification_status_stats(data, todays_deltas) + statistics.add_monthly_notification_status_stats(data, todays_deltas) return jsonify(data=data) diff --git a/tests/app/service/test_statistics_rest.py b/tests/app/service/test_statistics_rest.py index 2f7444282f..a978979248 100644 --- a/tests/app/service/test_statistics_rest.py +++ b/tests/app/service/test_statistics_rest.py @@ -132,7 +132,6 @@ def test_get_service_notification_statistics_with_unknown_service(admin_request) } -@pytest.mark.skip(reason="This test is failing due to an incident fix.") @pytest.mark.parametrize( "kwargs, expected_json", [ @@ -147,7 +146,6 @@ def test_get_monthly_notification_stats_returns_errors(admin_request, sample_ser assert response == expected_json -@pytest.mark.skip(reason="This test is failing due to an incident fix.") def test_get_monthly_notification_stats_returns_404_if_no_service(admin_request): response = admin_request.get( "service.get_monthly_notification_stats", @@ -157,7 +155,6 @@ def test_get_monthly_notification_stats_returns_404_if_no_service(admin_request) assert response == {"message": "No result found", "result": "error"} -@pytest.mark.skip(reason="This test is failing due to an incident fix.") def test_get_monthly_notification_stats_returns_empty_stats_with_correct_dates(admin_request, sample_service): response = admin_request.get( "service.get_monthly_notification_stats", @@ -185,7 +182,6 @@ def test_get_monthly_notification_stats_returns_empty_stats_with_correct_dates(a assert val == {"sms": {}, "email": {}, "letter": {}} -@pytest.mark.skip(reason="This test is failing due to an incident fix.") def test_get_monthly_notification_stats_returns_stats(admin_request, sample_service): sms_t1 = create_template(sample_service) sms_t2 = create_template(sample_service) @@ -225,7 +221,6 @@ def test_get_monthly_notification_stats_returns_stats(admin_request, sample_serv } -@pytest.mark.skip(reason="This test is failing due to an incident fix.") @freeze_time("2016-06-05 00:00:00") # This test assumes the local timezone is EST def test_get_monthly_notification_stats_combines_todays_data_and_historic_stats(admin_request, notify_api, sample_template): @@ -267,7 +262,6 @@ def test_get_monthly_notification_stats_combines_todays_data_and_historic_stats( # This test assumes the local timezone is EST -@pytest.mark.skip(reason="This test is failing due to an incident fix.") def test_get_monthly_notification_stats_ignores_test_keys(admin_request, sample_service): create_ft_notification_status(datetime(2016, 6, 1), service=sample_service, key_type=KEY_TYPE_NORMAL, count=1) create_ft_notification_status(datetime(2016, 6, 1), service=sample_service, key_type=KEY_TYPE_TEAM, count=2) @@ -283,7 +277,6 @@ def test_get_monthly_notification_stats_ignores_test_keys(admin_request, sample_ # This test assumes the local timezone is EST -@pytest.mark.skip(reason="This test is failing due to an incident fix.") def test_get_monthly_notification_stats_checks_dates(admin_request, sample_service): t = create_template(sample_service) create_ft_notification_status(datetime(2016, 3, 31), template=t, notification_status="created") @@ -303,7 +296,6 @@ def test_get_monthly_notification_stats_checks_dates(admin_request, sample_servi assert response["data"]["2017-03"]["sms"] == {"delivered": 1} -@pytest.mark.skip(reason="This test is failing due to an incident fix.") def test_get_monthly_notification_stats_only_gets_for_one_service(admin_request, notify_api, notify_db_session): services = [create_service(), create_service(service_name="2")] From 5f644943434164ee1b042dcf1c0999beebeded11 Mon Sep 17 00:00:00 2001 From: Jumana B Date: Mon, 6 Jan 2025 10:07:54 -0500 Subject: [PATCH 2/3] Remove date conversion (#2398) * Remove date conversion * additiona places it was left * fix * fix --- app/celery/process_pinpoint_receipts_tasks.py | 5 ++--- app/celery/process_ses_receipts_tasks.py | 5 ++--- app/celery/process_sns_receipts_tasks.py | 5 ++--- app/dao/date_util.py | 2 +- app/dao/fact_notification_status_dao.py | 7 +++++-- app/service/rest.py | 10 ++++++---- 6 files changed, 18 insertions(+), 16 deletions(-) diff --git a/app/celery/process_pinpoint_receipts_tasks.py b/app/celery/process_pinpoint_receipts_tasks.py index 5edd796169..82df455877 100644 --- a/app/celery/process_pinpoint_receipts_tasks.py +++ b/app/celery/process_pinpoint_receipts_tasks.py @@ -1,9 +1,8 @@ -from datetime import datetime +from datetime import datetime, timezone from typing import Union from flask import current_app, json from notifications_utils.statsd_decorators import statsd -from notifications_utils.timezones import convert_utc_to_local_timezone from sqlalchemy.orm.exc import NoResultFound from app import annual_limit_client, notify_celery, statsd_client @@ -120,7 +119,7 @@ def process_pinpoint_results(self, response): if not annual_limit_client.was_seeded_today(service_id): annual_limit_client.set_seeded_at(service_id) notifications_to_seed = fetch_notification_status_for_service_for_day( - convert_utc_to_local_timezone(datetime.utcnow()), + datetime.now(timezone.utc), service_id=service_id, ) annual_limit_client.seed_annual_limit_notifications( diff --git a/app/celery/process_ses_receipts_tasks.py b/app/celery/process_ses_receipts_tasks.py index 5d2c29b133..e147a0c42f 100644 --- a/app/celery/process_ses_receipts_tasks.py +++ b/app/celery/process_ses_receipts_tasks.py @@ -1,8 +1,7 @@ -from datetime import datetime +from datetime import datetime, timezone from flask import current_app, json from notifications_utils.statsd_decorators import statsd -from notifications_utils.timezones import convert_utc_to_local_timezone from sqlalchemy.orm.exc import NoResultFound from app import annual_limit_client, bounce_rate_client, notify_celery, statsd_client @@ -97,7 +96,7 @@ def process_ses_results(self, response): # noqa: C901 if not annual_limit_client.was_seeded_today(service_id): annual_limit_client.set_seeded_at(service_id) notifications_to_seed = fetch_notification_status_for_service_for_day( - convert_utc_to_local_timezone(datetime.utcnow()), + datetime.now(timezone.utc), service_id=service_id, ) annual_limit_client.seed_annual_limit_notifications( diff --git a/app/celery/process_sns_receipts_tasks.py b/app/celery/process_sns_receipts_tasks.py index 326784f3e3..7b0fe36166 100644 --- a/app/celery/process_sns_receipts_tasks.py +++ b/app/celery/process_sns_receipts_tasks.py @@ -1,8 +1,7 @@ -from datetime import datetime +from datetime import datetime, timezone from flask import current_app, json from notifications_utils.statsd_decorators import statsd -from notifications_utils.timezones import convert_utc_to_local_timezone from sqlalchemy.orm.exc import NoResultFound from app import annual_limit_client, notify_celery, statsd_client @@ -77,7 +76,7 @@ def process_sns_results(self, response): if not annual_limit_client.was_seeded_today(service_id): annual_limit_client.set_seeded_at(service_id) notifications_to_seed = fetch_notification_status_for_service_for_day( - convert_utc_to_local_timezone(datetime.utcnow()), + datetime.now(timezone.utc), service_id=service_id, ) annual_limit_client.seed_annual_limit_notifications( diff --git a/app/dao/date_util.py b/app/dao/date_util.py index c53684d5e2..94cdcba978 100644 --- a/app/dao/date_util.py +++ b/app/dao/date_util.py @@ -103,7 +103,7 @@ def utc_midnight_n_days_ago(number_of_days): """ Returns utc midnight a number of days ago. """ - return get_midnight(datetime.utcnow() - timedelta(days=number_of_days)) + return get_midnight(datetime.now(timezone.utc) - timedelta(days=number_of_days)) def get_query_date_based_on_retention_period(retention_period): diff --git a/app/dao/fact_notification_status_dao.py b/app/dao/fact_notification_status_dao.py index 2ee37d6f73..80ed2700fc 100644 --- a/app/dao/fact_notification_status_dao.py +++ b/app/dao/fact_notification_status_dao.py @@ -247,6 +247,9 @@ def fetch_notification_stats_for_trial_services(): def fetch_notification_status_for_service_for_day(bst_day, service_id): + # Fetch data from bst_day 00:00:00 to bst_day 23:59:59 + # bst_dat is currently in UTC and the return is in UTC + bst_day = bst_day.replace(hour=0, minute=0, second=0) return ( db.session.query( # return current month as a datetime so the data has the same shape as the ft_notification_status query @@ -256,8 +259,8 @@ def fetch_notification_status_for_service_for_day(bst_day, service_id): func.count().label("count"), ) .filter( - Notification.created_at >= get_local_timezone_midnight_in_utc(bst_day), - Notification.created_at < get_local_timezone_midnight_in_utc(bst_day + timedelta(days=1)), + Notification.created_at >= bst_day, + Notification.created_at < bst_day + timedelta(days=1), Notification.service_id == service_id, Notification.key_type != KEY_TYPE_TEST, ) diff --git a/app/service/rest.py b/app/service/rest.py index 0156490291..1934dabb7d 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -1,5 +1,5 @@ import itertools -from datetime import datetime +from datetime import datetime, timezone from flask import Blueprint, current_app, jsonify, request from notifications_utils.clients.redis import ( @@ -651,10 +651,12 @@ def get_monthly_notification_stats(service_id): stats = fetch_notification_status_for_service_by_month(start_date, end_date, service_id) statistics.add_monthly_notification_status_stats(data, stats) - now = datetime.utcnow() + now = datetime.now(timezone.utc) + # end_date doesn't have tzinfo, so we need to remove it from now + end_date_now = now.replace(tzinfo=None) # TODO FF_ANNUAL_LIMIT removal - if not current_app.config["FF_ANNUAL_LIMIT"] and end_date > now: - todays_deltas = fetch_notification_status_for_service_for_day(convert_utc_to_local_timezone(now), service_id=service_id) + if not current_app.config["FF_ANNUAL_LIMIT"] and end_date > end_date_now: + todays_deltas = fetch_notification_status_for_service_for_day(now, service_id=service_id) statistics.add_monthly_notification_status_stats(data, todays_deltas) return jsonify(data=data) From 8b565717c924e59c506f46478d23b756e10e3075 Mon Sep 17 00:00:00 2001 From: Stephen McMurtry Date: Mon, 6 Jan 2025 11:05:51 -0500 Subject: [PATCH 3/3] updated the bootstrap file so it works with poetry (#2399) --- scripts/bootstrap.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/bootstrap.sh b/scripts/bootstrap.sh index c23371d0e3..f1ab694ca6 100755 --- a/scripts/bootstrap.sh +++ b/scripts/bootstrap.sh @@ -7,4 +7,4 @@ make generate-version-file poetry install --only test # Upgrade databases -flask db upgrade +poetry run flask db upgrade