Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add latest current and reference job status to modelout #59

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions api/app/models/model_dto.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@
from pydantic import BaseModel, ConfigDict, model_validator
from pydantic.alias_generators import to_camel

from app.db.dao.current_dataset_dao import CurrentDataset
from app.db.dao.model_dao import Model
from app.db.dao.reference_dataset_dao import ReferenceDataset
from app.models.inferred_schema_dto import SupportedTypes
from app.models.job_status import JobStatus
from app.models.utils import is_none, is_number, is_number_or_string, is_optional_float


Expand Down Expand Up @@ -42,6 +45,7 @@ class OutputType(BaseModel, validate_assignment=True):
prediction: ColumnDefinition
prediction_proba: Optional[ColumnDefinition] = None
output: List[ColumnDefinition]

model_config = ConfigDict(populate_by_name=True, alias_generator=to_camel)

def to_dict(self):
Expand Down Expand Up @@ -175,6 +179,8 @@ class ModelOut(BaseModel):
updated_at: str
latest_reference_uuid: Optional[UUID]
latest_current_uuid: Optional[UUID]
latest_reference_job_status: JobStatus
latest_current_job_status: JobStatus

model_config = ConfigDict(
populate_by_name=True, alias_generator=to_camel, protected_namespaces=()
Expand All @@ -183,9 +189,27 @@ class ModelOut(BaseModel):
@staticmethod
def from_model(
model: Model,
latest_reference_uuid: Optional[UUID] = None,
latest_current_uuid: Optional[UUID] = None,
latest_reference_dataset: Optional[ReferenceDataset] = None,
latest_current_dataset: Optional[CurrentDataset] = None,
):
latest_reference_uuid = (
latest_reference_dataset.uuid if latest_reference_dataset else None
)
latest_current_uuid = (
latest_current_dataset.uuid if latest_current_dataset else None
)

latest_reference_job_status = (
latest_reference_dataset.status
if latest_reference_dataset
else JobStatus.MISSING_REFERENCE
)
latest_current_job_status = (
latest_current_dataset.status
if latest_current_dataset
else JobStatus.MISSING_CURRENT
)
Comment on lines +202 to +211
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question: why do we need different JobStatus for current and reference? Why not just MISSING for both?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you raise a fair point. We had opted to set it up dual status with the FE guys, but it could be planned to be unified in a later step if it's okay with them


return ModelOut(
uuid=model.uuid,
name=model.name,
Expand All @@ -203,4 +227,6 @@ def from_model(
updated_at=str(model.updated_at),
latest_reference_uuid=latest_reference_uuid,
latest_current_uuid=latest_current_uuid,
latest_reference_job_status=latest_reference_job_status,
latest_current_job_status=latest_current_job_status,
)
33 changes: 14 additions & 19 deletions api/app/services/model_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
from app.db.dao.current_dataset_dao import CurrentDatasetDAO
from app.db.dao.model_dao import ModelDAO
from app.db.dao.reference_dataset_dao import ReferenceDatasetDAO
from app.db.tables.current_dataset_table import CurrentDataset
from app.db.tables.model_table import Model
from app.db.tables.reference_dataset_table import ReferenceDataset
from app.models.exceptions import ModelInternalError, ModelNotFoundError
from app.models.model_dto import ModelIn, ModelOut
from app.models.model_order import OrderType
Expand Down Expand Up @@ -35,13 +37,13 @@ def create_model(self, model_in: ModelIn) -> ModelOut:

def get_model_by_uuid(self, model_uuid: UUID) -> Optional[ModelOut]:
model = self.check_and_get_model(model_uuid)
latest_reference_uuid, latest_current_uuid = self.get_latest_dataset_uuids(
latest_reference_dataset, latest_current_dataset = self.get_latest_datasets(
model_uuid
)
return ModelOut.from_model(
model=model,
latest_reference_uuid=latest_reference_uuid,
latest_current_uuid=latest_current_uuid,
latest_reference_dataset=latest_reference_dataset,
latest_current_dataset=latest_current_dataset,
)

def delete_model(self, model_uuid: UUID) -> Optional[ModelOut]:
Expand All @@ -55,13 +57,13 @@ def get_all_models(
models = self.model_dao.get_all()
model_out_list = []
for model in models:
latest_reference_uuid, latest_current_uuid = self.get_latest_dataset_uuids(
latest_reference_dataset, latest_current_dataset = self.get_latest_datasets(
model.uuid
)
model_out = ModelOut.from_model(
model=model,
latest_reference_uuid=latest_reference_uuid,
latest_current_uuid=latest_current_uuid,
latest_reference_dataset=latest_reference_dataset,
latest_current_dataset=latest_current_dataset,
)
model_out_list.append(model_out)
return model_out_list
Expand All @@ -78,13 +80,13 @@ def get_all_models_paginated(

_items = []
for model in models.items:
latest_reference_uuid, latest_current_uuid = self.get_latest_dataset_uuids(
latest_reference_dataset, latest_current_dataset = self.get_latest_datasets(
model.uuid
)
model_out = ModelOut.from_model(
model=model,
latest_reference_uuid=latest_reference_uuid,
latest_current_uuid=latest_current_uuid,
latest_reference_dataset=latest_reference_dataset,
latest_current_dataset=latest_current_dataset,
)
_items.append(model_out)

Expand All @@ -96,21 +98,14 @@ def check_and_get_model(self, model_uuid: UUID) -> Model:
raise ModelNotFoundError(f'Model {model_uuid} not found')
return model

def get_latest_dataset_uuids(
def get_latest_datasets(
self, model_uuid: UUID
) -> (Optional[UUID], Optional[UUID]):
) -> (Optional[ReferenceDataset], Optional[CurrentDataset]):
latest_reference_dataset = (
self.rd_dao.get_latest_reference_dataset_by_model_uuid(model_uuid)
)
latest_current_dataset = self.cd_dao.get_latest_current_dataset_by_model_uuid(
model_uuid
)

latest_reference_uuid = (
latest_reference_dataset.uuid if latest_reference_dataset else None
)
latest_current_uuid = (
latest_current_dataset.uuid if latest_current_dataset else None
)

return latest_reference_uuid, latest_current_uuid
return latest_reference_dataset, latest_current_dataset
4 changes: 2 additions & 2 deletions api/tests/services/model_service_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ def test_get_model_by_uuid_ok(self):

assert res == ModelOut.from_model(
model=model,
latest_reference_uuid=reference_dataset.uuid,
latest_current_uuid=current_dataset.uuid,
latest_reference_dataset=reference_dataset,
latest_current_dataset=current_dataset,
)

def test_get_model_by_uuid_not_found(self):
Expand Down
2 changes: 0 additions & 2 deletions sdk/radicalbit_platform_sdk/models/model_definition.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,5 @@ class ModelDefinition(BaseModelDefinition):
uuid: uuid_lib.UUID = Field(default_factory=lambda: uuid_lib.uuid4())
created_at: str = Field(alias='createdAt')
updated_at: str = Field(alias='updatedAt')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we have alias if we have the generator to_camel?

latest_reference_uuid: Optional[uuid_lib.UUID] = Field(alias='latestReferenceUuid')
latest_current_uuid: Optional[uuid_lib.UUID] = Field(alias='latestCurrentUuid')

model_config = ConfigDict(populate_by_name=True, alias_generator=to_camel)
16 changes: 0 additions & 16 deletions sdk/tests/apis/model_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ def test_delete_model(self):
timestamp=column_def,
created_at=str(time.time()),
updated_at=str(time.time()),
latest_reference_uuid=uuid.uuid4(),
latest_current_uuid=uuid.uuid4(),
),
)
responses.add(
Expand Down Expand Up @@ -82,8 +80,6 @@ def test_load_reference_dataset_without_object_name(self):
timestamp=ColumnDefinition(name='created_at', type='str'),
created_at=str(time.time()),
updated_at=str(time.time()),
latest_reference_uuid=uuid.uuid4(),
latest_current_uuid=uuid.uuid4(),
),
)
response = ReferenceFileUpload(
Expand Down Expand Up @@ -129,8 +125,6 @@ def test_load_reference_dataset_with_different_separator(self):
timestamp=ColumnDefinition(name='created_at', type='str'),
created_at=str(time.time()),
updated_at=str(time.time()),
latest_reference_uuid=uuid.uuid4(),
latest_current_uuid=uuid.uuid4(),
),
)
response = ReferenceFileUpload(
Expand Down Expand Up @@ -176,8 +170,6 @@ def test_load_reference_dataset_with_object_name(self):
timestamp=ColumnDefinition(name='created_at', type='str'),
created_at=str(time.time()),
updated_at=str(time.time()),
latest_reference_uuid=uuid.uuid4(),
latest_current_uuid=uuid.uuid4(),
),
)
response = ReferenceFileUpload(
Expand Down Expand Up @@ -214,8 +206,6 @@ def test_load_reference_dataset_wrong_headers(self):
timestamp=ColumnDefinition(name='created_at', type='str'),
created_at=str(time.time()),
updated_at=str(time.time()),
latest_reference_uuid=None,
latest_current_uuid=None,
),
)
with pytest.raises(ClientError):
Expand Down Expand Up @@ -249,8 +239,6 @@ def test_load_current_dataset_without_object_name(self):
timestamp=ColumnDefinition(name='created_at', type='str'),
created_at=str(time.time()),
updated_at=str(time.time()),
latest_reference_uuid=uuid.uuid4(),
latest_current_uuid=uuid.uuid4(),
),
)
response = CurrentFileUpload(
Expand Down Expand Up @@ -302,8 +290,6 @@ def test_load_current_dataset_with_object_name(self):
timestamp=ColumnDefinition(name='created_at', type='str'),
created_at=str(time.time()),
updated_at=str(time.time()),
latest_reference_uuid=uuid.uuid4(),
latest_current_uuid=uuid.uuid4(),
),
)
response = CurrentFileUpload(
Expand Down Expand Up @@ -346,8 +332,6 @@ def test_load_current_dataset_wrong_headers(self):
timestamp=ColumnDefinition(name='created_at', type='str'),
created_at=str(time.time()),
updated_at=str(time.time()),
latest_reference_uuid=None,
latest_current_uuid=None,
),
)
with pytest.raises(ClientError):
Expand Down
10 changes: 1 addition & 9 deletions sdk/tests/client_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ def test_get_model(self):
timestamp_name = 'when'
timestamp_type = 'str'
ts = str(time.time())
latest_reference_uuid = uuid.uuid4()
latest_current_uuid = uuid.uuid4()
json_string = f"""{{
"uuid": "{str(model_id)}",
"name": "{name}",
Expand Down Expand Up @@ -77,9 +75,7 @@ def test_get_model(self):
"algorithm": "{algorithm}",
"frameworks": "{frameworks}",
"createdAt": "{ts}",
"updatedAt": "{ts}",
"latestReferenceUuid": "{str(latest_reference_uuid)}",
"latestCurrentUuid": "{str(latest_current_uuid)}"
"updatedAt": "{ts}"
}}"""
responses.add(
method=responses.GET,
Expand Down Expand Up @@ -156,8 +152,6 @@ def test_create_model(self):
timestamp=model.timestamp,
created_at=str(time.time()),
updated_at=str(time.time()),
latest_reference_uuid=None,
latest_current_uuid=None,
)
responses.add(
method=responses.POST,
Expand Down Expand Up @@ -199,8 +193,6 @@ def test_search_models(self):
timestamp=ColumnDefinition(name='tst_column', type='string'),
created_at=str(time.time()),
updated_at=str(time.time()),
latest_reference_uuid=None,
latest_current_uuid=None,
)

