Skip to content

Commit

Permalink
refactor: review the argilla. _constants module usage (#4474)
Browse files Browse the repository at this point in the history
<!-- Thanks for your contribution! As part of our Community Growers
initiative 🌱, we're donating Justdiggit bunds in your name to reforest
sub-Saharan Africa. To claim your Community Growers certificate, please
contact David Berenstein in our Slack community or fill in this form
https://tally.so/r/n9XrxK once your PR has been merged. -->

# Description

In this PR the module `argilla._constants` has been reviewed to separate
server-side and client-side constant definitions. The server module
contains its constant definitions, even if they're duplicated. This will
help to separate client and server modules into different packages.


Closes #4469 

**Type of change**

(Please delete options that are not relevant. Remember to title the PR
according to the type of change)

- [ ] New feature (non-breaking change which adds functionality)
- [X] Refactor (change restructuring the codebase without changing
functionality)
- [X] Improvement (change adding some improvement to an existing
functionality)

**How Has This Been Tested**

(Please describe the tests that you ran to verify your changes. And
ideally, reference `tests`)

Changes have been tested locally. 

**Checklist**

- [ ] I added relevant documentation
- [X] I followed the style guidelines of this project
- [X] I did a self-review of my code
- [ ] I made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [X] I have added tests that prove my fix is effective or that my
feature works
- [ ] I filled out [the contributor form](https://tally.so/r/n9XrxK)
(see text above)
- [ ] I have added relevant notes to the `CHANGELOG.md` file (See
https://keepachangelog.com/)

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
frascuchon and pre-commit-ci[bot] authored Jan 10, 2024
1 parent a892dda commit 0b91c8f
Show file tree
Hide file tree
Showing 48 changed files with 108 additions and 67 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ These are the section headers that we use:
## Changed

- Module `argilla.cli.server` definitions have been moved to `argilla.server.cli` module. ([#4472](https://github.com/argilla-io/argilla/pull/4472))
- The constant definition `ES_INDEX_REGEX_PATTERN` in module `argilla._constants` is now private. ([#4472](https://github.com/argilla-io/argilla/pull/4474))

### Deprecated

Expand Down
11 changes: 4 additions & 7 deletions src/argilla/_constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.

_ES_INDEX_REGEX_PATTERN = r"^(?!-|_)[a-z0-9-_]+$"

API_KEY_HEADER_NAME = "X-Argilla-Api-Key"
WORKSPACE_HEADER_NAME = "X-Argilla-Workspace"

Expand All @@ -21,17 +23,12 @@
DEFAULT_API_URL = "http://localhost:6900"
DEFAULT_API_KEY = "argilla.apikey"
DEFAULT_MAX_KEYWORD_LENGTH = 128
DEFAULT_TELEMETRY_KEY = "C6FkcaoCbt78rACAgvyBxGBcMB3dM3nn"

# The metadata field name prefix defined for protected (non-searchable) values
PROTECTED_METADATA_FIELD_PREFIX = "_"

ES_INDEX_REGEX_PATTERN = r"^(?!-|_)[a-z0-9-_]+$"
DATASET_NAME_REGEX_PATTERN = _ES_INDEX_REGEX_PATTERN
WORKSPACE_NAME_REGEX_PATTERN = _ES_INDEX_REGEX_PATTERN

# constants for prepare_for_training(framework="openai")
OPENAI_SEPARATOR = "\n\n###\n\n"
OPENAI_END_TOKEN = " END"
OPENAI_WHITESPACE = " "
OPENAI_LEGACY_MODELS = ["babbage", "davinci", "curie", "ada"]

_JS_MAX_SAFE_INTEGER = 9007199254740991
28 changes: 19 additions & 9 deletions src/argilla/client/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,12 @@
from rich.progress import Progress

from argilla._constants import (
DATASET_NAME_REGEX_PATTERN,
DEFAULT_API_KEY,
DEFAULT_API_URL,
DEFAULT_USERNAME,
ES_INDEX_REGEX_PATTERN,
WORKSPACE_HEADER_NAME,
WORKSPACE_NAME_REGEX_PATTERN,
)
from argilla.client.apis.datasets import Datasets
from argilla.client.apis.metrics import MetricsAPI
Expand Down Expand Up @@ -58,21 +59,30 @@
from argilla.client.sdk.datasets.models import Dataset as DatasetModel
from argilla.client.sdk.metrics import api as metrics_api
from argilla.client.sdk.metrics.models import MetricInfo
from argilla.client.sdk.text2text.models import CreationText2TextRecord, Text2TextBulkData
from argilla.client.sdk.text2text.models import Text2TextRecord as SdkText2TextRecord
from argilla.client.sdk.text2text.models import (
CreationText2TextRecord,
Text2TextBulkData,
)
from argilla.client.sdk.text2text.models import (
Text2TextRecord as SdkText2TextRecord,
)
from argilla.client.sdk.text_classification import api as text_classification_api
from argilla.client.sdk.text_classification.models import (
CreationTextClassificationRecord,
LabelingRule,
LabelingRuleMetricsSummary,
TextClassificationBulkData,
)
from argilla.client.sdk.text_classification.models import TextClassificationRecord as SdkTextClassificationRecord
from argilla.client.sdk.text_classification.models import (
TextClassificationRecord as SdkTextClassificationRecord,
)
from argilla.client.sdk.token_classification.models import (
CreationTokenClassificationRecord,
TokenClassificationBulkData,
)
from argilla.client.sdk.token_classification.models import TokenClassificationRecord as SdkTokenClassificationRecord
from argilla.client.sdk.token_classification.models import (
TokenClassificationRecord as SdkTokenClassificationRecord,
)
from argilla.client.sdk.users import api as users_api
from argilla.client.sdk.v1.workspaces import api as workspaces_api_v1
from argilla.client.sdk.v1.workspaces.models import WorkspaceModel
Expand Down Expand Up @@ -238,10 +248,10 @@ def set_workspace(self, workspace: str):
if not workspace:
raise Exception("Must provide a workspace")

if not re.match(ES_INDEX_REGEX_PATTERN, workspace):
if not re.match(WORKSPACE_NAME_REGEX_PATTERN, workspace):
raise InputValueError(
f"Provided workspace name {workspace} does not match the pattern"
f" {ES_INDEX_REGEX_PATTERN}. Please, use a valid name for your"
f" {WORKSPACE_NAME_REGEX_PATTERN}. Please, use a valid name for your"
" workspace. This limitation is caused by naming conventions for indexes"
" in Elasticsearch. If applicable, you can try to lowercase the name of your workspace."
" https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-create-index.html"
Expand Down Expand Up @@ -392,10 +402,10 @@ def log(
if not name:
raise InputValueError("Empty dataset name has been passed as argument.")

if not re.match(ES_INDEX_REGEX_PATTERN, name):
if not re.match(DATASET_NAME_REGEX_PATTERN, name):
raise InputValueError(
f"Provided dataset name {name} does not match the pattern"
f" {ES_INDEX_REGEX_PATTERN}. Please, use a valid name for your"
f" {DATASET_NAME_REGEX_PATTERN}. Please, use a valid name for your"
" dataset. This limitation is caused by naming conventions for indexes"
" in Elasticsearch."
" https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-create-index.html"
Expand Down
11 changes: 8 additions & 3 deletions src/argilla/client/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
from deprecated import deprecated

from argilla import _messages
from argilla._constants import _JS_MAX_SAFE_INTEGER, DEFAULT_MAX_KEYWORD_LENGTH, PROTECTED_METADATA_FIELD_PREFIX
from argilla._constants import DEFAULT_MAX_KEYWORD_LENGTH
from argilla.pydantic_v1 import BaseModel, Field, PrivateAttr, root_validator, validator
from argilla.utils.span_utils import SpanUtils

Expand Down Expand Up @@ -91,6 +91,11 @@ def __str__(self) -> str:
class _Validators(BaseModel):
"""Base class for our record models that takes care of general validations"""

# The metadata field name prefix defined for protected (non-searchable) values
_PROTECTED_METADATA_FIELD_PREFIX = "_"

_JS_MAX_SAFE_INTEGER = 9007199254740991

@validator("metadata", check_fields=False)
def _check_value_length(cls, metadata):
"""Checks metadata values length and warn message for large values"""
Expand All @@ -99,7 +104,7 @@ def _check_value_length(cls, metadata):

default_length_exceeded = False
for k, v in metadata.items():
if k.startswith(PROTECTED_METADATA_FIELD_PREFIX):
if k.startswith(cls._PROTECTED_METADATA_FIELD_PREFIX):
continue
if isinstance(v, str) and len(v) > DEFAULT_MAX_KEYWORD_LENGTH:
default_length_exceeded = True
Expand Down Expand Up @@ -134,7 +139,7 @@ def _normalize_id(cls, v):
)
warnings.warn(message, DeprecationWarning, stacklevel=2)
# See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER
if v > _JS_MAX_SAFE_INTEGER:
if v > cls._JS_MAX_SAFE_INTEGER:
message = (
"You've provided a big integer value. Use a string instead, otherwise you may experience some "
"problems using the UI. See "
Expand Down
2 changes: 1 addition & 1 deletion src/argilla/server/apis/v0/models/commons/params.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

from fastapi import Header, Path, Query

from argilla._constants import ES_INDEX_REGEX_PATTERN, WORKSPACE_HEADER_NAME
from argilla.server.constants import ES_INDEX_REGEX_PATTERN, WORKSPACE_HEADER_NAME
from argilla.server.errors import BadRequestError, MissingInputParamError

DATASET_NAME_PATH_PARAM = Path(..., regex=ES_INDEX_REGEX_PATTERN, description="The dataset name")
Expand Down
2 changes: 1 addition & 1 deletion src/argilla/server/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@
from starlette.responses import RedirectResponse

from argilla import __version__ as argilla_version
from argilla._constants import DEFAULT_API_KEY, DEFAULT_PASSWORD, DEFAULT_USERNAME
from argilla.logging import configure_logging
from argilla.server import helpers
from argilla.server.constants import DEFAULT_API_KEY, DEFAULT_PASSWORD, DEFAULT_USERNAME
from argilla.server.contexts import accounts
from argilla.server.daos.backend import GenericElasticEngineBackend
from argilla.server.daos.backend.base import GenericSearchError
Expand Down
30 changes: 30 additions & 0 deletions src/argilla/server/constants.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# 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.

API_KEY_HEADER_NAME = "X-Argilla-Api-Key"
WORKSPACE_HEADER_NAME = "X-Argilla-Workspace"

DEFAULT_USERNAME = "argilla"
DEFAULT_PASSWORD = "1234"
DEFAULT_API_KEY = "argilla.apikey"

DEFAULT_MAX_KEYWORD_LENGTH = 128
DEFAULT_TELEMETRY_KEY = "C6FkcaoCbt78rACAgvyBxGBcMB3dM3nn"

# The metadata field name prefix defined for protected (non-searchable) values
PROTECTED_METADATA_FIELD_PREFIX = "_"

ES_INDEX_REGEX_PATTERN = r"^(?!-|_)[a-z0-9-_]+$"

JS_MAX_SAFE_INTEGER = 9007199254740991
2 changes: 1 addition & 1 deletion src/argilla/server/daos/backend/generic_elastic.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@

from typing import Any, Dict, Iterable, List, Optional, Tuple

from argilla._constants import PROTECTED_METADATA_FIELD_PREFIX
from argilla.logging import LoggingMixin
from argilla.server.commons.models import TaskType
from argilla.server.constants import PROTECTED_METADATA_FIELD_PREFIX
from argilla.server.daos.backend.base import IndexNotFoundError, InvalidSearchError
from argilla.server.daos.backend.client_adapters.base import IClientAdapter
from argilla.server.daos.backend.client_adapters.factory import ClientAdapterFactory
Expand Down
2 changes: 1 addition & 1 deletion src/argilla/server/daos/models/datasets.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
from datetime import datetime
from typing import Any, Dict, Optional, TypeVar, Union

from argilla._constants import ES_INDEX_REGEX_PATTERN
from argilla.server.commons.models import TaskType
from argilla.server.constants import ES_INDEX_REGEX_PATTERN
from argilla.server.pydantic_v1 import BaseModel, Field, root_validator


Expand Down
4 changes: 2 additions & 2 deletions src/argilla/server/daos/models/records.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
from typing import Any, Dict, Generic, List, Optional, TypeVar, Union

from argilla import _messages
from argilla._constants import _JS_MAX_SAFE_INTEGER, PROTECTED_METADATA_FIELD_PREFIX
from argilla.server.commons.models import PredictionStatus, TaskStatus, TaskType
from argilla.server.constants import JS_MAX_SAFE_INTEGER, PROTECTED_METADATA_FIELD_PREFIX
from argilla.server.daos.backend.search.model import BaseRecordsQuery, SortConfig
from argilla.server.helpers import flatten_dict
from argilla.server.pydantic_v1 import BaseModel, Field, conint, constr, root_validator, validator
Expand Down Expand Up @@ -127,7 +127,7 @@ def _normalize_id(cls, v):
)
warnings.warn(message, DeprecationWarning)
# See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER
if v > _JS_MAX_SAFE_INTEGER:
if v > JS_MAX_SAFE_INTEGER:
message = (
"You've provided a big integer value. Use a string instead, otherwise you may experience some "
"problems using the UI. See "
Expand Down
2 changes: 1 addition & 1 deletion src/argilla/server/schemas/v0/datasets.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
from typing import Any, Dict, Optional, Union
from uuid import UUID

from argilla._constants import ES_INDEX_REGEX_PATTERN
from argilla.server.commons.models import TaskType
from argilla.server.constants import ES_INDEX_REGEX_PATTERN
from argilla.server.pydantic_v1 import BaseModel, Field


Expand Down
2 changes: 1 addition & 1 deletion src/argilla/server/security/auth_provider/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from fastapi import Depends, FastAPI, Request
from fastapi.security import APIKeyHeader, SecurityScopes

from argilla._constants import API_KEY_HEADER_NAME
from argilla.server.constants import API_KEY_HEADER_NAME
from argilla.server.models import User

api_key_header = APIKeyHeader(name=API_KEY_HEADER_NAME, auto_error=False)
Expand Down
2 changes: 1 addition & 1 deletion src/argilla/server/security/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from typing import Any, List, Optional
from uuid import UUID

from argilla._constants import ES_INDEX_REGEX_PATTERN
from argilla.server.constants import ES_INDEX_REGEX_PATTERN
from argilla.server.models import UserRole
from argilla.server.pydantic_v1 import BaseModel, Field, constr
from argilla.server.pydantic_v1.utils import GetterDict
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
from datetime import datetime
from typing import Any, ClassVar, Dict, List, Optional, Union

from argilla._constants import DEFAULT_MAX_KEYWORD_LENGTH
from argilla.server.commons.models import PredictionStatus, TaskStatus, TaskType
from argilla.server.constants import DEFAULT_MAX_KEYWORD_LENGTH
from argilla.server.helpers import flatten_dict
from argilla.server.pydantic_v1 import BaseModel, Field, root_validator, validator
from argilla.server.services.datasets import ServiceBaseDataset
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@

from typing import Dict, List, Optional, Set, Tuple

from argilla._constants import DEFAULT_MAX_KEYWORD_LENGTH
from argilla.server.commons.models import PredictionStatus, TaskType
from argilla.server.constants import DEFAULT_MAX_KEYWORD_LENGTH
from argilla.server.pydantic_v1 import BaseModel, Field, validator
from argilla.server.services.search.model import ServiceBaseRecordsQuery, ServiceScoreRange
from argilla.server.services.tasks.commons import ServiceBaseAnnotation, ServiceBaseRecord
Expand Down
2 changes: 1 addition & 1 deletion src/argilla/server/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
from typing import List, Optional
from urllib.parse import urlparse

from argilla._constants import DEFAULT_MAX_KEYWORD_LENGTH, DEFAULT_TELEMETRY_KEY
from argilla.server.constants import DEFAULT_MAX_KEYWORD_LENGTH, DEFAULT_TELEMETRY_KEY
from argilla.server.pydantic_v1 import BaseSettings, Field, root_validator, validator


Expand Down
6 changes: 4 additions & 2 deletions src/argilla/utils/telemetry.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import uuid
from typing import TYPE_CHECKING, Any, Dict, Optional

from argilla._constants import DEFAULT_TELEMETRY_KEY
from argilla.pydantic_v1 import BaseSettings

if TYPE_CHECKING:
Expand All @@ -27,6 +26,9 @@
from argilla.server.commons.models import TaskType


_DEFAULT_TELEMETRY_KEY = "C6FkcaoCbt78rACAgvyBxGBcMB3dM3nn"


class TelemetrySettings(BaseSettings):
"""
Telemetry settings
Expand All @@ -35,7 +37,7 @@ class TelemetrySettings(BaseSettings):
"""

enable_telemetry: bool = True
telemetry_key: str = DEFAULT_TELEMETRY_KEY
telemetry_key: str = _DEFAULT_TELEMETRY_KEY

class Config:
env_prefix = "ARGILLA_"
Expand Down
2 changes: 0 additions & 2 deletions tests/integration/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

from argilla._constants import (
API_KEY_HEADER_NAME,
DEFAULT_API_KEY,
DEFAULT_USERNAME,
WORKSPACE_HEADER_NAME,
)
from argilla.server.models import User
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/server/api/v0/test_datasets.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
from typing import Any, Dict, List

import pytest
from argilla._constants import API_KEY_HEADER_NAME
from argilla.server.apis.v0.models.text_classification import TextClassificationBulkRequest
from argilla.server.commons.models import TaskType
from argilla.server.constants import API_KEY_HEADER_NAME
from argilla.server.models import UserRole
from argilla.server.schemas.v0.datasets import Dataset

Expand Down
2 changes: 1 addition & 1 deletion tests/unit/server/api/v0/test_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
from typing import TYPE_CHECKING

import pytest
from argilla._constants import API_KEY_HEADER_NAME
from argilla.server.apis.v0.models.text2text import (
Text2TextBulkRequest,
Text2TextRecord,
Expand All @@ -29,6 +28,7 @@
TokenClassificationRecord,
)
from argilla.server.commons.models import TaskType
from argilla.server.constants import API_KEY_HEADER_NAME
from argilla.server.models import User, UserRole
from argilla.server.services.metrics.models import CommonTasksMetrics
from argilla.server.services.tasks.text_classification.metrics import TextClassificationMetrics
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/server/api/v0/test_records_update.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@
from typing import TYPE_CHECKING

import pytest
from argilla._constants import API_KEY_HEADER_NAME
from argilla.server.apis.v0.models.text2text import Text2TextBulkRequest, Text2TextRecord, Text2TextRecordInputs
from argilla.server.apis.v0.models.text_classification import TextClassificationBulkRequest, TextClassificationRecord
from argilla.server.apis.v0.models.token_classification import TokenClassificationBulkRequest, TokenClassificationRecord
from argilla.server.commons.models import TaskType
from argilla.server.constants import API_KEY_HEADER_NAME
from argilla.server.models import User, UserRole

from tests.factories import UserFactory, WorkspaceFactory
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/server/api/v0/test_text2text.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@
from typing import TYPE_CHECKING, List, Optional

import pytest
from argilla._constants import API_KEY_HEADER_NAME
from argilla.server.apis.v0.models.commons.model import BulkResponse
from argilla.server.apis.v0.models.text2text import (
Text2TextBulkRequest,
Text2TextRecordInputs,
Text2TextSearchResults,
)
from argilla.server.commons.models import TaskType
from argilla.server.constants import API_KEY_HEADER_NAME
from argilla.server.models import User

if TYPE_CHECKING:
Expand Down
Loading

0 comments on commit 0b91c8f

Please sign in to comment.