From e12cee41671ae8741a517e7ad5011ba9b4f7e8c3 Mon Sep 17 00:00:00 2001 From: meua Date: Tue, 30 Jan 2024 09:55:33 +0800 Subject: [PATCH] change: Optimize code and fix bugs --- .../migrations/0003_auto_20240109_1048.py | 18 ++++ apps/base/utils.py | 10 +-- apps/challenges/aws_utils.py | 41 +++++++++ apps/challenges/challenge_config_utils.py | 2 + ...ter_challenge_ephemeral_storage_default.py | 18 ++++ .../0112_challenge_sqs_retention_period.py | 18 ++++ apps/challenges/models.py | 5 ++ apps/challenges/serializers.py | 11 ++- apps/challenges/urls.py | 5 ++ apps/challenges/utils.py | 8 +- apps/challenges/views.py | 84 ++++++++++++++++--- apps/jobs/sender.py | 5 +- apps/jobs/views.py | 36 ++++---- docs/source/architecture_decisions.md | 4 +- docs/source/submission.md | 2 +- .../workers/code_upload_submission_worker.py | 2 +- scripts/workers/remote_submission_worker.py | 2 +- scripts/workers/submission_worker.py | 7 +- tests/unit/challenges/test_views.py | 71 +++++++++++++++- tests/unit/participants/test_views.py | 2 + tests/unit/remoteworker/test_remote_worker.py | 8 +- 21 files changed, 301 insertions(+), 58 deletions(-) create mode 100644 apps/accounts/migrations/0003_auto_20240109_1048.py create mode 100644 apps/challenges/migrations/0111_alter_challenge_ephemeral_storage_default.py create mode 100644 apps/challenges/migrations/0112_challenge_sqs_retention_period.py diff --git a/apps/accounts/migrations/0003_auto_20240109_1048.py b/apps/accounts/migrations/0003_auto_20240109_1048.py new file mode 100644 index 0000000000..85ebc4aa96 --- /dev/null +++ b/apps/accounts/migrations/0003_auto_20240109_1048.py @@ -0,0 +1,18 @@ +# Generated by Django 2.2.20 on 2024-01-09 02:48 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('accounts', '0002_add_jwt_access_token_model'), + ] + + operations = [ + migrations.AlterField( + model_name='profile', + name='avatar', + field=models.URLField(blank=True, max_length=100, null=True), + ), + ] diff --git a/apps/base/utils.py b/apps/base/utils.py index aa5da33b7d..8896188b9c 100644 --- a/apps/base/utils.py +++ b/apps/base/utils.py @@ -149,10 +149,10 @@ def send_email( def get_url_from_hostname(hostname): - # if settings.DEBUG or settings.TEST: - scheme = "http" # TODO After https is supported, this needs to be restored to its original state. - # else: - # scheme = "https" + if settings.DEBUG or settings.TEST: + scheme = "http" + else: + scheme = "https" url = "{}://{}".format(scheme, hostname) return url @@ -180,7 +180,7 @@ def get_boto3_client(resource, aws_keys): def get_or_create_sqs_queue(queue_name, challenge=None): if settings.DEBUG or settings.TEST: - queue_name = "evalai_submission_queue" + queue_name = "arena_submission_queue" sqs = boto3.resource( "sqs", endpoint_url=os.environ.get("AWS_SQS_ENDPOINT", "http://sqs:9324"), diff --git a/apps/challenges/aws_utils.py b/apps/challenges/aws_utils.py index bafaa1b067..f1169fe4e7 100644 --- a/apps/challenges/aws_utils.py +++ b/apps/challenges/aws_utils.py @@ -771,6 +771,34 @@ def create_ec2_instance(challenge, ec2_storage=None, worker_instance_type=None, } +def update_sqs_retention_period(challenge): + """ + Update the SQS retention period for a challenge. + + Args: + challenge (Challenge): The challenge for which the SQS retention period is to be updated. + + Returns: + dict: A dictionary containing the status and message of the operation. + """ + sqs_retention_period = str(challenge.sqs_retention_period) + try: + sqs = get_boto3_client("sqs", aws_keys) + queue_url = sqs.get_queue_url(QueueName=challenge.queue)['QueueUrl'] + response = sqs.set_queue_attributes( + QueueUrl=queue_url, + Attributes={ + 'MessageRetentionPeriod': sqs_retention_period + } + ) + return {"message": response} + except Exception as e: + logger.exception(e) + return { + "error": str(e), + } + + def start_workers(queryset): """ The function called by the admin action method to start all the selected workers. @@ -1794,3 +1822,16 @@ def setup_ec2(challenge): if challenge_obj.ec2_instance_id: return start_ec2_instance(challenge_obj) return create_ec2_instance(challenge_obj) + + +@app.task +def update_sqs_retention_period_task(challenge): + """ + Updates sqs retention period for a challenge when the attribute is changed. + + Arguments: + challenge {} -- instance of the model calling the post hook + """ + for obj in serializers.deserialize("json", challenge): + challenge_obj = obj.object + return update_sqs_retention_period(challenge_obj) diff --git a/apps/challenges/challenge_config_utils.py b/apps/challenges/challenge_config_utils.py index f048bb4e45..bbdbe9d399 100644 --- a/apps/challenges/challenge_config_utils.py +++ b/apps/challenges/challenge_config_utils.py @@ -287,6 +287,8 @@ def get_value_from_field(data, base_location, field_name): "duplicate_rank": "ERROR: Duplicate rank {} found in YAML data.", "prize_amount_wrong": "ERROR: Invalid amount value {}. Amount should be in decimal format with three-letter currency code (e.g. 100.00USD, 500EUR, 1000INR).", "prize_rank_wrong": "ERROR: Invalid rank value {}. Rank should be an integer.", + "challenge_metadata_schema_errors": "ERROR: Unable to serialize the challenge because of the following errors: {}.", + "evaluation_script_not_zip": "ERROR: Please pass in a zip file as evaluation script. If using the `evaluation_script` directory (recommended), it should be `evaluation_script.zip`.", } diff --git a/apps/challenges/migrations/0111_alter_challenge_ephemeral_storage_default.py b/apps/challenges/migrations/0111_alter_challenge_ephemeral_storage_default.py new file mode 100644 index 0000000000..c70937e454 --- /dev/null +++ b/apps/challenges/migrations/0111_alter_challenge_ephemeral_storage_default.py @@ -0,0 +1,18 @@ +# Generated by Django 2.2.20 on 2023-12-01 15:46 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('challenges', '0110_challenge_ephemeral_storage'), + ] + + operations = [ + migrations.AlterField( + model_name='challenge', + name='ephemeral_storage', + field=models.PositiveIntegerField(default=21, verbose_name='Ephemeral Storage (GB)'), + ), + ] diff --git a/apps/challenges/migrations/0112_challenge_sqs_retention_period.py b/apps/challenges/migrations/0112_challenge_sqs_retention_period.py new file mode 100644 index 0000000000..9c786cd733 --- /dev/null +++ b/apps/challenges/migrations/0112_challenge_sqs_retention_period.py @@ -0,0 +1,18 @@ +# Generated by Django 2.2.20 on 2023-12-10 15:16 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('challenges', '0111_alter_challenge_ephemeral_storage_default'), + ] + + operations = [ + migrations.AddField( + model_name='challenge', + name='sqs_retention_period', + field=models.PositiveIntegerField(default=345600, verbose_name='SQS Retention Period'), + ), + ] diff --git a/apps/challenges/models.py b/apps/challenges/models.py index 465aef3b88..18ac4bdd36 100644 --- a/apps/challenges/models.py +++ b/apps/challenges/models.py @@ -35,6 +35,7 @@ def __init__(self, *args, **kwargs): super(Challenge, self).__init__(*args, **kwargs) self._original_evaluation_script = self.evaluation_script self._original_approved_by_admin = self.approved_by_admin + self._original_sqs_retention_period = self.sqs_retention_period title = models.CharField(max_length=100, db_index=True, unique=True) short_description = models.TextField(null=True, blank=True) @@ -128,6 +129,10 @@ def __init__(self, *args, **kwargs): null=False, blank=False ) + sqs_retention_period = models.PositiveIntegerField( + default=345600, + verbose_name="SQS Retention Period" + ) is_docker_based = models.BooleanField( default=False, verbose_name="Is Docker Based", db_index=True ) diff --git a/apps/challenges/serializers.py b/apps/challenges/serializers.py index a3bef0fab7..c3d50128ad 100644 --- a/apps/challenges/serializers.py +++ b/apps/challenges/serializers.py @@ -94,7 +94,8 @@ class Meta: "ephemeral_storage", "evaluation_module_error", "worker_image_url", - "worker_instance_type" + "worker_instance_type", + "sqs_retention_period" ) @@ -276,6 +277,7 @@ class Meta: "enable_forum", "anonymous_leaderboard", "leaderboard_description", + "manual_participant_approval", "image", "is_active", "evaluation_script", @@ -291,6 +293,10 @@ class Meta: "is_docker_based", "queue", "queue_aws_region", + "aws_account_id", + "aws_access_key_id", + "aws_secret_access_key", + "aws_region", "is_static_dataset_code_upload", "slug", "max_docker_image_size", @@ -312,7 +318,8 @@ class Meta: "ec2_storage", "ephemeral_storage", "evaluation_module_error", - "worker_image_url" + "worker_image_url", + "sqs_retention_period" ) diff --git a/apps/challenges/urls.py b/apps/challenges/urls.py index 8cbf19de67..67f2fe90e7 100644 --- a/apps/challenges/urls.py +++ b/apps/challenges/urls.py @@ -289,6 +289,11 @@ views.update_challenge_tags_and_domain, name="update_challenge_tags_and_domain", ), + url( + r"^challenge/update_challenge_attributes/$", + views.update_challenge_attributes, + name="update_challenge_attributes", + ), url( r"^challenge/get_domain_choices/$", views.get_domain_choices, diff --git a/apps/challenges/utils.py b/apps/challenges/utils.py index 94f8f5fb5b..72753a3f7e 100644 --- a/apps/challenges/utils.py +++ b/apps/challenges/utils.py @@ -9,12 +9,12 @@ from botocore.exceptions import ClientError from django.conf import settings from django.core.files.base import ContentFile -# from moto import mock_ecr, mock_sts +from moto import mock_ecr, mock_sts from base.utils import ( get_model_object, get_boto3_client, - # mock_if_non_prod_aws, + mock_if_non_prod_aws, send_email, ) @@ -357,8 +357,8 @@ def create_federated_user(name, repository, aws_keys): return response -# @mock_if_non_prod_aws(mock_ecr) -# @mock_if_non_prod_aws(mock_sts) +@mock_if_non_prod_aws(mock_ecr) +@mock_if_non_prod_aws(mock_sts) def get_aws_credentials_for_submission(challenge, participant_team): """ Method to generate AWS Credentails for CLI's Push diff --git a/apps/challenges/views.py b/apps/challenges/views.py index 7601fca607..3e832e5724 100644 --- a/apps/challenges/views.py +++ b/apps/challenges/views.py @@ -2532,7 +2532,7 @@ def get_or_update_leaderboard(request, leaderboard_pk): if request.method == "PATCH": if "schema" in request.data.keys(): - request.data['schema'] = json.loads(request.data['schema']) + request.data['schema'] = request.data['schema'] serializer = LeaderboardSerializer( leaderboard, data=request.data, partial=True ) @@ -3974,7 +3974,7 @@ def create_or_update_github_challenge(request, challenge_host_team_pk): if serializer.is_valid(): serializer.save() else: - error_messages = serializer.errors + error_messages = f"leaderboard {data['id']} :{str(serializer.errors)}" raise RuntimeError() leaderboard_ids[ str(data["id"]) @@ -4014,7 +4014,7 @@ def create_or_update_github_challenge(request, challenge_host_team_pk): if serializer.is_valid(): serializer.save() else: - error_messages = serializer.errors + error_messages = f"challenge phase {data['id']} :{str(serializer.errors)}" raise RuntimeError() challenge_phase_ids[ str(data["id"]) @@ -4032,7 +4032,7 @@ def create_or_update_github_challenge(request, challenge_host_team_pk): if serializer.is_valid(): serializer.save() else: - error_messages = serializer.errors + error_messages = f"dataset split {data['id']} :{str(serializer.errors)}" raise RuntimeError() dataset_split_ids[ str(data["id"]) @@ -4089,7 +4089,7 @@ def create_or_update_github_challenge(request, challenge_host_team_pk): if serializer.is_valid(): serializer.save() else: - error_messages = serializer.errors + error_messages = f"challenge phase split (phase:{data['challenge_phase_id']}, leaderboard:{data['leaderboard_id']}, dataset split: {data['dataset_split_id']}):{str(serializer.errors)}" raise RuntimeError() zip_config = ChallengeConfiguration.objects.get( @@ -4152,7 +4152,7 @@ def create_or_update_github_challenge(request, challenge_host_team_pk): except: # noqa: E722 response_data = { - "error": "Error in creating challenge. Please check the yaml configuration!" + "error": f"Error in creating challenge: {error_messages}. Please check the yaml configuration!" } if error_messages: response_data["error_message"] = json.dumps(error_messages) @@ -4201,7 +4201,7 @@ def create_or_update_github_challenge(request, challenge_host_team_pk): if serializer.is_valid(): serializer.save() else: - error_messages = serializer.errors + error_messages = f"challenge :{str(serializer.errors)}" raise RuntimeError() challenge = serializer.instance @@ -4247,7 +4247,7 @@ def create_or_update_github_challenge(request, challenge_host_team_pk): serializer.save() leaderboard_ids[str(data["id"])] = serializer.instance.pk else: - error_messages = serializer.errors + error_messages = f"leaderboard update {(data['id'])} :{str(serializer.errors)}" raise RuntimeError() # Updating ChallengePhase objects @@ -4312,7 +4312,7 @@ def create_or_update_github_challenge(request, challenge_host_team_pk): str(data["id"]) ] = serializer.instance.pk else: - error_messages = serializer.errors + error_messages = f"challenge phase update {(data['id'])} :{str(serializer.errors)}" raise RuntimeError() # Updating DatasetSplit objects @@ -4339,7 +4339,7 @@ def create_or_update_github_challenge(request, challenge_host_team_pk): serializer.save() dataset_split_ids[str(data["id"])] = serializer.instance.pk else: - error_messages = serializer.errors + error_messages = f"dataset split update {(data['id'])} :{str(serializer.errors)}" raise RuntimeError() # Update ChallengePhaseSplit objects @@ -4400,7 +4400,7 @@ def create_or_update_github_challenge(request, challenge_host_team_pk): if serializer.is_valid(): serializer.save() else: - error_messages = serializer.errors + error_messages = f"challenge phase split update (phase:{data['challenge_phase_id']}, leaderboard:{data['leaderboard_id']}, dataset split: {data['dataset_split_id']}):{str(serializer.errors)}" raise RuntimeError() response_data = { @@ -4411,7 +4411,7 @@ def create_or_update_github_challenge(request, challenge_host_team_pk): return Response(response_data, status=status.HTTP_200_OK) except: # noqa: E722 response_data = { - "error": "Error in creating challenge. Please check the yaml configuration!" + "error": f"Error in creating challenge: {error_messages}. Please check the yaml configuration!" } if error_messages: response_data["error_message"] = json.dumps(error_messages) @@ -4462,6 +4462,10 @@ def create_or_update_challenge(request, challenge_host_team_pk): data["is_docker_based"] = True data["enable_forum"] = True data["is_registration_open"] = True + data["aws_account_id"] = os.environ.get("HOST_AWS_ACCOUNT_ID", "") + data["aws_access_key_id"] = os.environ.get("HOST_AWS_ACCESS_KEY_ID", "") + data["aws_secret_access_key"] = os.environ.get("HOST_AWS_SECRET_ACCESS_KEY", "") + data["aws_region"] = os.environ.get("HOST_AWS_DEFAULT_REGION", "us-west-1") serializer = ZipChallengeSerializer( data=data, context={ @@ -4519,6 +4523,10 @@ def create_or_update_challenge(request, challenge_host_team_pk): challenge.image = simple_image_url(image) else: challenge.image = image + manual_participant_approval = request.data.get("manual_participant_approval") + if manual_participant_approval is not None: + manual_participant_approval = manual_participant_approval.lower() == 'true' + challenge.manual_participant_approval = manual_participant_approval published = request.data.get("published") if published is not None: published = published.lower() == 'true' @@ -4873,6 +4881,58 @@ def update_allowed_email_ids(request, challenge_pk, phase_pk): return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) +@api_view(["POST"]) +@throttle_classes([UserRateThrottle]) +@permission_classes((permissions.IsAuthenticated, HasVerifiedEmail)) +@authentication_classes((JWTAuthentication, ExpiringTokenAuthentication)) +def update_challenge_attributes(request): + """ + API to update attributes of the Challenge model + Arguments: + request {dict} -- Request object + + Query Parameters: + challenge_pk {int} -- Challenge primary key + **kwargs {any} -- Key-value pairs representing attributes and their new values + """ + if not request.user.is_staff: + response_data = { + "error": "Sorry, you are not authorized to access this resource!" + } + return Response(response_data, status=status.HTTP_401_UNAUTHORIZED) + + challenge_pk = request.data.get("challenge_pk") + + if not challenge_pk: + response_data = { + "error": "Challenge primary key is missing!" + } + return Response(response_data, status=status.HTTP_400_BAD_REQUEST) + + try: + challenge = Challenge.objects.get(pk=challenge_pk) + except Challenge.DoesNotExist: + response_data = { + "error": f"Challenge with primary key {challenge_pk} not found!" + } + return Response(response_data, status=status.HTTP_404_NOT_FOUND) + + # Update attributes based on the request data + for key, value in request.data.items(): + if key != "challenge_pk" and hasattr(challenge, key): + setattr(challenge, key, value) + + try: + challenge.save() + except Exception as e: + return Response({"error": str(e)}, status=status.HTTP_400_BAD_REQUEST) + + response_data = { + "message": f"Challenge attributes updated successfully for challenge with primary key {challenge_pk}!" + } + return Response(response_data, status=status.HTTP_200_OK) + + @api_view(["GET"]) @throttle_classes([UserRateThrottle]) @permission_classes((permissions.IsAuthenticated, HasVerifiedEmail)) diff --git a/apps/jobs/sender.py b/apps/jobs/sender.py index 5561c391b0..bf11fb0c63 100644 --- a/apps/jobs/sender.py +++ b/apps/jobs/sender.py @@ -34,7 +34,7 @@ def get_or_create_sqs_queue(queue_name, challenge=None): aws_access_key_id=os.environ.get("AWS_ACCESS_KEY_ID", "x"), ) # Use default queue name in dev and test environment - queue_name = "evalai_submission_queue" + queue_name = "arena_submission_queue" else: if challenge and challenge.use_host_sqs: sqs = boto3.resource( @@ -62,9 +62,10 @@ def get_or_create_sqs_queue(queue_name, challenge=None): ex.response["Error"]["Code"] == "AWS.SimpleQueueService.NonExistentQueue" ): + sqs_retention_period = SQS_RETENTION_PERIOD if challenge is None else str(challenge.sqs_retention_period) queue = sqs.create_queue( QueueName=queue_name, - Attributes={"MessageRetentionPeriod": SQS_RETENTION_PERIOD}, + Attributes={"MessageRetentionPeriod": sqs_retention_period}, ) else: logger.exception("Cannot get or create Queue") diff --git a/apps/jobs/views.py b/apps/jobs/views.py index 3949c8813f..870f35bf65 100644 --- a/apps/jobs/views.py +++ b/apps/jobs/views.py @@ -1159,7 +1159,7 @@ def update_submission(request, challenge_pk): leaderboard_data_list = [] for phase_result in results: - split = phase_result.get("split") + split = phase_result.get("split", "split1") accuracies = phase_result.get("accuracies") show_to_participant = phase_result.get( "show_to_participant", False @@ -1171,7 +1171,7 @@ def update_submission(request, challenge_pk): ) except ChallengePhaseSplit.DoesNotExist: response_data = { - "error": "Challenge Phase Split does not exist with phase_id: {} and" + "error": "Challenge Phase Split does not exist with phase_id: {} and " "split codename: {}".format(challenge_phase_pk, split) } return Response( @@ -1274,10 +1274,10 @@ def update_submission(request, challenge_pk): submission.stderr_file.save("stderr.txt", ContentFile(stderr_content)) submission.environment_log_file.save("environment_log.txt", ContentFile(environment_log_content)) submission.submission_result_file.save( - "submission_result.json", ContentFile(str(public_results)) + "submission_result.json", ContentFile(str(public_results).encode("utf-8")) ) submission.submission_metadata_file.save( - "submission_metadata_file.json", ContentFile(str(metadata)) + "submission_metadata_file.json", ContentFile(str(metadata).encode("utf-8")) ) submission.save() response_data = { @@ -1512,8 +1512,8 @@ def update_partially_evaluated_submission(request, challenge_pk): challenge_phase_pk = request.data.get("challenge_phase") submission_pk = request.data.get("submission") submission_status = request.data.get("submission_status", "").lower() - stdout_content = request.data.get("stdout", "") - stderr_content = request.data.get("stderr", "") + stdout_content = request.data.get("stdout", "").encode("utf-8") + stderr_content = request.data.get("stderr", "").encode("utf-8") submission_result = request.data.get("result", "") metadata = request.data.get("metadata", "") submission = get_submission_model(submission_pk) @@ -1550,7 +1550,7 @@ def update_partially_evaluated_submission(request, challenge_pk): leaderboard_data_list = [] for phase_result in results: - split = phase_result.get("split") + split = phase_result.get("split", "split1") accuracies = phase_result.get("accuracies") show_to_participant = phase_result.get( "show_to_participant", False @@ -1562,7 +1562,7 @@ def update_partially_evaluated_submission(request, challenge_pk): ) except ChallengePhaseSplit.DoesNotExist: response_data = { - "error": "Challenge Phase Split does not exist with phase_id: {} and" + "error": "Challenge Phase Split does not exist with phase_id: {} and " "split codename: {}".format(challenge_phase_pk, split) } return Response( @@ -1669,10 +1669,10 @@ def update_partially_evaluated_submission(request, challenge_pk): submission.stdout_file.save("stdout.txt", ContentFile(stdout_content)) submission.stderr_file.save("stderr.txt", ContentFile(stderr_content)) submission.submission_result_file.save( - "submission_result.json", ContentFile(str(public_results)) + "submission_result.json", ContentFile(str(public_results).encode("utf-8")) ) submission.submission_metadata_file.save( - "submission_metadata_file.json", ContentFile(str(metadata)) + "submission_metadata_file.json", ContentFile(str(metadata).encode("utf-8")) ) submission.save() response_data = { @@ -1720,8 +1720,8 @@ def update_partially_evaluated_submission(request, challenge_pk): or submission_status == Submission.FINISHED ): challenge_phase_pk = request.data.get("challenge_phase") - stdout_content = request.data.get("stdout", "") - stderr_content = request.data.get("stderr", "") + stdout_content = request.data.get("stdout", "").encode('utf-8') + stderr_content = request.data.get("stderr", "").encode('utf-8') submission_result = request.data.get("result", "") try: @@ -1738,7 +1738,7 @@ def update_partially_evaluated_submission(request, challenge_pk): public_results = [] leaderboard_data_list = [] for phase_result in results: - split = phase_result.get("split") + split = phase_result.get("split", "split1") accuracies = phase_result.get("accuracies") show_to_participant = phase_result.get( "show_to_participant", False @@ -1750,7 +1750,7 @@ def update_partially_evaluated_submission(request, challenge_pk): ) except ChallengePhaseSplit.DoesNotExist: response_data = { - "error": "Challenge Phase Split does not exist with phase_id: {} and" + "error": "Challenge Phase Split does not exist with phase_id: {} and " "split codename: {}".format(challenge_phase_pk, split) } return Response( @@ -1763,7 +1763,7 @@ def update_partially_evaluated_submission(request, challenge_pk): ) except LeaderboardData.DoesNotExist: response_data = { - "error": "Leaderboard Data does not exist with phase_id: {} and" + "error": "Leaderboard Data does not exist with phase_id: {} and " "submission id: {}".format( challenge_phase_pk, submission_pk ) @@ -1793,7 +1793,7 @@ def update_partially_evaluated_submission(request, challenge_pk): ) if len(missing_metrics) and not is_partial_evaluation_phase: response_data = { - "error": "Following metrics are missing in the" + "error": "Following metrics are missing in the " "leaderboard data: {} of challenge phase: {}".format( missing_metrics, challenge_phase_pk ) @@ -1804,7 +1804,7 @@ def update_partially_evaluated_submission(request, challenge_pk): if len(malformed_metrics): response_data = { - "error": "Values for following metrics are not of" + "error": "Values for following metrics are not of " "float/int: {}".format(malformed_metrics) } return Response( @@ -1861,7 +1861,7 @@ def update_partially_evaluated_submission(request, challenge_pk): "stderr.txt", ContentFile(stderr_content) ) submission.submission_result_file.save( - "submission_result.json", ContentFile(str(public_results)) + "submission_result.json", ContentFile(str(public_results).encode('utf-8')) ) submission.save() response_data = { diff --git a/docs/source/architecture_decisions.md b/docs/source/architecture_decisions.md index 9c1da73373..416bc24dec 100644 --- a/docs/source/architecture_decisions.md +++ b/docs/source/architecture_decisions.md @@ -40,11 +40,11 @@ Hence we decided to process and evaluate submission message in an asynchronous m Out of all the awesome messaging frameworks available, we have chosen Amazon Simple Queue Service (SQS) because it can support decoupled environments. It allows developers to focus on application development, rather than creating their own sophisticated message-based applications. It also eliminates queuing management tasks, such as storage. SQS also works with AWS resources, so you can use it to make reliable and scalable applications on top of an AWS infrastructure. -For the worker, we went ahead with a normal python worker, which simply runs a process and loads all the required data in its memory. As soon as the worker starts, it listens on a SQS queue named `evalai_submission_queue` for new submission messages. +For the worker, we went ahead with a normal python worker, which simply runs a process and loads all the required data in its memory. As soon as the worker starts, it listens on a SQS queue named `arena_submission_queue` for new submission messages. ### Submission Worker -The submission worker is responsible for processing submission messages. It listens on a queue named `evalai_submission_queue`, and on receiving a message for a submission, it processes and evaluates the submission. +The submission worker is responsible for processing submission messages. It listens on a queue named `arena_submission_queue`, and on receiving a message for a submission, it processes and evaluates the submission. One of the major design changes that we decided to implement in the submission worker was to load all the data related to the challenge in the worker's memory, instead of fetching it every time a new submission message arrives. So the worker, when starting, fetches the list of active challenges from the database and then loads it into memory by maintaining the map `EVALUATION_SCRIPTS` on challenge id. This was actually a major performance improvement. diff --git a/docs/source/submission.md b/docs/source/submission.md index 0c9aee3cbe..6c5870d704 100644 --- a/docs/source/submission.md +++ b/docs/source/submission.md @@ -73,7 +73,7 @@ EVALUATION_SCRIPTS = { } ``` -After the challenges are successfully loaded, it creates a connection with the SQS queue `evalai_submission_queue` and listens to it for new submissions. +After the challenges are successfully loaded, it creates a connection with the SQS queue `arena_submission_queue` and listens to it for new submissions. ### How is submission made? diff --git a/scripts/workers/code_upload_submission_worker.py b/scripts/workers/code_upload_submission_worker.py index 22ab75dce1..d830057442 100755 --- a/scripts/workers/code_upload_submission_worker.py +++ b/scripts/workers/code_upload_submission_worker.py @@ -41,7 +41,7 @@ def exit_gracefully(self, signum, frame): EVALAI_API_SERVER = os.environ.get( "EVALAI_API_SERVER", "http://localhost:8000" ) -QUEUE_NAME = os.environ.get("QUEUE_NAME", "evalai_submission_queue") +QUEUE_NAME = os.environ.get("QUEUE_NAME", "arena_submission_queue") script_config_map_name = "evalai-scripts-cm" diff --git a/scripts/workers/remote_submission_worker.py b/scripts/workers/remote_submission_worker.py index 2252671458..9e988d64e4 100644 --- a/scripts/workers/remote_submission_worker.py +++ b/scripts/workers/remote_submission_worker.py @@ -28,7 +28,7 @@ AUTH_TOKEN = os.environ.get("AUTH_TOKEN") DJANGO_SERVER = os.environ.get("DJANGO_SERVER", "localhost") DJANGO_SERVER_PORT = os.environ.get("DJANGO_SERVER_PORT", "8000") -QUEUE_NAME = os.environ.get("QUEUE_NAME", "evalai_submission_queue") +QUEUE_NAME = os.environ.get("QUEUE_NAME", "arena_submission_queue") CHALLENGE_DATA_BASE_DIR = join(COMPUTE_DIRECTORY_PATH, "challenge_data") SUBMISSION_DATA_BASE_DIR = join(COMPUTE_DIRECTORY_PATH, "submission_files") diff --git a/scripts/workers/submission_worker.py b/scripts/workers/submission_worker.py index 38d662416d..fbc98bc206 100644 --- a/scripts/workers/submission_worker.py +++ b/scripts/workers/submission_worker.py @@ -796,7 +796,7 @@ def get_or_create_sqs_queue(queue_name, challenge=None): aws_access_key_id=os.environ.get("AWS_ACCESS_KEY_ID"), ) if queue_name == "": - queue_name = "evalai_submission_queue" + queue_name = "arena_submission_queue" # Check if the queue exists. If no, then create one try: queue = sqs.get_queue_by_name(QueueName=queue_name) @@ -806,9 +806,10 @@ def get_or_create_sqs_queue(queue_name, challenge=None): != "AWS.SimpleQueueService.NonExistentQueue" ): logger.exception("Cannot get queue: {}".format(queue_name)) + sqs_retention_period = SQS_RETENTION_PERIOD if challenge is None else str(challenge.sqs_retention_period) queue = sqs.create_queue( QueueName=queue_name, - Attributes={"MessageRetentionPeriod": SQS_RETENTION_PERIOD}, + Attributes={"MessageRetentionPeriod": sqs_retention_period}, ) return queue @@ -873,7 +874,7 @@ def main(): # create submission base data directory create_dir_as_python_package(SUBMISSION_DATA_BASE_DIR) - queue_name = os.environ.get("CHALLENGE_QUEUE", "evalai_submission_queue") + queue_name = os.environ.get("CHALLENGE_QUEUE", "arena_submission_queue") queue = get_or_create_sqs_queue(queue_name, challenge) is_remote = int(challenge.remote_evaluation) while True: diff --git a/tests/unit/challenges/test_views.py b/tests/unit/challenges/test_views.py index d8bfcbb9f7..5ea9939341 100644 --- a/tests/unit/challenges/test_views.py +++ b/tests/unit/challenges/test_views.py @@ -194,6 +194,7 @@ def test_get_challenge(self): "ephemeral_storage": self.challenge.ephemeral_storage, "worker_image_url": self.challenge.worker_image_url, "worker_instance_type": self.challenge.worker_instance_type, + "sqs_retention_period": self.challenge.sqs_retention_period, } ] @@ -548,6 +549,7 @@ def test_get_particular_challenge(self): "ephemeral_storage": self.challenge.ephemeral_storage, "worker_image_url": self.challenge.worker_image_url, "worker_instance_type": self.challenge.worker_instance_type, + "sqs_retention_period": self.challenge.sqs_retention_period, } response = self.client.get(self.url, {}) self.assertEqual(response.data, expected) @@ -649,6 +651,7 @@ def test_update_challenge_when_user_is_its_creator(self): "ephemeral_storage": self.challenge.ephemeral_storage, "worker_image_url": self.challenge.worker_image_url, "worker_instance_type": self.challenge.worker_instance_type, + "sqs_retention_period": self.challenge.sqs_retention_period, } response = self.client.put( self.url, {"title": new_title, "description": new_description} @@ -776,6 +779,7 @@ def test_particular_challenge_partial_update(self): "ephemeral_storage": self.challenge.ephemeral_storage, "worker_image_url": self.challenge.worker_image_url, "worker_instance_type": self.challenge.worker_instance_type, + "sqs_retention_period": self.challenge.sqs_retention_period, } response = self.client.patch(self.url, self.partial_update_data) self.assertEqual(response.data, expected) @@ -852,6 +856,7 @@ def test_particular_challenge_update(self): "ephemeral_storage": self.challenge.ephemeral_storage, "worker_image_url": self.challenge.worker_image_url, "worker_instance_type": self.challenge.worker_instance_type, + "sqs_retention_period": self.challenge.sqs_retention_period, } response = self.client.put(self.url, self.data) self.assertEqual(response.data, expected) @@ -1444,6 +1449,7 @@ def test_get_past_challenges(self): "ephemeral_storage": self.challenge3.ephemeral_storage, "worker_image_url": self.challenge3.worker_image_url, "worker_instance_type": self.challenge3.worker_instance_type, + "sqs_retention_period": self.challenge3.sqs_retention_period, } ] response = self.client.get(self.url, {}, format="json") @@ -1526,6 +1532,7 @@ def test_get_present_challenges(self): "ephemeral_storage": self.challenge.ephemeral_storage, "worker_image_url": self.challenge.worker_image_url, "worker_instance_type": self.challenge.worker_instance_type, + "sqs_retention_period": self.challenge.sqs_retention_period, } ] response = self.client.get(self.url, {}, format="json") @@ -1608,6 +1615,7 @@ def test_get_future_challenges(self): "ephemeral_storage": self.challenge4.ephemeral_storage, "worker_image_url": self.challenge4.worker_image_url, "worker_instance_type": self.challenge4.worker_instance_type, + "sqs_retention_period": self.challenge4.sqs_retention_period, } ] response = self.client.get(self.url, {}, format="json") @@ -1690,6 +1698,7 @@ def test_get_all_challenges(self): "ephemeral_storage": self.challenge3.ephemeral_storage, "worker_image_url": self.challenge3.worker_image_url, "worker_instance_type": self.challenge3.worker_instance_type, + "sqs_retention_period": self.challenge3.sqs_retention_period, }, { "id": self.challenge3.pk, @@ -1756,6 +1765,7 @@ def test_get_all_challenges(self): "ephemeral_storage": self.challenge3.ephemeral_storage, "worker_image_url": self.challenge3.worker_image_url, "worker_instance_type": self.challenge3.worker_instance_type, + "sqs_retention_period": self.challenge3.sqs_retention_period, }, { "id": self.challenge2.pk, @@ -1822,6 +1832,7 @@ def test_get_all_challenges(self): "ephemeral_storage": self.challenge2.ephemeral_storage, "worker_image_url": self.challenge2.worker_image_url, "worker_instance_type": self.challenge2.worker_instance_type, + "sqs_retention_period": self.challenge2.sqs_retention_period, }, ] response = self.client.get(self.url, {}, format="json") @@ -1959,6 +1970,7 @@ def test_get_featured_challenges(self): "ephemeral_storage": self.challenge3.ephemeral_storage, "worker_image_url": self.challenge3.worker_image_url, "worker_instance_type": self.challenge3.worker_instance_type, + "sqs_retention_period": self.challenge3.sqs_retention_period, } ] response = self.client.get(self.url, {}, format="json") @@ -2120,6 +2132,7 @@ def test_get_challenge_by_pk_when_user_is_challenge_host(self): "ephemeral_storage": self.challenge3.ephemeral_storage, "worker_image_url": self.challenge3.worker_image_url, "worker_instance_type": self.challenge3.worker_instance_type, + "sqs_retention_period": self.challenge3.sqs_retention_period, } response = self.client.get(self.url, {}) @@ -2210,6 +2223,7 @@ def test_get_challenge_by_pk_when_user_is_participant(self): "ephemeral_storage": self.challenge4.ephemeral_storage, "worker_image_url": self.challenge4.worker_image_url, "worker_instance_type": self.challenge4.worker_instance_type, + "sqs_retention_period": self.challenge4.sqs_retention_period, } self.client.force_authenticate(user=self.user1) @@ -2360,6 +2374,7 @@ def test_get_challenge_when_host_team_is_given(self): "ephemeral_storage": self.challenge2.ephemeral_storage, "worker_image_url": self.challenge2.worker_image_url, "worker_instance_type": self.challenge2.worker_instance_type, + "sqs_retention_period": self.challenge2.sqs_retention_period, } ] @@ -2438,6 +2453,7 @@ def test_get_challenge_when_participant_team_is_given(self): "ephemeral_storage": self.challenge2.ephemeral_storage, "worker_image_url": self.challenge2.worker_image_url, "worker_instance_type": self.challenge2.worker_instance_type, + "sqs_retention_period": self.challenge2.sqs_retention_period, } ] @@ -2516,6 +2532,7 @@ def test_get_challenge_when_mode_is_participant(self): "ephemeral_storage": self.challenge2.ephemeral_storage, "worker_image_url": self.challenge2.worker_image_url, "worker_instance_type": self.challenge2.worker_instance_type, + "sqs_retention_period": self.challenge2.sqs_retention_period, } ] @@ -2592,6 +2609,7 @@ def test_get_challenge_when_mode_is_host(self): "ephemeral_storage": self.challenge.ephemeral_storage, "worker_image_url": self.challenge.worker_image_url, "worker_instance_type": self.challenge.worker_instance_type, + "sqs_retention_period": self.challenge.sqs_retention_period, }, { "id": self.challenge2.pk, @@ -2658,6 +2676,7 @@ def test_get_challenge_when_mode_is_host(self): "ephemeral_storage": self.challenge2.ephemeral_storage, "worker_image_url": self.challenge2.worker_image_url, "worker_instance_type": self.challenge2.worker_instance_type, + "sqs_retention_period": self.challenge2.sqs_retention_period, }, ] @@ -6131,13 +6150,13 @@ def test_modify_leaderboard_data_with_other_parameters(self): self.assertEqual(response.status_code, status.HTTP_200_OK) -class TestUpdateChallenge(BaseAPITestClass): +class TestUpdateChallengeApproval(BaseAPITestClass): def setUp(self): settings.AWS_SES_REGION_NAME = "us-east-1" settings.AWS_SES_REGION_ENDPOINT = "email.us-east-1.amazonaws.com" return super().setUp() - def test_update_challenge_when_challenge_exists(self): + def test_update_challenge_approval_when_challenge_exists(self): self.user.is_staff = True self.user.save() self.url = reverse_lazy("challenges:update_challenge_approval") @@ -6151,7 +6170,7 @@ def test_update_challenge_when_challenge_exists(self): self.assertEqual(response.data, expected) self.assertEqual(response.status_code, status.HTTP_200_OK) - def test_update_challenge_when_not_a_staff(self): + def test_update_challenge_approval_when_not_a_staff(self): self.url = reverse_lazy("challenges:update_challenge_approval") self.user.is_staff = False self.user.save() @@ -6164,3 +6183,49 @@ def test_update_challenge_when_not_a_staff(self): }) self.assertEqual(response.data, expected) self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + +class TestUpdateChallengeAttributes(BaseAPITestClass): + def setUp(self): + settings.AWS_SES_REGION_NAME = "us-east-1" + settings.AWS_SES_REGION_ENDPOINT = "email.us-east-1.amazonaws.com" + return super().setUp() + + def test_update_challenge_attributes_when_challenge_exists(self): + self.url = reverse_lazy("challenges:update_challenge_attributes") + self.user.is_staff = True + self.user.save() + + expected = { + "message": f"Challenge attributes updated successfully for challenge with primary key {self.challenge.pk}!" + } + + response = self.client.post(self.url, { + "challenge_pk": self.challenge.pk, + "title": "Updated Title", + "description": "Updated Description", + "approved_by_admin": True, + "ephemeral_storage": 25, + }) + + self.assertEqual(response.data, expected) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + def test_update_challenge_attributes_when_not_a_staff(self): + self.url = reverse_lazy("challenges:update_challenge_attributes") + self.user.is_staff = False + self.user.save() + expected = { + "error": "Sorry, you are not authorized to access this resource!" + } + + response = self.client.post(self.url, { + "challenge_pk": self.challenge.pk, + "title": "Updated Title", + "description": "Updated Description", + "approved_by_admin": True, + "ephemeral_storage": 25, + }) + + self.assertEqual(response.data, expected) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) diff --git a/tests/unit/participants/test_views.py b/tests/unit/participants/test_views.py index 8c6bd1f2fe..f021cd48bd 100644 --- a/tests/unit/participants/test_views.py +++ b/tests/unit/participants/test_views.py @@ -887,6 +887,7 @@ def test_get_teams_and_corresponding_challenges_for_a_participant(self): "evaluation_module_error": self.challenge1.evaluation_module_error, "worker_image_url": self.challenge1.worker_image_url, "worker_instance_type": self.challenge1.worker_instance_type, + "sqs_retention_period": self.challenge1.sqs_retention_period, }, "participant_team": { "id": self.participant_team.id, @@ -980,6 +981,7 @@ def test_get_participant_team_challenge_list(self): "evaluation_module_error": self.challenge1.evaluation_module_error, "worker_image_url": self.challenge1.worker_image_url, "worker_instance_type": self.challenge1.worker_instance_type, + "sqs_retention_period": self.challenge1.sqs_retention_period, } ] diff --git a/tests/unit/remoteworker/test_remote_worker.py b/tests/unit/remoteworker/test_remote_worker.py index e961a65f85..c79119bafb 100644 --- a/tests/unit/remoteworker/test_remote_worker.py +++ b/tests/unit/remoteworker/test_remote_worker.py @@ -97,7 +97,7 @@ def test_make_request_post(self, mock_make_request): @mock.patch( "scripts.workers.remote_submission_worker.QUEUE_NAME", - "evalai_submission_queue", + "arena_submission_queue", ) @mock.patch( "scripts.workers.remote_submission_worker.return_url_per_environment" @@ -105,7 +105,7 @@ def test_make_request_post(self, mock_make_request): @mock.patch("scripts.workers.remote_submission_worker.make_request") class APICallsTestClass(BaseTestClass): def test_get_message_from_sqs_queue(self, mock_make_request, mock_url): - url = self.get_message_from_sqs_queue_url("evalai_submission_queue") + url = self.get_message_from_sqs_queue_url("arena_submission_queue") get_message_from_sqs_queue() mock_url.assert_called_with(url) url = mock_url(url) @@ -115,7 +115,7 @@ def test_delete_message_from_sqs_queue(self, mock_make_request, mock_url): test_receipt_handle = ( "MbZj6wDWli+JvwwJaBV+3dcjk2YW2vA3+STFFljTM8tJJg6HRG6PYSasuWXPJB+Cw" ) - url = self.delete_message_from_sqs_queue_url("evalai_submission_queue") + url = self.delete_message_from_sqs_queue_url("arena_submission_queue") delete_message_from_sqs_queue(test_receipt_handle) mock_url.assert_called_with(url) url = mock_url(url) @@ -125,7 +125,7 @@ def test_delete_message_from_sqs_queue(self, mock_make_request, mock_url): mock_make_request.assert_called_with(url, "POST", data=expected_data) def test_get_challenge_by_queue_name(self, mock_make_request, mock_url): - url = self.get_challenge_by_queue_name_url("evalai_submission_queue") + url = self.get_challenge_by_queue_name_url("arena_submission_queue") get_challenge_by_queue_name() mock_url.assert_called_with(url) url = mock_url(url)