Skip to content

Commit 72e2ca9

Browse files
Refactor: Rename ensure_workers_for_host_submission to ensure_workers_for_submission for clarity and broaden functionality. Update all references in the codebase to reflect this change, ensuring both host and participant submissions are handled appropriately.
1 parent 162d96b commit 72e2ca9

File tree

6 files changed

+68
-41
lines changed

6 files changed

+68
-41
lines changed

apps/challenges/aws_utils.py

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -513,24 +513,22 @@ def delete_challenge_cleanup_schedule(challenge):
513513
pass # Schedule already deleted or never created
514514

515515

516-
def ensure_workers_for_host_submission(challenge):
516+
def ensure_workers_for_submission(challenge):
517517
"""
518518
Ensures the worker stack (ECS service, auto-scaling, EventBridge cleanup)
519-
exists for a challenge when a host makes a submission. If the stack has
520-
never been created (challenge.workers is None), this triggers full stack
521-
creation via start_workers.
519+
exists for a challenge when a submission is made (host or participant).
520+
If no active workers exist (workers is None or workers == 0), this triggers
521+
stack creation/start via start_workers.
522522
523-
This allows challenge hosts to test their evaluation pipeline before
524-
requesting admin approval.
525-
526-
Called from submission endpoints when the submitter is a challenge host.
523+
This allows both challenge hosts (pre-approval testing) and participants
524+
(approved challenges) to recover from missing/stopped workers.
527525
528526
Parameters:
529527
challenge (<class 'challenges.models.Challenge'>): The challenge to ensure workers for.
530528
"""
531529
if settings.DEBUG or settings.TEST:
532530
logger.info(
533-
"Skipping ensure_workers_for_host_submission for challenge %s "
531+
"Skipping ensure_workers_for_submission for challenge %s "
534532
"in development/test environment.",
535533
challenge.pk,
536534
)
@@ -544,12 +542,12 @@ def ensure_workers_for_host_submission(challenge):
544542
):
545543
return
546544

547-
# Stack already exists; auto-scaling will handle scale-up
548-
if challenge.workers is not None:
545+
# Stack already has active workers
546+
if challenge.workers not in (None, 0):
549547
return
550548

551549
logger.info(
552-
"Host submission detected for challenge %s with no workers. "
550+
"Submission detected for challenge %s with no active workers. "
553551
"Creating worker stack.",
554552
challenge.pk,
555553
)

apps/jobs/admin.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import logging
22

33
from base.admin import ImportExportTimeStampedAdmin
4-
from challenges.aws_utils import ensure_workers_for_host_submission
4+
from challenges.aws_utils import ensure_workers_for_submission
55
from django.contrib import admin
66