responses.add(
Expand Down
8 changes: 1 addition & 7 deletions sdk/tests/models/model_definition_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ def test_model_definition_from_json(self):
timestamp_name = 'when'
timestamp_type = 'str'
ts = str(time.time())
latest_reference_uuid = uuid.uuid4()
latest_current_uuid = uuid.uuid4()
json_string = f"""{{
"uuid": "{str(id)}",
"name": "{name}",
Expand Down Expand Up @@ -68,9 +66,7 @@ def test_model_definition_from_json(self):
"algorithm": "{algorithm}",
"frameworks": "{frameworks}",
"createdAt": "{ts}",
"updatedAt": "{ts}",
"latestReferenceUuid": "{str(latest_reference_uuid)}",
"latestCurrentUuid": "{str(latest_current_uuid)}"
"updatedAt": "{ts}"
}}"""
model_definition = ModelDefinition.model_validate(json.loads(json_string))
assert model_definition.uuid == id
Expand All @@ -83,8 +79,6 @@ def test_model_definition_from_json(self):
assert model_definition.frameworks == frameworks
assert model_definition.created_at == ts
assert model_definition.updated_at == ts
assert model_definition.latest_reference_uuid == latest_reference_uuid
assert model_definition.latest_current_uuid == latest_current_uuid
assert len(model_definition.features) == 1
assert model_definition.features[0].name == feature_name
assert model_definition.features[0].type == feature_type
Expand Down