Skip to content

Commit

Permalink
[Refactor] Remove settings name pattern restrictions (#5573)
Browse files Browse the repository at this point in the history
# Description
<!-- Please include a summary of the changes and the related issue.
Please also include relevant motivation and context. List any
dependencies that are required for this change. -->

This PR removes name pattern validation for field, question,
metadata-property and vector-settings

**Type of change**
<!-- Please delete options that are not relevant. Remember to title the
PR according to the type of change -->

- Refactor (change restructuring the codebase without changing
functionality)
- Improvement (change adding some improvement to an existing
functionality)

**How Has This Been Tested**
<!-- Please add some reference about how your feature has been tested.
-->

**Checklist**
<!-- Please go over the list and make sure you've taken everything into
account -->

- I added relevant documentation
- I followed the style guidelines of this project
- I did a self-review of my code
- I made corresponding changes to the documentation
- I confirm My changes generate no new warnings
- I have added tests that prove my fix is effective or that my feature
works
- I have added relevant notes to the CHANGELOG.md file (See
https://keepachangelog.com/)
  • Loading branch information
frascuchon authored Oct 7, 2024
1 parent 1427abb commit c25c88c
Show file tree
Hide file tree
Showing 20 changed files with 275 additions and 442 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/argilla.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
build:
services:
argilla-server:
image: argilladev/argilla-hf-spaces:develop
image: argilladev/argilla-hf-spaces:pr-5573
ports:
- 6900:6900
env:
Expand Down
7 changes: 7 additions & 0 deletions argilla-server/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ These are the section headers that we use:

- Now it is possible to publish a dataset without required fields. Allowing being published with at least one field (required or not). ([#5569](https://github.com/argilla-io/argilla/pull/5569))

### Removed

- Removed name pattern restrictions for Fields. ([#5573](https://github.com/argilla-io/argilla/pull/5573))
- Removed name pattern restrictions for Questions. ([#5573](https://github.com/argilla-io/argilla/pull/5573))
- Removed name pattern restrictions for Metadata Properties. ([#5573](https://github.com/argilla-io/argilla/pull/5573))
- Removed name pattern restrictions for Vector Settings. ([#5573](https://github.com/argilla-io/argilla/pull/5573))

## [2.3.0](https://github.com/argilla-io/argilla/compare/v2.2.0...v2.3.0)

### Added
Expand Down
2 changes: 0 additions & 2 deletions argilla-server/src/argilla_server/api/schemas/v1/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
from argilla_server.pydantic_v1 import BaseModel, constr
from argilla_server.pydantic_v1 import Field as PydanticField

FIELD_CREATE_NAME_REGEX = r"^(?=.*[a-z0-9])[a-z0-9_-]+$"
FIELD_CREATE_NAME_MIN_LENGTH = 1
FIELD_CREATE_NAME_MAX_LENGTH = 200

Expand All @@ -30,7 +29,6 @@

FieldName = Annotated[
constr(
regex=FIELD_CREATE_NAME_REGEX,
min_length=FIELD_CREATE_NAME_MIN_LENGTH,
max_length=FIELD_CREATE_NAME_MAX_LENGTH,
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

FLOAT_METADATA_METRICS_PRECISION = 5

METADATA_PROPERTY_CREATE_NAME_REGEX = r"^(?=.*[a-z0-9])[a-z0-9_-]+$"
METADATA_PROPERTY_CREATE_NAME_MIN_LENGTH = 1
METADATA_PROPERTY_CREATE_NAME_MAX_LENGTH = 200

Expand Down Expand Up @@ -103,7 +102,6 @@ class FloatMetadataProperty(BaseModel):
str,
Field(
...,
regex=METADATA_PROPERTY_CREATE_NAME_REGEX,
min_length=METADATA_PROPERTY_CREATE_NAME_MIN_LENGTH,
max_length=METADATA_PROPERTY_CREATE_NAME_MAX_LENGTH,
),
Expand Down
2 changes: 0 additions & 2 deletions argilla-server/src/argilla_server/api/schemas/v1/questions.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
from typing_extensions import Annotated


QUESTION_CREATE_NAME_REGEX = r"^(?=.*[a-z0-9])[a-z0-9_-]+$"
QUESTION_CREATE_NAME_MIN_LENGTH = 1
QUESTION_CREATE_NAME_MAX_LENGTH = 200

Expand Down Expand Up @@ -287,7 +286,6 @@ class SpanQuestionSettingsUpdate(UpdateSchema):

QuestionName = Annotated[
constr(
regex=QUESTION_CREATE_NAME_REGEX,
min_length=QUESTION_CREATE_NAME_MIN_LENGTH,
max_length=QUESTION_CREATE_NAME_MAX_LENGTH,
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
from argilla_server.errors.future import UnprocessableEntityError
from argilla_server.pydantic_v1 import BaseModel, Field, PositiveInt, constr

VECTOR_SETTINGS_CREATE_NAME_REGEX = r"^(?=.*[a-z0-9])[a-z0-9_-]+$"
VECTOR_SETTINGS_CREATE_NAME_MIN_LENGTH = 1
VECTOR_SETTINGS_CREATE_NAME_MAX_LENGTH = 200

Expand Down Expand Up @@ -63,7 +62,6 @@ class VectorsSettings(BaseModel):
class VectorSettingsCreate(BaseModel):
name: str = Field(
...,
regex=VECTOR_SETTINGS_CREATE_NAME_REGEX,
min_length=VECTOR_SETTINGS_CREATE_NAME_MIN_LENGTH,
max_length=VECTOR_SETTINGS_CREATE_NAME_MAX_LENGTH,
description="The title of the vector settings",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,259 @@
# Copyright 2021-present, the Recognai S.L. team.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from datetime import datetime
from typing import TYPE_CHECKING
from uuid import UUID, uuid4

import pytest
from sqlalchemy import func, select

from argilla_server.api.schemas.v1.fields import FIELD_CREATE_NAME_MAX_LENGTH, FIELD_CREATE_TITLE_MAX_LENGTH
from argilla_server.constants import API_KEY_HEADER_NAME
from argilla_server.enums import (
DatasetStatus,
)
from argilla_server.models import (
Field,
)
from tests.factories import (
AdminFactory,
AnnotatorFactory,
DatasetFactory,
FieldFactory,
WorkspaceFactory,
)

if TYPE_CHECKING:
from httpx import AsyncClient
from sqlalchemy.ext.asyncio import AsyncSession


@pytest.mark.asyncio
class TestCreateDatasetField:
@pytest.mark.parametrize(
("settings", "expected_settings"),
[
({"type": "text"}, {"type": "text", "use_markdown": False}),
({"type": "text", "discarded": "value"}, {"type": "text", "use_markdown": False}),
({"type": "text", "use_markdown": False}, {"type": "text", "use_markdown": False}),
],
)
async def test_create_dataset_field(
self,
async_client: "AsyncClient",
db: "AsyncSession",
owner_auth_header: dict,
settings: dict,
expected_settings: dict,
):
dataset = await DatasetFactory.create()
field_json = {"name": "name", "title": "title", "settings": settings}

response = await async_client.post(
f"/api/v1/datasets/{dataset.id}/fields", headers=owner_auth_header, json=field_json
)

assert response.status_code == 201
assert (await db.execute(select(func.count(Field.id)))).scalar() == 1

response_body = response.json()
assert await db.get(Field, UUID(response_body["id"]))
assert response_body == {
"id": str(UUID(response_body["id"])),
"name": "name",
"title": "title",
"required": False,
"settings": expected_settings,
"dataset_id": str(dataset.id),
"inserted_at": datetime.fromisoformat(response_body["inserted_at"]).isoformat(),
"updated_at": datetime.fromisoformat(response_body["updated_at"]).isoformat(),
}

async def test_create_dataset_field_without_authentication(self, async_client: "AsyncClient", db: "AsyncSession"):
dataset = await DatasetFactory.create()
field_json = {
"name": "name",
"title": "title",
"settings": {"type": "text"},
}

response = await async_client.post(f"/api/v1/datasets/{dataset.id}/fields", json=field_json)

assert response.status_code == 401
assert (await db.execute(select(func.count(Field.id)))).scalar() == 0

async def test_create_dataset_field_as_admin(self, async_client: "AsyncClient", db: "AsyncSession"):
workspace = await WorkspaceFactory.create()
admin = await AdminFactory.create(workspaces=[workspace])
dataset = await DatasetFactory.create(workspace=workspace)
field_json = {
"name": "name",
"title": "title",
"settings": {"type": "text"},
}

response = await async_client.post(
f"/api/v1/datasets/{dataset.id}/fields",
headers={API_KEY_HEADER_NAME: admin.api_key},
json=field_json,
)

assert response.status_code == 201
assert (await db.execute(select(func.count(Field.id)))).scalar() == 1

async def test_create_dataset_field_as_annotator(self, async_client: "AsyncClient", db: "AsyncSession"):
annotator = await AnnotatorFactory.create()
dataset = await DatasetFactory.create()
field_json = {
"name": "name",
"title": "title",
"settings": {"type": "text"},
}

response = await async_client.post(
f"/api/v1/datasets/{dataset.id}/fields",
headers={API_KEY_HEADER_NAME: annotator.api_key},
json=field_json,
)

assert response.status_code == 403
assert (await db.execute(select(func.count(Field.id)))).scalar() == 0

async def test_create_dataset_field_with_invalid_max_length_name(
self, async_client: "AsyncClient", db: "AsyncSession", owner_auth_header: dict
):
dataset = await DatasetFactory.create()
field_json = {
"name": "a" * (FIELD_CREATE_NAME_MAX_LENGTH + 1),
"title": "title",
"settings": {"type": "text"},
}

response = await async_client.post(
f"/api/v1/datasets/{dataset.id}/fields", headers=owner_auth_header, json=field_json
)

assert response.status_code == 422
# assert db.query(Field).count() == 0
assert (await db.execute(select(func.count(Field.id)))).scalar() == 0

async def test_create_dataset_field_with_invalid_max_length_title(
self, async_client: "AsyncClient", db: "AsyncSession", owner_auth_header: dict
):
dataset = await DatasetFactory.create()
field_json = {
"name": "name",
"title": "a" * (FIELD_CREATE_TITLE_MAX_LENGTH + 1),
"settings": {"type": "text"},
}

response = await async_client.post(
f"/api/v1/datasets/{dataset.id}/fields", headers=owner_auth_header, json=field_json
)

assert response.status_code == 422
assert (await db.execute(select(func.count(Field.id)))).scalar() == 0

@pytest.mark.parametrize(
"settings",
[
{},
None,
{"type": "wrong-type"},
{"type": "text", "use_markdown": None},
{"type": "rating", "options": None},
{"type": "rating", "options": []},
],
)
async def test_create_dataset_field_with_invalid_settings(
self, async_client: "AsyncClient", db: "AsyncSession", owner_auth_header: dict, settings: dict
):
dataset = await DatasetFactory.create()
field_json = {
"name": "name",
"title": "Title",
"settings": settings,
}

response = await async_client.post(
f"/api/v1/datasets/{dataset.id}/fields", headers=owner_auth_header, json=field_json
)

assert response.status_code == 422
assert (await db.execute(select(func.count(Field.id)))).scalar() == 0

async def test_create_dataset_field_with_existent_name(
self, async_client: "AsyncClient", db: "AsyncSession", owner_auth_header: dict
):
field = await FieldFactory.create(name="name")

response = await async_client.post(
f"/api/v1/datasets/{field.dataset_id}/fields",
headers=owner_auth_header,
json={
"name": "name",
"title": "title",
"settings": {"type": "text"},
},
)

assert response.status_code == 409
assert response.json() == {
"detail": f"Field with name `{field.name}` already exists for dataset with id `{field.dataset_id}`"
}

assert (await db.execute(select(func.count(Field.id)))).scalar() == 1

async def test_create_dataset_field_with_published_dataset(
self, async_client: "AsyncClient", db: "AsyncSession", owner_auth_header: dict
):
dataset = await DatasetFactory.create(status=DatasetStatus.ready)

response = await async_client.post(
f"/api/v1/datasets/{dataset.id}/fields",
headers=owner_auth_header,
json={
"name": "name",
"title": "title",
"settings": {"type": "text"},
},
)

assert response.status_code == 422
assert response.json() == {"detail": "Field cannot be created for a published dataset"}

assert (await db.execute(select(func.count(Field.id)))).scalar() == 0

async def test_create_dataset_field_with_nonexistent_dataset_id(
self, async_client: "AsyncClient", db: "AsyncSession", owner_auth_header: dict
):
dataset_id = uuid4()

await DatasetFactory.create()

response = await async_client.post(
f"/api/v1/datasets/{dataset_id}/fields",
headers=owner_auth_header,
json={
"name": "text",
"title": "Text",
"settings": {"type": "text"},
},
)

assert response.status_code == 404
assert response.json() == {"detail": f"Dataset with id `{dataset_id}` not found"}

assert (await db.execute(select(func.count(Field.id)))).scalar() == 0
Original file line number Diff line number Diff line change
Expand Up @@ -329,24 +329,6 @@ async def test_create_dataset_question_as_annotator(self, async_client: "AsyncCl
assert response.status_code == 403
assert (await db.execute(select(func.count(Question.id)))).scalar() == 0

@pytest.mark.parametrize("invalid_name", ["", " ", " ", "-", "--", "_", "__", "A", "AA", "invalid_nAmE"])
async def test_create_dataset_question_with_invalid_name(
self, async_client: "AsyncClient", db: "AsyncSession", owner_auth_header: dict, invalid_name: str
):
dataset = await DatasetFactory.create()
question_json = {
"name": invalid_name,
"title": "title",
"settings": {"type": "text"},
}

response = await async_client.post(
f"/api/v1/datasets/{dataset.id}/questions", headers=owner_auth_header, json=question_json
)

assert response.status_code == 422
assert (await db.execute(select(func.count(Question.id)))).scalar() == 0

async def test_create_dataset_question_with_invalid_max_length_name(
self, async_client: "AsyncClient", db: "AsyncSession", owner_auth_header: dict
):
Expand Down
Loading

0 comments on commit c25c88c

Please sign in to comment.