77
from .admin_filters import (
@@ -72,7 +72,7 @@ def submit_job_to_worker(self, request, queryset):
7272
for submission in queryset:
7373
challenge = submission.challenge_phase.challenge
7474
if challenge.pk not in challenges_checked:
75-
ensure_workers_for_host_submission(challenge)
75+
ensure_workers_for_submission(challenge)
7676
challenges_checked.add(challenge.pk)
7777
message = handle_submission_rerun(submission, Submission.CANCELLED)
7878
publish_submission_message(message)

apps/jobs/views.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
is_user_a_staff,
1414
paginated_queryset,
1515
)
16-
from challenges.aws_utils import ensure_workers_for_host_submission
16+
from challenges.aws_utils import ensure_workers_for_submission
1717
from challenges.models import (
1818
Challenge,
1919
ChallengeEvaluationCluster,
@@ -261,9 +261,11 @@ def challenge_submission(request, challenge_id, challenge_phase_id):
261261
return Response(
262262
response_data, status=status.HTTP_403_FORBIDDEN
263263
)
264+
# Ensure worker stack exists for participant submissions
265+
ensure_workers_for_submission(challenge)
264266
else:
265267
# Ensure the worker stack exists for host submissions
266-
ensure_workers_for_host_submission(challenge)
268+
ensure_workers_for_submission(challenge)
267269

268270
participant_team_id = get_participant_team_id_of_user_for_a_challenge(
269271
request.user, challenge_id
@@ -2881,9 +2883,11 @@ def get_submission_file_presigned_url(request, challenge_phase_pk):
28812883
return Response(
28822884
response_data, status=status.HTTP_403_FORBIDDEN
28832885
)
2886+
# Ensure worker stack exists for participant submissions
2887+
ensure_workers_for_submission(challenge)
28842888
else:
28852889
# Ensure the worker stack exists for host submissions
2886-
ensure_workers_for_host_submission(challenge)
2890+
ensure_workers_for_submission(challenge)
28872891

28882892
participant_team_id = get_participant_team_id_of_user_for_a_challenge(
28892893
request.user, challenge.pk
@@ -3177,9 +3181,11 @@ def send_submission_message(request, challenge_phase_pk, submission_pk):
31773181
return Response(
31783182
response_data, status=status.HTTP_403_FORBIDDEN
31793183
)
3184+
# Ensure worker stack exists for participant submissions
3185+
ensure_workers_for_submission(challenge)
31803186
else:
31813187
# Ensure the worker stack exists for host submissions
3182-
ensure_workers_for_host_submission(challenge)
3188+
ensure_workers_for_submission(challenge)
31833189

31843190
participant_team_id = get_participant_team_id_of_user_for_a_challenge(
31853191
request.user, challenge.pk

tests/unit/challenges/test_aws_utils.py

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
delete_service_by_challenge_pk,
2020
delete_workers,
2121
describe_ec2_instance,
22-
ensure_workers_for_host_submission,
22+
ensure_workers_for_submission,
2323
get_capacity_provider_strategy,
2424
get_code_upload_setup_meta_for_challenge,
2525
get_logs_from_cloudwatch,
@@ -4798,7 +4798,7 @@ def test_end_date_change_skipped_on_create(self):
47984798
mock_start.assert_not_called()
47994799

48004800

4801-
class TestEnsureWorkersForHostSubmission(TestCase):
4801+
class TestEnsureWorkersForSubmission(TestCase):
48024802
@patch("challenges.aws_utils.settings")
48034803
def test_returns_early_in_debug_mode(self, mock_settings):
48044804
"""Should return early without calling start_workers in DEBUG mode."""
@@ -4807,7 +4807,7 @@ def test_returns_early_in_debug_mode(self, mock_settings):
48074807
challenge.pk = 1
48084808

48094809
with patch("challenges.aws_utils.start_workers") as mock_start:
4810-
ensure_workers_for_host_submission(challenge)
4810+
ensure_workers_for_submission(challenge)
48114811
mock_start.assert_not_called()
48124812

48134813
@patch("challenges.aws_utils.settings")
@@ -4819,7 +4819,7 @@ def test_returns_early_in_test_mode(self, mock_settings):
48194819
challenge.pk = 1
48204820

48214821
with patch("challenges.aws_utils.start_workers") as mock_start:
4822-
ensure_workers_for_host_submission(challenge)
4822+
ensure_workers_for_submission(challenge)
48234823
mock_start.assert_not_called()
48244824

48254825
@patch("challenges.aws_utils.settings")
@@ -4834,7 +4834,7 @@ def test_returns_early_for_docker_based_challenge(self, mock_settings):
48344834
challenge.remote_evaluation = False
48354835

48364836
with patch("challenges.aws_utils.start_workers") as mock_start:
4837-
ensure_workers_for_host_submission(challenge)
4837+
ensure_workers_for_submission(challenge)
48384838
mock_start.assert_not_called()
48394839

48404840
@patch("challenges.aws_utils.settings")
@@ -4849,7 +4849,7 @@ def test_returns_early_for_ec2_challenge(self, mock_settings):
48494849
challenge.remote_evaluation = False
48504850

48514851
with patch("challenges.aws_utils.start_workers") as mock_start:
4852-
ensure_workers_for_host_submission(challenge)
4852+
ensure_workers_for_submission(challenge)
48534853
mock_start.assert_not_called()
48544854

48554855
@patch("challenges.aws_utils.settings")
@@ -4866,7 +4866,7 @@ def test_returns_early_for_remote_evaluation_challenge(
48664866
challenge.remote_evaluation = True
48674867

48684868
with patch("challenges.aws_utils.start_workers") as mock_start:
4869-
ensure_workers_for_host_submission(challenge)
4869+
ensure_workers_for_submission(challenge)
48704870
mock_start.assert_not_called()
48714871

48724872
@patch("challenges.aws_utils.settings")
@@ -4882,13 +4882,12 @@ def test_returns_early_when_workers_already_exist(self, mock_settings):
48824882
challenge.workers = 1 # Workers already exist
48834883

48844884
with patch("challenges.aws_utils.start_workers") as mock_start:
4885-
ensure_workers_for_host_submission(challenge)
4885+
ensure_workers_for_submission(challenge)
48864886
mock_start.assert_not_called()
48874887

48884888
@patch("challenges.aws_utils.settings")
4889-
def test_returns_early_when_workers_stopped(self, mock_settings):
4890-
"""Should be a no-op when workers are stopped (0) since
4891-
auto-scaling will handle scale-up."""
4889+
def test_starts_workers_when_workers_stopped(self, mock_settings):
4890+
"""Should start workers when workers are stopped (0)."""
48924891
mock_settings.DEBUG = False
48934892
mock_settings.TEST = False
48944893
challenge = MagicMock()
@@ -4899,8 +4898,9 @@ def test_returns_early_when_workers_stopped(self, mock_settings):
48994898
challenge.workers = 0 # Workers exist but stopped
49004899

49014900
with patch("challenges.aws_utils.start_workers") as mock_start:
4902-
ensure_workers_for_host_submission(challenge)
4903-
mock_start.assert_not_called()
4901+
mock_start.return_value = {"count": 1, "failures": []}
4902+
ensure_workers_for_submission(challenge)
4903+
mock_start.assert_called_once_with([challenge])
49044904

49054905
@patch("challenges.aws_utils.start_workers")
49064906
@patch("challenges.aws_utils.settings")
@@ -4919,7 +4919,7 @@ def test_creates_workers_when_none_exist(
49194919

49204920
mock_start_workers.return_value = {"count": 1, "failures": []}
49214921

4922-
ensure_workers_for_host_submission(challenge)
4922+
ensure_workers_for_submission(challenge)
49234923

49244924
mock_start_workers.assert_called_once_with([challenge])
49254925

@@ -4946,7 +4946,7 @@ def test_logs_error_when_worker_creation_fails(
49464946
}
49474947

49484948
# Should not raise, just log the error
4949-
ensure_workers_for_host_submission(challenge)
4949+
ensure_workers_for_submission(challenge)
49504950

49514951
mock_start_workers.assert_called_once_with([challenge])
49524952

tests/unit/jobs/test_admin.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -194,11 +194,11 @@ def test_get_challenge_name_and_id(self):
194194

195195
@patch("jobs.admin.publish_submission_message")
196196
@patch("jobs.admin.handle_submission_rerun")
197-
@patch("jobs.admin.ensure_workers_for_host_submission")
197+
@patch("jobs.admin.ensure_workers_for_submission")
198198
def test_submit_job_calls_ensure_workers(
199199
self, mock_ensure_workers, mock_rerun, mock_publish
200200
):
201-
"""ensure_workers_for_host_submission should be called for
201+
"""ensure_workers_for_submission should be called for
202202
each unique challenge when re-running submissions."""
203203
mock_rerun.return_value = {"challenge_pk": self.challenge.pk}
204204
queryset = Submission.objects.filter(pk=self.submission.pk)
@@ -211,11 +211,11 @@ def test_submit_job_calls_ensure_workers(
211211

212212
@patch("jobs.admin.publish_submission_message")
213213
@patch("jobs.admin.handle_submission_rerun")
214-
@patch("jobs.admin.ensure_workers_for_host_submission")
214+
@patch("jobs.admin.ensure_workers_for_submission")
215215
def test_submit_job_calls_ensure_workers_once_per_challenge(
216216
self, mock_ensure_workers, mock_rerun, mock_publish
217217
):
218-
"""ensure_workers_for_host_submission should only be called once
218+
"""ensure_workers_for_submission should only be called once
219219
per challenge even when re-running multiple submissions."""
220220
second_submission = Submission.objects.create(
221221
participant_team=self.participant_team,
@@ -242,11 +242,11 @@ def test_submit_job_calls_ensure_workers_once_per_challenge(
242242

243243
@patch("jobs.admin.publish_submission_message")
244244
@patch("jobs.admin.handle_submission_rerun")
245-
@patch("jobs.admin.ensure_workers_for_host_submission")
245+
@patch("jobs.admin.ensure_workers_for_submission")
246246
def test_submit_job_calls_ensure_workers_per_distinct_challenge(
247247
self, mock_ensure_workers, mock_rerun, mock_publish
248248
):
249-
"""ensure_workers_for_host_submission should be called once per
249+
"""ensure_workers_for_submission should be called once per
250250
distinct challenge when submissions span multiple challenges."""
251251
second_challenge = Challenge.objects.create(
252252
title="Second Challenge",

tests/unit/jobs/test_views.py

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,29 @@ def test_challenge_submission_for_successful_submission(self):
563563
)
564564
self.assertEqual(response.status_code, status.HTTP_201_CREATED)
565565

566+
@mock.patch("jobs.views.ensure_workers_for_submission")
567+
def test_challenge_submission_calls_ensure_workers_for_participant(
568+
self, mock_ensure_workers
569+
):
570+
self.url = reverse_lazy(
571+
"jobs:challenge_submission",
572+
kwargs={
573+
"challenge_id": self.challenge.pk,
574+
"challenge_phase_id": self.challenge_phase.pk,
575+
},
576+
)
577+
578+
self.challenge.participant_teams.add(self.participant_team)
579+
self.challenge.save()
580+
581+
response = self.client.post(
582+
self.url,
583+
{"status": "submitting", "input_file": self.input_file},
584+
format="multipart",
585+
)
586+
self.assertEqual(response.status_code, status.HTTP_201_CREATED)
587+
mock_ensure_workers.assert_called_once_with(self.challenge)
588+
566589
def test_challenge_submission_when_maximum_limit_exceeded(self):
567590
self.url = reverse_lazy(
568591
"jobs:challenge_submission",
@@ -2677,7 +2700,7 @@ class PresignedURLSubmissionTest(BaseAPITestClass):
26772700
def setUp(self):
26782701
super(PresignedURLSubmissionTest, self).setUp()
26792702

2680-
@mock.patch("jobs.views.ensure_workers_for_host_submission")
2703+
@mock.patch("jobs.views.ensure_workers_for_submission")
26812704
@mock.patch("challenges.utils.get_aws_credentials_for_challenge")
26822705
def test_get_submission_presigned_url(
26832706
self, mock_get_aws_creds, mock_ensure_workers
@@ -2722,7 +2745,7 @@ def test_get_submission_presigned_url(
27222745
)
27232746
self.assertEqual(response.status_code, status.HTTP_201_CREATED)
27242747

2725-
@mock.patch("jobs.views.ensure_workers_for_host_submission")
2748+
@mock.patch("jobs.views.ensure_workers_for_submission")
27262749
@mock.patch("challenges.utils.get_aws_credentials_for_challenge")
27272750
def test_finish_submission_file_upload(
27282751
self, mock_get_aws_creds, mock_ensure_workers

0 commit comments

Comments
 (0)