From 5677d805c35b94fde390a229e92ca9cc2215a1c9 Mon Sep 17 00:00:00 2001 From: Thomas Koopman Date: Wed, 11 Dec 2024 11:20:35 +0100 Subject: [PATCH 01/20] Add field to Challenge for budget warning thresholds --- ...ge_budget_warning_thresholds_percentage.py | 35 +++++++++++++++++++ app/grandchallenge/challenges/models.py | 18 ++++++++++ 2 files changed, 53 insertions(+) create mode 100644 app/grandchallenge/challenges/migrations/0046_challenge_budget_warning_thresholds_percentage.py diff --git a/app/grandchallenge/challenges/migrations/0046_challenge_budget_warning_thresholds_percentage.py b/app/grandchallenge/challenges/migrations/0046_challenge_budget_warning_thresholds_percentage.py new file mode 100644 index 000000000..58bfa869d --- /dev/null +++ b/app/grandchallenge/challenges/migrations/0046_challenge_budget_warning_thresholds_percentage.py @@ -0,0 +1,35 @@ +# Generated by Django 4.2.17 on 2024-12-11 10:16 + +from django.db import migrations, models + +import grandchallenge.challenges.models +import grandchallenge.core.validators + + +class Migration(migrations.Migration): + dependencies = [ + ( + "challenges", + "0045_challengerequest_algorithm_maximum_settable_memory_gb_and_more", + ), + ] + + operations = [ + migrations.AddField( + model_name="challenge", + name="percent_budget_consumed_warning_thresholds", + field=models.JSONField( + default=grandchallenge.challenges.models.get_default_percent_budget_consumed_warning_thresholds, + validators=[ + grandchallenge.core.validators.JSONValidator( + schema={ + "$schema": "http://json-schema.org/draft-07/schema", + "items": {"type": "integer"}, + "type": "array", + "uniqueItems": True, + } + ) + ], + ), + ), + ] diff --git a/app/grandchallenge/challenges/models.py b/app/grandchallenge/challenges/models.py index 2b7fdc49c..8b46a60ba 100644 --- a/app/grandchallenge/challenges/models.py +++ b/app/grandchallenge/challenges/models.py @@ -211,6 +211,10 @@ class Meta: abstract = True +def get_default_percent_budget_consumed_warning_thresholds(): + return [70, 90, 100] + + class Challenge(ChallengeBase): created = models.DateTimeField(auto_now_add=True) modified = models.DateTimeField(auto_now=True) @@ -376,6 +380,20 @@ class Challenge(ChallengeBase): help_text="This email will be listed as the contact email for the challenge and will be visible to all users of Grand Challenge.", ) + percent_budget_consumed_warning_thresholds = models.JSONField( + default=get_default_percent_budget_consumed_warning_thresholds, + validators=[ + JSONValidator( + schema={ + "$schema": "http://json-schema.org/draft-07/schema", + "type": "array", + "items": {"type": "integer"}, + "uniqueItems": True, + } + ) + ], + ) + accumulated_compute_cost_in_cents = deprecate_field( models.IntegerField(default=0, blank=True) ) From 933d7caa0a340e570be896af303265c2d1660e26 Mon Sep 17 00:00:00 2001 From: Thomas Koopman Date: Thu, 12 Dec 2024 19:08:46 +0100 Subject: [PATCH 02/20] Send email alert on budget exceed during compute cost update task --- app/grandchallenge/challenges/emails.py | 21 +++++++++ app/grandchallenge/challenges/models.py | 6 +-- app/grandchallenge/challenges/tasks.py | 27 +++++++++++- app/tests/challenges_tests/test_tasks.py | 55 ++++++++++++++++++++++-- 4 files changed, 102 insertions(+), 7 deletions(-) diff --git a/app/grandchallenge/challenges/emails.py b/app/grandchallenge/challenges/emails.py index ed3f4571f..3251f310c 100644 --- a/app/grandchallenge/challenges/emails.py +++ b/app/grandchallenge/challenges/emails.py @@ -91,3 +91,24 @@ def send_challenge_status_update_email(challengerequest, challenge=None): recipients=[challengerequest.creator], subscription_type=EmailSubscriptionTypes.SYSTEM, ) + + +def send_email_percent_budget_consumed_alert(challenge, warning_threshold): + send_standard_email_batch( + site=Site.objects.get_current(), + subject=format_html( + "[{challenge_name}] Challenge {warning_threshold}% Budget Consumed Alert", + challenge_name=challenge.short_name, + warning_threshold=warning_threshold, + ), + markdown_message=format_html( + "We would like to inform you that {percent_budget_consumed}% of the " + "compute budget for your challenge has been used.", + percent_budget_consumed=challenge.percent_budget_consumed, + ), + recipients=[ + *challenge.get_admins(), + *get_user_model().objects.filter(is_staff=True, is_active=True), + ], + subscription_type=EmailSubscriptionTypes.SYSTEM, + ) diff --git a/app/grandchallenge/challenges/models.py b/app/grandchallenge/challenges/models.py index 8b46a60ba..8a129af13 100644 --- a/app/grandchallenge/challenges/models.py +++ b/app/grandchallenge/challenges/models.py @@ -50,7 +50,7 @@ GPUTypeChoices, get_default_gpu_type_choices, ) -from grandchallenge.core.models import UUIDModel +from grandchallenge.core.models import FieldChangeMixin, UUIDModel from grandchallenge.core.storage import ( get_banner_path, get_logo_path, @@ -215,7 +215,7 @@ def get_default_percent_budget_consumed_warning_thresholds(): return [70, 90, 100] -class Challenge(ChallengeBase): +class Challenge(ChallengeBase, FieldChangeMixin): created = models.DateTimeField(auto_now_add=True) modified = models.DateTimeField(auto_now=True) description = models.CharField( @@ -725,7 +725,7 @@ def should_be_open_but_is_over_budget(self): for phase in self.phase_set.all() ) - @cached_property + @property def percent_budget_consumed(self): if self.approved_compute_costs_euro_millicents: return int( diff --git a/app/grandchallenge/challenges/tasks.py b/app/grandchallenge/challenges/tasks.py index 4429bd52b..1468cedce 100644 --- a/app/grandchallenge/challenges/tasks.py +++ b/app/grandchallenge/challenges/tasks.py @@ -5,6 +5,9 @@ annotate_compute_costs_and_storage_size, annotate_job_duration_and_compute_costs, ) +from grandchallenge.challenges.emails import ( + send_email_percent_budget_consumed_alert, +) from grandchallenge.challenges.models import Challenge from grandchallenge.core.celery import acks_late_2xlarge_task from grandchallenge.evaluation.models import Evaluation, Phase @@ -55,12 +58,34 @@ def update_challenge_results_cache(): ) +def send_alert_if_budget_consumed_warning_threshold_exceeded(challenge): + if ( + challenge.has_changed("compute_cost_euro_millicents") + and challenge.approved_compute_costs_euro_millicents + ): + for threshold in sorted( + challenge.percent_budget_consumed_warning_thresholds, reverse=True + ): + if ( + challenge.initial_value("compute_cost_euro_millicents") + < challenge.approved_compute_costs_euro_millicents + * threshold + / 100 + <= challenge.compute_cost_euro_millicents + ): + send_email_percent_budget_consumed_alert(challenge, threshold) + break + + @acks_late_2xlarge_task def update_compute_costs_and_storage_size(): challenges = Challenge.objects.all() - for challenge in challenges: + for challenge in challenges.with_available_compute(): annotate_compute_costs_and_storage_size(challenge=challenge) + send_alert_if_budget_consumed_warning_threshold_exceeded( + challenge=challenge + ) Challenge.objects.bulk_update( challenges, diff --git a/app/tests/challenges_tests/test_tasks.py b/app/tests/challenges_tests/test_tasks.py index 195ea0943..a10fb0563 100644 --- a/app/tests/challenges_tests/test_tasks.py +++ b/app/tests/challenges_tests/test_tasks.py @@ -1,9 +1,19 @@ import pytest +from django.core import mail from grandchallenge.challenges.models import Challenge, ChallengeRequest -from grandchallenge.challenges.tasks import update_challenge_results_cache -from tests.evaluation_tests.factories import EvaluationFactory -from tests.factories import ChallengeFactory, ChallengeRequestFactory +from grandchallenge.challenges.tasks import ( + update_challenge_results_cache, + update_compute_costs_and_storage_size, +) +from grandchallenge.invoices.models import PaymentStatusChoices +from tests.evaluation_tests.factories import EvaluationFactory, PhaseFactory +from tests.factories import ( + ChallengeFactory, + ChallengeRequestFactory, + UserFactory, +) +from tests.invoices_tests.factories import InvoiceFactory @pytest.mark.django_db @@ -121,3 +131,42 @@ def test_challenge_request_budget_calculation(settings): + challenge_request.budget["Total phase 2"] + challenge_request.budget["Docker storage cost"] ) + + +@pytest.mark.django_db +def test_challenge_budget_alert_email(): + challenge = ChallengeFactory( + short_name="test", + ) + challenge_admin = UserFactory() + challenge.add_admin(challenge_admin) + staff_user = UserFactory(is_staff=True) + InvoiceFactory( + challenge=challenge, + support_costs_euros=0, + compute_costs_euros=10, + storage_costs_euros=0, + payment_status=PaymentStatusChoices.PAID, + ) + phase = PhaseFactory(challenge=challenge) + EvaluationFactory( + submission__phase=phase, + compute_cost_euro_millicents=800000, + time_limit=60, + ) + update_compute_costs_and_storage_size() + assert len(mail.outbox) == 3 + recipients = [r for m in mail.outbox for r in m.to] + assert recipients == [ + challenge.creator.email, + challenge_admin.email, + staff_user.email, + ] + assert ( + mail.outbox[0].subject + == "[testserver] [test] Challenge 70% Budget Consumed Alert" + ) + assert ( + "We would like to inform you that 80% of the compute budget for your challenge has been used." + in mail.outbox[0].body + ) From f4795cf8249fa6ba2830779a44c40ba84724f705 Mon Sep 17 00:00:00 2001 From: Thomas Koopman Date: Mon, 16 Dec 2024 16:55:23 +0100 Subject: [PATCH 03/20] Shorten email subject Co-authored-by: Anne Mickan --- app/grandchallenge/challenges/emails.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/grandchallenge/challenges/emails.py b/app/grandchallenge/challenges/emails.py index 3251f310c..d42336987 100644 --- a/app/grandchallenge/challenges/emails.py +++ b/app/grandchallenge/challenges/emails.py @@ -97,7 +97,7 @@ def send_email_percent_budget_consumed_alert(challenge, warning_threshold): send_standard_email_batch( site=Site.objects.get_current(), subject=format_html( - "[{challenge_name}] Challenge {warning_threshold}% Budget Consumed Alert", + "[{challenge_name}] {warning_threshold}% Budget Consumed Alert", challenge_name=challenge.short_name, warning_threshold=warning_threshold, ), From cb613391165241e4eb734b6b1420010c8bd23dcb Mon Sep 17 00:00:00 2001 From: Thomas Koopman Date: Mon, 16 Dec 2024 15:28:25 +0100 Subject: [PATCH 04/20] Revert change to cached_property --- app/grandchallenge/challenges/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/grandchallenge/challenges/models.py b/app/grandchallenge/challenges/models.py index 8a129af13..4f88e1387 100644 --- a/app/grandchallenge/challenges/models.py +++ b/app/grandchallenge/challenges/models.py @@ -725,7 +725,7 @@ def should_be_open_but_is_over_budget(self): for phase in self.phase_set.all() ) - @property + @cached_property def percent_budget_consumed(self): if self.approved_compute_costs_euro_millicents: return int( From e4820d7c6068b6f1a588e631e08b621a81e5b4b3 Mon Sep 17 00:00:00 2001 From: Thomas Koopman Date: Mon, 16 Dec 2024 15:52:37 +0100 Subject: [PATCH 05/20] Use warning threshold instead of actual consumed in budget alert email --- app/grandchallenge/challenges/emails.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/grandchallenge/challenges/emails.py b/app/grandchallenge/challenges/emails.py index d42336987..c79ed64a4 100644 --- a/app/grandchallenge/challenges/emails.py +++ b/app/grandchallenge/challenges/emails.py @@ -102,9 +102,9 @@ def send_email_percent_budget_consumed_alert(challenge, warning_threshold): warning_threshold=warning_threshold, ), markdown_message=format_html( - "We would like to inform you that {percent_budget_consumed}% of the " + "We would like to inform you that more than {warning_threshold}% of the " "compute budget for your challenge has been used.", - percent_budget_consumed=challenge.percent_budget_consumed, + warning_threshold=warning_threshold, ), recipients=[ *challenge.get_admins(), From 3078ddbe9b3931131932d9da44c4f88f0e841d4a Mon Sep 17 00:00:00 2001 From: Thomas Koopman Date: Mon, 16 Dec 2024 20:47:49 +0100 Subject: [PATCH 06/20] Use mail_managers function to inform support staff --- app/grandchallenge/challenges/emails.py | 33 ++++++++++++++----------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/app/grandchallenge/challenges/emails.py b/app/grandchallenge/challenges/emails.py index c79ed64a4..059c6258f 100644 --- a/app/grandchallenge/challenges/emails.py +++ b/app/grandchallenge/challenges/emails.py @@ -1,5 +1,6 @@ from django.contrib.auth import get_user_model from django.contrib.sites.models import Site +from django.core.mail import mail_managers from django.template.loader import render_to_string from django.utils.html import format_html @@ -94,21 +95,25 @@ def send_challenge_status_update_email(challengerequest, challenge=None): def send_email_percent_budget_consumed_alert(challenge, warning_threshold): + subject = format_html( + "[{challenge_name}] {warning_threshold}% Budget Consumed Alert", + challenge_name=challenge.short_name, + warning_threshold=warning_threshold, + ) + message = format_html( + "We would like to inform you that more than {warning_threshold}% of the " + "compute budget for the {challenge_name} challenge has been used.", + challenge_name=challenge.short_name, + warning_threshold=warning_threshold, + ) send_standard_email_batch( site=Site.objects.get_current(), - subject=format_html( - "[{challenge_name}] {warning_threshold}% Budget Consumed Alert", - challenge_name=challenge.short_name, - warning_threshold=warning_threshold, - ), - markdown_message=format_html( - "We would like to inform you that more than {warning_threshold}% of the " - "compute budget for your challenge has been used.", - warning_threshold=warning_threshold, - ), - recipients=[ - *challenge.get_admins(), - *get_user_model().objects.filter(is_staff=True, is_active=True), - ], + subject=subject, + markdown_message=message, + recipients=[*challenge.get_admins()], subscription_type=EmailSubscriptionTypes.SYSTEM, ) + mail_managers( + subject=subject, + message=message, + ) From 3829308ca7081e5e2e7c60d2540720fbb3d32e3a Mon Sep 17 00:00:00 2001 From: Thomas Koopman Date: Mon, 16 Dec 2024 20:51:19 +0100 Subject: [PATCH 07/20] Include link to cost overview in budget alert email --- app/grandchallenge/challenges/emails.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/grandchallenge/challenges/emails.py b/app/grandchallenge/challenges/emails.py index 059c6258f..27be7b9d3 100644 --- a/app/grandchallenge/challenges/emails.py +++ b/app/grandchallenge/challenges/emails.py @@ -102,9 +102,14 @@ def send_email_percent_budget_consumed_alert(challenge, warning_threshold): ) message = format_html( "We would like to inform you that more than {warning_threshold}% of the " - "compute budget for the {challenge_name} challenge has been used.", + "compute budget for the {challenge_name} challenge has been used. " + "You can find an overview of the costs [here]({statistics_url}).", challenge_name=challenge.short_name, warning_threshold=warning_threshold, + statistics_url=reverse( + "pages:statistics", + kwargs={"challenge_short_name": challenge.short_name}, + ), ) send_standard_email_batch( site=Site.objects.get_current(), From c4f9c3c420295d6a1a1480ea098080f8da5e207d Mon Sep 17 00:00:00 2001 From: Thomas Koopman Date: Mon, 16 Dec 2024 20:58:27 +0100 Subject: [PATCH 08/20] Update the test after changes --- app/tests/challenges_tests/test_tasks.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/tests/challenges_tests/test_tasks.py b/app/tests/challenges_tests/test_tasks.py index a10fb0563..ddd0b8fc9 100644 --- a/app/tests/challenges_tests/test_tasks.py +++ b/app/tests/challenges_tests/test_tasks.py @@ -134,13 +134,14 @@ def test_challenge_request_budget_calculation(settings): @pytest.mark.django_db -def test_challenge_budget_alert_email(): +def test_challenge_budget_alert_email(settings): challenge = ChallengeFactory( short_name="test", ) challenge_admin = UserFactory() challenge.add_admin(challenge_admin) staff_user = UserFactory(is_staff=True) + settings.MANAGERS = [(staff_user.last_name, staff_user.email)] InvoiceFactory( challenge=challenge, support_costs_euros=0, @@ -164,9 +165,10 @@ def test_challenge_budget_alert_email(): ] assert ( mail.outbox[0].subject - == "[testserver] [test] Challenge 70% Budget Consumed Alert" + == "[testserver] [test] 70% Budget Consumed Alert" ) assert ( - "We would like to inform you that 80% of the compute budget for your challenge has been used." - in mail.outbox[0].body + "We would like to inform you that more than 70% of the compute budget for " + "the test challenge has been used. You can find an overview of the costs " + "[here](https://test.testserver/statistics/)." in mail.outbox[0].body ) From 346f4c8ed664561edc0d46c30ab7c26048fdea3b Mon Sep 17 00:00:00 2001 From: Thomas Koopman Date: Wed, 18 Dec 2024 11:39:22 +0100 Subject: [PATCH 09/20] Ensure single transaction --- app/grandchallenge/challenges/tasks.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/grandchallenge/challenges/tasks.py b/app/grandchallenge/challenges/tasks.py index 1468cedce..836f004e2 100644 --- a/app/grandchallenge/challenges/tasks.py +++ b/app/grandchallenge/challenges/tasks.py @@ -1,4 +1,5 @@ from django.contrib.auth import get_user_model +from django.db import transaction from django.db.models import Count, Max from grandchallenge.challenges.costs import ( @@ -78,6 +79,7 @@ def send_alert_if_budget_consumed_warning_threshold_exceeded(challenge): @acks_late_2xlarge_task +@transaction.atomic def update_compute_costs_and_storage_size(): challenges = Challenge.objects.all() From 4c99f1016f54c9782e2869274b87e15a329eafae Mon Sep 17 00:00:00 2001 From: Thomas Koopman Date: Wed, 18 Dec 2024 11:48:07 +0100 Subject: [PATCH 10/20] Make if-statement readable --- app/grandchallenge/challenges/tasks.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/app/grandchallenge/challenges/tasks.py b/app/grandchallenge/challenges/tasks.py index 836f004e2..f54227005 100644 --- a/app/grandchallenge/challenges/tasks.py +++ b/app/grandchallenge/challenges/tasks.py @@ -64,16 +64,19 @@ def send_alert_if_budget_consumed_warning_threshold_exceeded(challenge): challenge.has_changed("compute_cost_euro_millicents") and challenge.approved_compute_costs_euro_millicents ): - for threshold in sorted( + for percent_threshold in sorted( challenge.percent_budget_consumed_warning_thresholds, reverse=True ): - if ( - challenge.initial_value("compute_cost_euro_millicents") - < challenge.approved_compute_costs_euro_millicents - * threshold + previous_cost = challenge.initial_value( + "compute_cost_euro_millicents" + ) + threshold = ( + challenge.approved_compute_costs_euro_millicents + * percent_threshold / 100 - <= challenge.compute_cost_euro_millicents - ): + ) + current_cost = challenge.compute_cost_euro_millicents + if previous_cost < threshold <= current_cost: send_email_percent_budget_consumed_alert(challenge, threshold) break From d1341db30c0b8c77b47f46e0042043b90abd5b52 Mon Sep 17 00:00:00 2001 From: Thomas Koopman Date: Wed, 18 Dec 2024 11:54:24 +0100 Subject: [PATCH 11/20] fix --- app/grandchallenge/challenges/emails.py | 10 +++++----- app/grandchallenge/challenges/tasks.py | 4 +++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/app/grandchallenge/challenges/emails.py b/app/grandchallenge/challenges/emails.py index 27be7b9d3..446def997 100644 --- a/app/grandchallenge/challenges/emails.py +++ b/app/grandchallenge/challenges/emails.py @@ -94,18 +94,18 @@ def send_challenge_status_update_email(challengerequest, challenge=None): ) -def send_email_percent_budget_consumed_alert(challenge, warning_threshold): +def send_email_percent_budget_consumed_alert(challenge, percent_threshold): subject = format_html( - "[{challenge_name}] {warning_threshold}% Budget Consumed Alert", + "[{challenge_name}] {percent_threshold}% Budget Consumed Alert", challenge_name=challenge.short_name, - warning_threshold=warning_threshold, + percent_threshold=percent_threshold, ) message = format_html( - "We would like to inform you that more than {warning_threshold}% of the " + "We would like to inform you that more than {percent_threshold}% of the " "compute budget for the {challenge_name} challenge has been used. " "You can find an overview of the costs [here]({statistics_url}).", challenge_name=challenge.short_name, - warning_threshold=warning_threshold, + percent_threshold=percent_threshold, statistics_url=reverse( "pages:statistics", kwargs={"challenge_short_name": challenge.short_name}, diff --git a/app/grandchallenge/challenges/tasks.py b/app/grandchallenge/challenges/tasks.py index f54227005..b996cabc3 100644 --- a/app/grandchallenge/challenges/tasks.py +++ b/app/grandchallenge/challenges/tasks.py @@ -77,7 +77,9 @@ def send_alert_if_budget_consumed_warning_threshold_exceeded(challenge): ) current_cost = challenge.compute_cost_euro_millicents if previous_cost < threshold <= current_cost: - send_email_percent_budget_consumed_alert(challenge, threshold) + send_email_percent_budget_consumed_alert( + challenge, percent_threshold + ) break From 4bc5caaa19bccad9ce0ea9c9e7377fa566948e20 Mon Sep 17 00:00:00 2001 From: Thomas Koopman Date: Wed, 18 Dec 2024 13:10:22 +0100 Subject: [PATCH 12/20] Add test for no budget --- app/tests/challenges_tests/test_tasks.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/app/tests/challenges_tests/test_tasks.py b/app/tests/challenges_tests/test_tasks.py index ddd0b8fc9..709228b4d 100644 --- a/app/tests/challenges_tests/test_tasks.py +++ b/app/tests/challenges_tests/test_tasks.py @@ -172,3 +172,16 @@ def test_challenge_budget_alert_email(settings): "the test challenge has been used. You can find an overview of the costs " "[here](https://test.testserver/statistics/)." in mail.outbox[0].body ) + + +@pytest.mark.django_db +def test_challenge_budget_alert_no_budget(): + challenge = ChallengeFactory() + phase = PhaseFactory(challenge=challenge) + EvaluationFactory( + submission__phase=phase, + compute_cost_euro_millicents=1, + time_limit=60, + ) + update_compute_costs_and_storage_size() + assert len(mail.outbox) != 0 From b901cdcbaad3539e82a0fd0d1b6d1792b18cbd92 Mon Sep 17 00:00:00 2001 From: Thomas Koopman Date: Wed, 18 Dec 2024 13:13:47 +0100 Subject: [PATCH 13/20] Remove condition for approved budget larger than zero --- app/grandchallenge/challenges/tasks.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/app/grandchallenge/challenges/tasks.py b/app/grandchallenge/challenges/tasks.py index b996cabc3..dafdcb79e 100644 --- a/app/grandchallenge/challenges/tasks.py +++ b/app/grandchallenge/challenges/tasks.py @@ -60,10 +60,7 @@ def update_challenge_results_cache(): def send_alert_if_budget_consumed_warning_threshold_exceeded(challenge): - if ( - challenge.has_changed("compute_cost_euro_millicents") - and challenge.approved_compute_costs_euro_millicents - ): + if challenge.has_changed("compute_cost_euro_millicents"): for percent_threshold in sorted( challenge.percent_budget_consumed_warning_thresholds, reverse=True ): From ded7e90cb9f087945899596ee3011fd6eafe7761 Mon Sep 17 00:00:00 2001 From: Thomas Koopman Date: Wed, 18 Dec 2024 13:14:59 +0100 Subject: [PATCH 14/20] Alert also when treshold is zero --- app/grandchallenge/challenges/tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/grandchallenge/challenges/tasks.py b/app/grandchallenge/challenges/tasks.py index dafdcb79e..f48eadb83 100644 --- a/app/grandchallenge/challenges/tasks.py +++ b/app/grandchallenge/challenges/tasks.py @@ -73,7 +73,7 @@ def send_alert_if_budget_consumed_warning_threshold_exceeded(challenge): / 100 ) current_cost = challenge.compute_cost_euro_millicents - if previous_cost < threshold <= current_cost: + if previous_cost <= threshold < current_cost: send_email_percent_budget_consumed_alert( challenge, percent_threshold ) From 4c5b78dc88c5d950e1da59c648390ebe182d8df9 Mon Sep 17 00:00:00 2001 From: Thomas Koopman Date: Wed, 18 Dec 2024 13:15:53 +0100 Subject: [PATCH 15/20] Use correct query so the challenges are actually updated --- app/grandchallenge/challenges/tasks.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/grandchallenge/challenges/tasks.py b/app/grandchallenge/challenges/tasks.py index f48eadb83..1261e9486 100644 --- a/app/grandchallenge/challenges/tasks.py +++ b/app/grandchallenge/challenges/tasks.py @@ -83,9 +83,9 @@ def send_alert_if_budget_consumed_warning_threshold_exceeded(challenge): @acks_late_2xlarge_task @transaction.atomic def update_compute_costs_and_storage_size(): - challenges = Challenge.objects.all() + challenges = Challenge.objects.all().with_available_compute() - for challenge in challenges.with_available_compute(): + for challenge in challenges: annotate_compute_costs_and_storage_size(challenge=challenge) send_alert_if_budget_consumed_warning_threshold_exceeded( challenge=challenge From 583705c9254cd15d7784497ab097e37577dca945 Mon Sep 17 00:00:00 2001 From: Thomas Koopman Date: Wed, 18 Dec 2024 13:16:41 +0100 Subject: [PATCH 16/20] Extend test to include budget not exceeded and repeat exceeded --- app/tests/challenges_tests/test_tasks.py | 39 +++++++++++++++++++++--- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/app/tests/challenges_tests/test_tasks.py b/app/tests/challenges_tests/test_tasks.py index 709228b4d..06f4c169e 100644 --- a/app/tests/challenges_tests/test_tasks.py +++ b/app/tests/challenges_tests/test_tasks.py @@ -135,9 +135,7 @@ def test_challenge_request_budget_calculation(settings): @pytest.mark.django_db def test_challenge_budget_alert_email(settings): - challenge = ChallengeFactory( - short_name="test", - ) + challenge = ChallengeFactory(short_name="test") challenge_admin = UserFactory() challenge.add_admin(challenge_admin) staff_user = UserFactory(is_staff=True) @@ -152,10 +150,22 @@ def test_challenge_budget_alert_email(settings): phase = PhaseFactory(challenge=challenge) EvaluationFactory( submission__phase=phase, - compute_cost_euro_millicents=800000, + compute_cost_euro_millicents=500000, time_limit=60, ) update_compute_costs_and_storage_size() + + # Budget alert threshold not exceeded + assert len(mail.outbox) == 0 + + EvaluationFactory( + submission__phase=phase, + compute_cost_euro_millicents=300000, + time_limit=60, + ) + update_compute_costs_and_storage_size() + + # Budget alert threshold exceeded assert len(mail.outbox) == 3 recipients = [r for m in mail.outbox for r in m.to] assert recipients == [ @@ -173,6 +183,27 @@ def test_challenge_budget_alert_email(settings): "[here](https://test.testserver/statistics/)." in mail.outbox[0].body ) + mail.outbox.clear() + EvaluationFactory( + submission__phase=phase, + compute_cost_euro_millicents=100000, + time_limit=60, + ) + update_compute_costs_and_storage_size() + + # Next budget alert threshold not exceeded + assert len(mail.outbox) == 0 + + EvaluationFactory( + submission__phase=phase, + compute_cost_euro_millicents=1, + time_limit=60, + ) + update_compute_costs_and_storage_size() + + # Next budget alert threshold exceeded + assert len(mail.outbox) != 0 + @pytest.mark.django_db def test_challenge_budget_alert_no_budget(): From 642f4c3ade30ce9f96a19586a6551293e81412fd Mon Sep 17 00:00:00 2001 From: Thomas Koopman Date: Wed, 18 Dec 2024 13:26:05 +0100 Subject: [PATCH 17/20] Edit test --- app/tests/challenges_tests/test_tasks.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/tests/challenges_tests/test_tasks.py b/app/tests/challenges_tests/test_tasks.py index 06f4c169e..51d583dcd 100644 --- a/app/tests/challenges_tests/test_tasks.py +++ b/app/tests/challenges_tests/test_tasks.py @@ -203,6 +203,10 @@ def test_challenge_budget_alert_email(settings): # Next budget alert threshold exceeded assert len(mail.outbox) != 0 + assert ( + mail.outbox[0].subject + == "[testserver] [test] 90% Budget Consumed Alert" + ) @pytest.mark.django_db @@ -214,5 +218,7 @@ def test_challenge_budget_alert_no_budget(): compute_cost_euro_millicents=1, time_limit=60, ) + assert len(mail.outbox) == 0 update_compute_costs_and_storage_size() assert len(mail.outbox) != 0 + assert "Budget Consumed Alert" in mail.outbox[0].subject From 1abf4493172e12e3b92128aa30fbf5e6218dc7ed Mon Sep 17 00:00:00 2001 From: Thomas Koopman Date: Wed, 8 Jan 2025 12:54:53 +0100 Subject: [PATCH 18/20] Add constraints to json input for thresholds --- ...enge_percent_budget_consumed_warning_thresholds.py} | 10 +++++++--- app/grandchallenge/challenges/models.py | 6 +++++- 2 files changed, 12 insertions(+), 4 deletions(-) rename app/grandchallenge/challenges/migrations/{0046_challenge_budget_warning_thresholds_percentage.py => 0047_challenge_percent_budget_consumed_warning_thresholds.py} (72%) diff --git a/app/grandchallenge/challenges/migrations/0046_challenge_budget_warning_thresholds_percentage.py b/app/grandchallenge/challenges/migrations/0047_challenge_percent_budget_consumed_warning_thresholds.py similarity index 72% rename from app/grandchallenge/challenges/migrations/0046_challenge_budget_warning_thresholds_percentage.py rename to app/grandchallenge/challenges/migrations/0047_challenge_percent_budget_consumed_warning_thresholds.py index 58bfa869d..2bc91529e 100644 --- a/app/grandchallenge/challenges/migrations/0046_challenge_budget_warning_thresholds_percentage.py +++ b/app/grandchallenge/challenges/migrations/0047_challenge_percent_budget_consumed_warning_thresholds.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.17 on 2024-12-11 10:16 +# Generated by Django 4.2.17 on 2025-01-08 11:46 from django.db import migrations, models @@ -10,7 +10,7 @@ class Migration(migrations.Migration): dependencies = [ ( "challenges", - "0045_challengerequest_algorithm_maximum_settable_memory_gb_and_more", + "0046_remove_challengerequest_budget_for_hosting_challenge", ), ] @@ -24,7 +24,11 @@ class Migration(migrations.Migration): grandchallenge.core.validators.JSONValidator( schema={ "$schema": "http://json-schema.org/draft-07/schema", - "items": {"type": "integer"}, + "items": { + "exclusiveMinimum": 0, + "maximum": 100, + "type": "integer", + }, "type": "array", "uniqueItems": True, } diff --git a/app/grandchallenge/challenges/models.py b/app/grandchallenge/challenges/models.py index c1e08f3d0..ec43ca5af 100644 --- a/app/grandchallenge/challenges/models.py +++ b/app/grandchallenge/challenges/models.py @@ -387,7 +387,11 @@ class Challenge(ChallengeBase, FieldChangeMixin): schema={ "$schema": "http://json-schema.org/draft-07/schema", "type": "array", - "items": {"type": "integer"}, + "items": { + "type": "integer", + "exclusiveMinimum": 0, + "maximum": 100, + }, "uniqueItems": True, } ) From e5b43564633037996d9a4af8f2583a71f454d073 Mon Sep 17 00:00:00 2001 From: Thomas Koopman Date: Wed, 8 Jan 2025 13:10:38 +0100 Subject: [PATCH 19/20] Update email title --- app/grandchallenge/challenges/emails.py | 2 +- app/tests/challenges_tests/test_tasks.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/grandchallenge/challenges/emails.py b/app/grandchallenge/challenges/emails.py index 446def997..bc1be5b0d 100644 --- a/app/grandchallenge/challenges/emails.py +++ b/app/grandchallenge/challenges/emails.py @@ -96,7 +96,7 @@ def send_challenge_status_update_email(challengerequest, challenge=None): def send_email_percent_budget_consumed_alert(challenge, percent_threshold): subject = format_html( - "[{challenge_name}] {percent_threshold}% Budget Consumed Alert", + "[{challenge_name}] over {percent_threshold}% Budget Consumed Alert", challenge_name=challenge.short_name, percent_threshold=percent_threshold, ) diff --git a/app/tests/challenges_tests/test_tasks.py b/app/tests/challenges_tests/test_tasks.py index 51d583dcd..612565256 100644 --- a/app/tests/challenges_tests/test_tasks.py +++ b/app/tests/challenges_tests/test_tasks.py @@ -175,7 +175,7 @@ def test_challenge_budget_alert_email(settings): ] assert ( mail.outbox[0].subject - == "[testserver] [test] 70% Budget Consumed Alert" + == "[testserver] [test] over 70% Budget Consumed Alert" ) assert ( "We would like to inform you that more than 70% of the compute budget for " @@ -205,7 +205,7 @@ def test_challenge_budget_alert_email(settings): assert len(mail.outbox) != 0 assert ( mail.outbox[0].subject - == "[testserver] [test] 90% Budget Consumed Alert" + == "[testserver] [test] over 90% Budget Consumed Alert" ) From df95d53f3de203e42fcd51caf03d844522fcbece Mon Sep 17 00:00:00 2001 From: Thomas Koopman Date: Wed, 8 Jan 2025 13:11:19 +0100 Subject: [PATCH 20/20] Add test for exceeding two thresholds at once. --- app/tests/challenges_tests/test_tasks.py | 41 ++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/app/tests/challenges_tests/test_tasks.py b/app/tests/challenges_tests/test_tasks.py index 612565256..6cf6d61e3 100644 --- a/app/tests/challenges_tests/test_tasks.py +++ b/app/tests/challenges_tests/test_tasks.py @@ -209,6 +209,47 @@ def test_challenge_budget_alert_email(settings): ) +@pytest.mark.django_db +def test_challenge_budget_alert_two_thresholds_one_email(settings): + challenge = ChallengeFactory(short_name="test") + assert challenge.percent_budget_consumed_warning_thresholds == [ + 70, + 90, + 100, + ] + challenge_admin = UserFactory() + challenge.add_admin(challenge_admin) + staff_user = UserFactory(is_staff=True) + settings.MANAGERS = [(staff_user.last_name, staff_user.email)] + InvoiceFactory( + challenge=challenge, + support_costs_euros=0, + compute_costs_euros=10, + storage_costs_euros=0, + payment_status=PaymentStatusChoices.PAID, + ) + phase = PhaseFactory(challenge=challenge) + EvaluationFactory( + submission__phase=phase, + compute_cost_euro_millicents=950000, + time_limit=60, + ) + update_compute_costs_and_storage_size() + + # Two budget alert thresholds exceeded, alert only sent for last one. + assert len(mail.outbox) == 3 + recipients = [r for m in mail.outbox for r in m.to] + assert recipients == [ + challenge.creator.email, + challenge_admin.email, + staff_user.email, + ] + assert ( + mail.outbox[0].subject + == "[testserver] [test] over 90% Budget Consumed Alert" + ) + + @pytest.mark.django_db def test_challenge_budget_alert_no_budget(): challenge = ChallengeFactory()