From 9c971b14265008532d67e03271b45509e196556c Mon Sep 17 00:00:00 2001 From: Erwan Rouchet <rouchet@teklia.com> Date: Wed, 11 Dec 2024 14:16:20 +0000 Subject: [PATCH] Support multiline strings in user configurations --- arkindex/process/serializers/workers.py | 19 +- .../tests/worker_versions/test_create.py | 670 +++++------------- 2 files changed, 202 insertions(+), 487 deletions(-) diff --git a/arkindex/process/serializers/workers.py b/arkindex/process/serializers/workers.py index a2f77accb8..e969205e36 100644 --- a/arkindex/process/serializers/workers.py +++ b/arkindex/process/serializers/workers.py @@ -169,11 +169,12 @@ class UserConfigurationFieldSerializer(serializers.Serializer): subtype = EnumField(UserConfigurationFieldType, required=False) required = serializers.BooleanField(default=False) choices = serializers.ListField(required=False, allow_empty=False, allow_null=True) + multiline = serializers.BooleanField(required=False) def to_internal_value(self, data): errors = defaultdict(list) - allowed_fields = ["title", "type", "required", "default", "choices", "subtype"] + allowed_fields = ("title", "type", "required", "default", "choices", "subtype", "multiline") if not isinstance(data, dict): errors["__all__"] = [f"User configuration field definitions should be of type dict, not {type(data).__name__}."] @@ -182,7 +183,7 @@ class UserConfigurationFieldSerializer(serializers.Serializer): for field in data: if field not in allowed_fields: errors[field].append( - "Configurable properties can only be defined using the following keys: title, type, required, default, subtype, choices." + f"Configurable properties can only be defined using the following keys: {', '.join(allowed_fields)}." ) default_value = data.get("default") @@ -194,12 +195,14 @@ class UserConfigurationFieldSerializer(serializers.Serializer): if field_type == UserConfigurationFieldType.List and not subtype: errors["subtype"].append('The "subtype" field must be set for "list" type properties.') + # Handle subtypes if subtype is not None: if field_type != UserConfigurationFieldType.List: errors["subtype"].append('The "subtype" field can only be set for a "list" type property.') if subtype not in [UserConfigurationFieldType.Int, UserConfigurationFieldType.Float, UserConfigurationFieldType.String, UserConfigurationFieldType.Boolean]: errors["subtype"].append("Subtype can only be int, float, bool or string.") + # Handle enums if choices is not None: if field_type != UserConfigurationFieldType.Enum: @@ -207,6 +210,11 @@ class UserConfigurationFieldSerializer(serializers.Serializer): # If the configuration parameter is of enum type, an eventual default value won't match the field type if default_value and default_value not in choices: errors["default"].append(f"{default_value} is not an available choice.") + + # Only allow the multiline property on string fields + if "multiline" in data and field_type != UserConfigurationFieldType.String: + errors["multiline"].append('The "multiline" field can only be set for a "string" type property.') + # Handle everything else if default_value is not None and field_type != UserConfigurationFieldType.Enum: try: @@ -278,17 +286,16 @@ class WorkerVersionSerializer(serializers.ModelSerializer): return tag def validate_configuration(self, configuration): - errors = defaultdict(list) user_configuration = configuration.get("user_configuration") if not user_configuration: return configuration + field = serializers.DictField(child=UserConfigurationFieldSerializer()) try: field.to_internal_value(user_configuration) except ValidationError as e: - errors["user_configuration"].append(e.detail) - if errors: - raise ValidationError(errors) + raise ValidationError({"user_configuration": e.detail}) + return configuration def validate(self, data): diff --git a/arkindex/process/tests/worker_versions/test_create.py b/arkindex/process/tests/worker_versions/test_create.py index 15ca24971d..d060bb9e27 100644 --- a/arkindex/process/tests/worker_versions/test_create.py +++ b/arkindex/process/tests/worker_versions/test_create.py @@ -38,7 +38,7 @@ class TestWorkerVersionCreate(FixtureAPITestCase): public=False, ) - def test_create_non_existing_worker(self): + def test_non_existing_worker(self): with self.assertNumQueries(2): response = self.client.post( reverse("api:worker-versions", kwargs={"pk": "12341234-1234-1234-1234-123412341234"}), @@ -49,7 +49,7 @@ class TestWorkerVersionCreate(FixtureAPITestCase): self.assertDictEqual(response.json(), {"detail": "No Worker matches the given query."}) - def test_create_available_requires_docker_image(self): + def test_available_requires_docker_image(self): with self.assertNumQueries(2): response = self.client.post( reverse("api:worker-versions", kwargs={"pk": str(self.worker_reco.id)}), @@ -67,7 +67,7 @@ class TestWorkerVersionCreate(FixtureAPITestCase): ] }) - def test_create_invalid_docker_image(self): + def test_invalid_docker_image(self): with self.assertNumQueries(2): response = self.client.post( reverse("api:worker-versions", kwargs={"pk": str(self.worker_reco.id)}), @@ -86,7 +86,7 @@ class TestWorkerVersionCreate(FixtureAPITestCase): ] }) - def test_create_empty(self): + def test_empty(self): self.user.can_create_worker_version = True self.user.save() self.client.force_login(self.user) @@ -98,7 +98,7 @@ class TestWorkerVersionCreate(FixtureAPITestCase): self.assertDictEqual(response.json(), {"configuration": ["This field is required."]}) - def test_create_archived(self): + def test_archived(self): self.worker_reco.archived = datetime.now(timezone.utc) self.worker_reco.save() @@ -117,7 +117,7 @@ class TestWorkerVersionCreate(FixtureAPITestCase): "worker": ["This worker is archived."], }) - def test_create_null_revision_url_requires_null_worker_repository_url(self): + def test_null_revision_url_requires_null_worker_repository_url(self): self.worker_reco.repository_url = "https://gitlab.com/NERV/eva" self.worker_reco.save() self.user.can_create_worker_version = True @@ -135,7 +135,7 @@ class TestWorkerVersionCreate(FixtureAPITestCase): "revision_url": ["A revision url is required when creating a version for a worker linked to a repository."], }) - def test_create_null_revision_url_forbidden_task_auth(self): + def test_null_revision_url_forbidden_task_auth(self): """ Ponos Task auth cannot create a version on a worker that is not linked to a repository. """ @@ -152,7 +152,7 @@ class TestWorkerVersionCreate(FixtureAPITestCase): }) @patch("arkindex.users.utils.get_max_level", return_value=Role.Contributor.value) - def test_create_user_auth_requires_admin_access_to_worker(self, max_level_mock): + def test_user_auth_requires_admin_access_to_worker(self, max_level_mock): self.user.can_create_worker_version = True self.user.save() self.client.force_login(self.user) @@ -169,7 +169,7 @@ class TestWorkerVersionCreate(FixtureAPITestCase): self.assertEqual(max_level_mock.call_count, 1) self.assertEqual(max_level_mock.call_args, call(self.user, self.worker_reco)) - def test_create_user_auth_with_worker_repository_url_ok(self): + def test_user_auth_with_worker_repository_url_ok(self): self.user.can_create_worker_version = True self.user.save() self.client.force_login(self.user) @@ -208,7 +208,7 @@ class TestWorkerVersionCreate(FixtureAPITestCase): }, }) - def test_create_null_revision_url(self): + def test_null_revision_url(self): """ A worker version can be created with no revision_url through user authentication """ @@ -249,7 +249,7 @@ class TestWorkerVersionCreate(FixtureAPITestCase): }, }) - def test_create_configuration_wrong_type(self): + def test_configuration_wrong_type(self): """ Configuration body must be an object """ @@ -264,7 +264,7 @@ class TestWorkerVersionCreate(FixtureAPITestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(response.json(), {"configuration": ['Expected a dictionary of items but got type "str".']}) - def test_create_requires_permission(self): + def test_requires_permission(self): self.assertFalse(self.user.can_create_worker_version) self.client.force_login(self.user) response = self.client.post( @@ -289,7 +289,7 @@ class TestWorkerVersionCreate(FixtureAPITestCase): ) self.assertEqual(response.status_code, status.HTTP_201_CREATED) - def test_create_task_auth(self): + def test_task_auth(self): response = self.client.post( reverse("api:worker-versions", kwargs={"pk": str(self.worker_reco.id)}), data={ @@ -310,7 +310,7 @@ class TestWorkerVersionCreate(FixtureAPITestCase): self.assertEqual(data["gpu_usage"], "disabled") self.assertEqual(data["model_usage"], FeatureUsage.Required.value) - def test_create_duplicate_revision_url(self): + def test_duplicate_revision_url(self): self.version_1.revision_url = "https://gitlab.com/NERV/eva/commit/eva-01" self.version_1.version = None self.version_1.save() @@ -325,7 +325,7 @@ class TestWorkerVersionCreate(FixtureAPITestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertDictEqual(response.json(), {"revision_url": ["A version already exists for this worker with this revision_url."]}) - def test_create_with_tag(self): + def test_with_tag(self): with self.assertNumQueries(7): response = self.client.post( reverse("api:worker-versions", kwargs={"pk": str(self.worker_reco.id)}), @@ -349,7 +349,7 @@ class TestWorkerVersionCreate(FixtureAPITestCase): self.assertEqual(data["model_usage"], FeatureUsage.Required.value) self.assertEqual(data["tag"], "eva-01") - def test_create_unique_tag(self): + def test_unique_tag(self): self.version_1.tag = "eva-01" self.version_1.save() @@ -362,7 +362,7 @@ class TestWorkerVersionCreate(FixtureAPITestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertDictEqual(response.json(), {"tag": ["A version already exists for this worker with this tag."]}) - def test_create_unassign_branch(self): + def test_unassign_branch(self): self.version_1.branch = "operation-yashima" self.version_1.save() self.version_2.branch = "operation-yashima" @@ -389,7 +389,7 @@ class TestWorkerVersionCreate(FixtureAPITestCase): self.version_2.refresh_from_db() self.assertEqual(self.version_2.branch, "operation-yashima") - def test_create_empty_tag_branch(self): + def test_empty_tag_branch(self): with self.assertNumQueries(3): response = self.client.post( reverse("api:worker-versions", kwargs={"pk": str(self.worker_reco.id)}), @@ -407,7 +407,7 @@ class TestWorkerVersionCreate(FixtureAPITestCase): "branch": ["This field may not be blank."], }) - def test_create_wrong_gpu_usage(self): + def test_wrong_gpu_usage(self): response = self.client.post( reverse("api:worker-versions", kwargs={"pk": str(self.worker_reco.id)}), data={"configuration": {"test": "test2"}, "revision_url": "https://gitlab.com/NERV/eva/commit/eva-01", "gpu_usage": "not_supported"}, @@ -415,7 +415,7 @@ class TestWorkerVersionCreate(FixtureAPITestCase): ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - def test_create_no_user_configuration_ok(self): + def test_no_user_configuration_ok(self): response = self.client.post( reverse("api:worker-versions", kwargs={"pk": str(self.worker_dla.id)}), data={ @@ -427,475 +427,192 @@ class TestWorkerVersionCreate(FixtureAPITestCase): ) self.assertEqual(response.status_code, status.HTTP_201_CREATED) - def test_create_valid_user_configuration(self): - response = self.client.post( - reverse("api:worker-versions", kwargs={"pk": str(self.worker_dla.id)}), - data={ - "configuration": { - "user_configuration": { - "demo_integer": {"title": "Demo Integer", "type": "int", "required": True, "default": 1}, - "demo_boolean": {"title": "Demo Boolean", "type": "bool", "required": False, "default": True}, - } - }, - "gpu_usage": "disabled", - "revision_url": "https://gitlab.com/NERV/eva/commit/eva-01", + def test_valid_user_configuration(self): + user_configuration = { + "demo_integer": { + "title": "Demo Integer", + "type": "int", + "required": True, + "default": 1, }, - HTTP_AUTHORIZATION=f"Ponos {self.task.token}", - ) - self.assertEqual(response.status_code, status.HTTP_201_CREATED) - self.assertEqual(response.json()["configuration"], { - "user_configuration": { - "demo_integer": { - "title": "Demo Integer", - "type": "int", - "required": True, - "default": 1 - }, - "demo_boolean": { - "title": "Demo Boolean", - "type": "bool", - "required": False, - "default": True - } - } - }) - - def test_create_valid_user_configuration_dict(self): - response = self.client.post( - reverse("api:worker-versions", kwargs={"pk": str(self.worker_dla.id)}), - data={ - "configuration": { - "user_configuration": { - "demo_dict": {"title": "Demo Dict", "type": "dict", "required": True, "default": {"a": "b", "c": "d"}}, - } - }, - "revision_url": "https://gitlab.com/NERV/eva/commit/eva-01", - "gpu_usage": "disabled", + "demo_boolean": { + "title": "Demo Boolean", + "type": "bool", + "required": False, + "default": True, }, - HTTP_AUTHORIZATION=f"Ponos {self.task.token}", - ) - self.assertEqual(response.status_code, status.HTTP_201_CREATED) - self.assertEqual(response.json()["configuration"], { - "user_configuration": { - "demo_dict": { - "title": "Demo Dict", - "type": "dict", - "required": True, - "default": {"a": "b", "c": "d"} - } - } - }) - - def test_create_user_configuration_dict_strings_only(self): - response = self.client.post( - reverse("api:worker-versions", kwargs={"pk": str(self.worker_dla.id)}), - data={ - "configuration": { - "user_configuration": { - "demo_dict": {"title": "Demo Dict", "type": "dict", "required": True, "default": {"a": ["12", "13"], "c": "d"}}, - } - }, - "gpu_usage": "disabled", + "demo_dict": { + "title": "Demo Dict", + "type": "dict", + "required": True, + "default": {"a": "b", "c": "d"}, }, - HTTP_AUTHORIZATION=f"Ponos {self.task.token}", - ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - - def test_create_valid_user_configuration_enum(self): - response = self.client.post( - reverse("api:worker-versions", kwargs={"pk": str(self.worker_dla.id)}), - data={ - "configuration": { - "user_configuration": { - "demo_choice": {"title": "Decisions", "type": "enum", "required": True, "default": 1, "choices": [1, 2, 3]} - } - }, - "revision_url": "https://gitlab.com/NERV/eva/commit/eva-01", - "gpu_usage": "disabled", + "demo_choice": { + "title": "Decisions", + "type": "enum", + "required": True, + "default": 1, + "choices": [1, 2, 3], }, - HTTP_AUTHORIZATION=f"Ponos {self.task.token}", - ) - self.assertEqual(response.status_code, status.HTTP_201_CREATED) - self.assertEqual(response.json()["configuration"], { - "user_configuration": { - "demo_choice": { - "title": "Decisions", - "type": "enum", - "required": True, - "default": 1, - "choices": [1, 2, 3] - } - } - }) - - def test_create_valid_user_configuration_list(self): - response = self.client.post( - reverse("api:worker-versions", kwargs={"pk": str(self.worker_dla.id)}), - data={ - "configuration": { - "user_configuration": { - "demo_list": {"title": "Demo List", "type": "list", "required": True, "subtype": "int", "default": [1, 2, 3, 4]}, - "boolean_list": {"title": "It's a list of booleans", "type": "list", "required": False, "subtype": "bool", "default": [True, False, False]} - } - }, - "revision_url": "https://gitlab.com/NERV/eva/commit/eva-01", - "gpu_usage": "disabled", + "demo_list": { + "title": "Demo List", + "type": "list", + "required": True, + "subtype": "int", + "default": [1, 2, 3, 4], }, - HTTP_AUTHORIZATION=f"Ponos {self.task.token}", - ) - self.assertEqual(response.status_code, status.HTTP_201_CREATED) - self.assertEqual(response.json()["configuration"], { - "user_configuration": { - "demo_list": { - "title": "Demo List", - "type": "list", - "subtype": "int", - "required": True, - "default": [1, 2, 3, 4] - }, - "boolean_list": { - "title": "It's a list of booleans", - "type": "list", - "subtype": "bool", - "required": False, - "default": [True, False, False] - } - } - }) - - def test_create_valid_user_configuration_model(self): - response = self.client.post( - reverse("api:worker-versions", kwargs={"pk": str(self.worker_dla.id)}), - data={ - "configuration": { - "user_configuration": { - "demo_model": {"title": "Model for training", "type": "model", "required": True}, - "other_model": {"title": "Model the second", "type": "model", "default": str(self.model.id)} - } - }, - "revision_url": "https://gitlab.com/NERV/eva/commit/eva-01", - "gpu_usage": "disabled", + "boolean_list": { + "title": "It's a list of booleans", + "type": "list", + "required": False, + "subtype": "bool", + "default": [True, False, False], }, - HTTP_AUTHORIZATION=f"Ponos {self.task.token}", - ) - self.assertEqual(response.status_code, status.HTTP_201_CREATED) - self.assertEqual(response.json()["configuration"], { - "user_configuration": { - "demo_model": { - "title": "Model for training", - "type": "model", - "required": True - }, - "other_model": { - "title": "Model the second", - "type": "model", - "default": str(self.model.id) - } - } - }) - - def test_create_invalid_user_configuration_list_requires_subtype(self): - response = self.client.post( - reverse("api:worker-versions", kwargs={"pk": str(self.worker_dla.id)}), - data={ - "configuration": { - "user_configuration": { - "demo_list": {"title": "Demo List", "type": "list", "required": True, "default": [1, 2, 3, 4]}, - } - }, - "gpu_usage": "disabled", + "demo_model": { + "title": "Model for training", + "type": "model", + "required": True, }, - HTTP_AUTHORIZATION=f"Ponos {self.task.token}", - ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), { - "configuration": { - "user_configuration": [{ - "demo_list": { - "subtype": ['The "subtype" field must be set for "list" type properties.'] - } - }] - } - }) - - def test_create_invalid_user_configuration_list_wrong_default(self): - response = self.client.post( - reverse("api:worker-versions", kwargs={"pk": str(self.worker_dla.id)}), - data={ - "configuration": { - "user_configuration": { - "demo_list": {"title": "Demo List", "type": "list", "required": True, "subtype": "int", "default": 12}, - } - }, - "gpu_usage": "disabled", - }, - HTTP_AUTHORIZATION=f"Ponos {self.task.token}", - ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), { - "configuration": { - "user_configuration": [{ - "demo_list": { - "default": ["This is not a valid value for a field of type list."] - } - }] - } - }) - - def test_create_invalid_user_configuration_list_wrong_subtype(self): - response = self.client.post( - reverse("api:worker-versions", kwargs={"pk": str(self.worker_dla.id)}), - data={ - "configuration": { - "user_configuration": { - "demo_list": {"title": "Demo List", "type": "list", "required": True, "subtype": "dict", "default": [1, 2, 3, 4]}, - } - }, - "gpu_usage": "disabled", + "other_model": { + "title": "Model the second", + "type": "model", + "default": str(self.model.id), }, - HTTP_AUTHORIZATION=f"Ponos {self.task.token}", - ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), { - "configuration": { - "user_configuration": [{ - "demo_list": { - "subtype": ["Subtype can only be int, float, bool or string."] - } - }] + "multiline_string": { + "title": "Multiline string", + "type": "string", + "multiline": True, + "required": True, } - }) + } - def test_create_invalid_user_configuration_list_wrong_default_subtype(self): response = self.client.post( reverse("api:worker-versions", kwargs={"pk": str(self.worker_dla.id)}), data={ "configuration": { - "user_configuration": { - "demo_list": {"title": "Demo List", "type": "list", "required": True, "subtype": "int", "default": [1, 2, "three", 4]}, - } + "user_configuration": user_configuration, }, - "gpu_usage": "disabled", - }, - HTTP_AUTHORIZATION=f"Ponos {self.task.token}", - ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), { - "configuration": { - "user_configuration": [{ - "demo_list": { - "default": ["All items in the default value must be of type int."] - } - }] - } - }) - - def test_create_invalid_user_configuration_not_list_choices(self): - response = self.client.post( - reverse("api:worker-versions", kwargs={"pk": str(self.worker_dla.id)}), - data={ - "configuration": { - "user_configuration": { - "demo_choice": {"title": "Decisions", "type": "enum", "required": True, "default": 1, "choices": "eeee"} - } - }, - "gpu_usage": "disabled", - }, - HTTP_AUTHORIZATION=f"Ponos {self.task.token}", - ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), { - "configuration": { - "user_configuration": [{ - "demo_choice": { - "choices": ['Expected a list of items but got type "str".'] - } - }] - } - }) - - def test_create_invalid_user_configuration_not_dict(self): - response = self.client.post( - reverse("api:worker-versions", kwargs={"pk": str(self.worker_dla.id)}), - data={ - "configuration": { - "user_configuration": "non" - }, - "gpu_usage": "disabled", + "revision_url": "https://gitlab.com/NERV/eva/commit/eva-01", }, HTTP_AUTHORIZATION=f"Ponos {self.task.token}", ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), {"configuration": {"user_configuration": [['Expected a dictionary of items but got type "str".']]}}) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) - def test_create_invalid_user_configuration_item_not_dict(self): - response = self.client.post( - reverse("api:worker-versions", kwargs={"pk": str(self.worker_dla.id)}), - data={ - "configuration": { - "user_configuration": { - "secrets": ["aaaaaa"] - } - }, - "gpu_usage": "disabled", - }, - HTTP_AUTHORIZATION=f"Ponos {self.task.token}", - ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), {"configuration": { - "user_configuration": [{ - "secrets": - {"__all__": ["User configuration field definitions should be of type dict, not list."]} - }] - }}) - - def test_create_invalid_user_configuration_wrong_field_type(self): - response = self.client.post( - reverse("api:worker-versions", kwargs={"pk": str(self.worker_dla.id)}), - data={ - "configuration": { - "user_configuration": { - "something": { - "title": "some thing", - "type": "uh oh", - "required": 2 - } - } - }, - "gpu_usage": "disabled", - }, - HTTP_AUTHORIZATION=f"Ponos {self.task.token}", - ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), { - "configuration": { - "user_configuration": [{ - "something": { - "required": ["Must be a valid boolean."], - "type": ["Value is not of type UserConfigurationFieldType"] - } - }] - } + data = response.json() + self.assertEqual(data["configuration"], { + "user_configuration": user_configuration, }) - - def test_create_invalid_user_configuration_wrong_default_type(self): - response = self.client.post( - reverse("api:worker-versions", kwargs={"pk": str(self.worker_dla.id)}), - data={ - "configuration": { - "user_configuration": { - "one_float": { - "title": "a float", - "type": "float", - "default": "bonjour", - "required": True - } - } - }, - "gpu_usage": "disabled", - }, - HTTP_AUTHORIZATION=f"Ponos {self.task.token}", - ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), { - "configuration": { - "user_configuration": [{ - "one_float": { - "default": ["This is not a valid value for a field of type float."] - } - }] - } + self.assertEqual(WorkerVersion.objects.get(id=data["id"]).configuration, { + "user_configuration": user_configuration, }) - def test_create_invalid_user_configuration_choices_no_enum(self): - response = self.client.post( - reverse("api:worker-versions", kwargs={"pk": str(self.worker_dla.id)}), - data={ - "configuration": { - "user_configuration": { - "something": { - "title": "some thing", - "type": "int", - "required": False, - "choices": [1, 2, 3] - } - } - }, - "gpu_usage": "disabled", - }, - HTTP_AUTHORIZATION=f"Ponos {self.task.token}", - ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), { - "configuration": { - "user_configuration": [{ - "something": { - "choices": ['The "choices" field can only be set for an "enum" type property.'] - } - }] - } - }) + def test_invalid_user_configuration(self): + cases = [ + ( + # User configuration must be a dict + "non", + ['Expected a dictionary of items but got type "str".'], + ), + ( + # User a configuration must a dict of dicts + {"something": ["aaaaa"]}, + {"something": {"__all__": ["User configuration field definitions should be of type dict, not list."]}}, + ), + ( + # Fields have required keys + {"something": {}}, + {"something": { + "title": ["This field is required."], + "type": ["This field is required."], + }}, + ), + ( + # Unsupported keys cause errors + {"something": {"title": "some thing", "type": "int", "required": True, "some_key": "some_value"}}, + {"something": { + "some_key": [ + "Configurable properties can only be defined using the following keys: " + "title, type, required, default, choices, subtype, multiline." + ], + }} + ), + ( + # Field types should be valid + {"something": {"title": "some thing", "type": "uh oh", "required": True}}, + {"something": {"type": ["Value is not of type UserConfigurationFieldType"]}}, + ), + ( + # `required` should be a boolean + {"something": {"title": "some thing", "type": "string", "required": "maybe"}}, + {"something": {"required": ["Must be a valid boolean."]}}, + ), + ( + # Field defaults must match their field types + {"something": {"title": "some thing", "type": "float", "required": False, "default": "bonjour"}}, + {"something": {"default": ["This is not a valid value for a field of type float."]}}, + ), + ( + # Dict field defaults can only contain strings + {"demo_dict": {"title": "Demo Dict", "type": "dict", "required": True, "default": {"a": ["12", "13"], "c": "d"}}}, + {"demo_dict": {"default": ["This is not a valid value for a field of type dict."]}}, + ), + ( + # List fields require a subtype + {"demo_list": {"title": "Demo List", "type": "list", "required": True, "default": [1, 2, 3, 4]}}, + {"demo_list": {"subtype": ['The "subtype" field must be set for "list" type properties.']}}, + ), + ( + # List field subtypes are restricted + {"demo_list": {"title": "Demo List", "type": "list", "required": True, "subtype": "dict", "default": [{"a": "b"}]}}, + {"demo_list": {"subtype": ["Subtype can only be int, float, bool or string."]}}, + ), + ( + # List field defaults must match the subtype + {"demo_list": {"title": "Demo List", "type": "list", "required": True, "subtype": "int", "default": [1, 2, "three", 4]}}, + {"demo_list": {"default": ["All items in the default value must be of type int."]}}, + ), + ( + # Choices can only be set on enums + {"something": {"title": "some thing", "type": "int", "required": False, "choices": [1, 2, 3]}}, + {"something": {"choices": ['The "choices" field can only be set for an "enum" type property.']}}, + ), + ( + # Enum field choices must be a list + {"demo_choice": {"title": "Decisions", "type": "enum", "required": True, "default": 1, "choices": "eeee"}}, + {"demo_choice": {"choices": ['Expected a list of items but got type "str".']}}, + ), + ( + # Model field defaults must be existing model IDs + {"model": {"title": "Model to train", "type": "model", "default": "12341234-1234-1234-1234-123412341234"}}, + {"model": {"default": ["Model 12341234-1234-1234-1234-123412341234 not found."]}} + ), + ( + {"stringn't": {"title": "Stringn't", "type": "int", "multiline": True}}, + {"stringn't": {"multiline": ['The "multiline" field can only be set for a "string" type property.']}}, + ), + ( + # `multiline` should be a boolean + {"string": {"title": "String", "type": "string", "multiline": "only during the full moon"}}, + {"string": {"multiline": ["Must be a valid boolean."]}}, + ), + ] - def test_create_invalid_user_configuration_missing_key(self): - response = self.client.post( - reverse("api:worker-versions", kwargs={"pk": str(self.worker_dla.id)}), - data={ - "configuration": { - "user_configuration": { - "demo_integer": {"type": "int", "required": True, "default": 1} - } - }, - "gpu_usage": "disabled", - }, - HTTP_AUTHORIZATION=f"Ponos {self.task.token}", - ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual( - response.json(), - { - "configuration": { - "user_configuration": [{ - "demo_integer": { - "title": ["This field is required."] - } - }] - } - } - ) + self.client.force_login(self.superuser) - def test_create_invalid_user_configuration_invalid_key(self): - response = self.client.post( - reverse("api:worker-versions", kwargs={"pk": str(self.worker_dla.id)}), - data={ - "configuration": { - "user_configuration": { - "demo_integer": { - "title": "an integer", - "type": "int", - "required": True, - "default": 1, - "some_key": "oh no", - } - } - }, - "gpu_usage": "disabled", - }, - HTTP_AUTHORIZATION=f"Ponos {self.task.token}", - ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), { - "configuration": { - "user_configuration": [ - { - "demo_integer": { - "some_key": ["Configurable properties can only be defined using the following keys: title, type, required, default, subtype, choices."] - } - } - ] - } - }) + for user_configuration, expected_errors in cases: + with self.subTest(user_configuration=user_configuration): + response = self.client.post( + reverse("api:worker-versions", kwargs={"pk": str(self.worker_dla.id)}), + data={ + "configuration": { + "user_configuration": user_configuration, + }, + }, + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.json(), {"configuration": {"user_configuration": expected_errors}}) - def test_create_invalid_user_configuration_default_value(self): + def test_invalid_user_configuration_default_value(self): cases = [ ({"type": "int", "default": False}, "int"), ({"type": "int", "default": True}, "int"), @@ -922,21 +639,12 @@ class TestWorkerVersionCreate(FixtureAPITestCase): HTTP_AUTHORIZATION=f"Ponos {self.task.token}", ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), {"configuration": {"user_configuration": [{"param": {"default": [f"This is not a valid value for a field of type {expected}."]}}]}}) - - def test_create_user_configuration_model_default_doesnt_exist(self): - response = self.client.post( - reverse("api:worker-versions", kwargs={"pk": str(self.worker_dla.id)}), - data={ - "configuration": { - "user_configuration": { - "param": {"title": "Model to train", "type": "model", "default": "12341234-1234-1234-1234-123412341234"} + self.assertEqual(response.json(), { + "configuration": { + "user_configuration": { + "param": { + "default": [f"This is not a valid value for a field of type {expected}."], + }, + }, } - }, - }, - HTTP_AUTHORIZATION=f"Ponos {self.task.token}", - ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), {"configuration": {"user_configuration": [{"param": {"default": [ - "Model 12341234-1234-1234-1234-123412341234 not found." - ]}}]}}) + }) -- GitLab