Skip to content

Commit

Permalink
Update Task model validation to prevent status being NULL
Browse files Browse the repository at this point in the history
Change the eq method of Task so that it doesn't raise ValidationError
when the "other" object is a dict which isn't a valid task. It should
simply return that they are not equal.

Simplified the eq method a bit by removing redundant "else"s.

Some expected test results needed changing to accommodate the new Task
status.
  • Loading branch information
kjsanger committed Jul 11, 2024
1 parent ea576dd commit f524120
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 13 deletions.
19 changes: 11 additions & 8 deletions src/npg_porch/models/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from enum import Enum
import hashlib
import ujson
from pydantic import BaseModel, Field
from pydantic import BaseModel, Field, ValidationError

from npg_porch.models.pipeline import Pipeline

Expand All @@ -45,7 +45,7 @@ class Task(BaseModel):
title='Task Input',
description='A structured parameter set that uniquely identifies a piece of work, and enables an iteration of a pipeline' # noqa: E501
)
status: TaskStateEnum | None = None
status: TaskStateEnum = TaskStateEnum.PENDING

def generate_task_id(self):
return hashlib.sha256(ujson.dumps(self.task_input, sort_keys=True).encode()).hexdigest()
Expand All @@ -56,18 +56,21 @@ def __eq__(self, other):
The pipeline and task_input_ids can partially differ and it still be a
valid comparison. Clients do not get to create task_input_ids and may
not fully specify a pipeline. Status is also optional
not fully specify a pipeline.
Automatically attempts to cast a dict into a Task, and therefore
ignores any properties not valid for a Task
'''
if not isinstance(other, Task):
if isinstance(other, dict):
if isinstance(other, dict):
try:
other = Task.model_validate(other)
else:
except ValidationError:
return False

if not isinstance(other, Task):
return False

truths = []
for k, v in self.model_dump().items():
other_d = other.model_dump()
Expand All @@ -81,5 +84,5 @@ def __eq__(self, other):
truths.append(v == other_d[k])
if all(truths):
return True
else:
return False

return False
6 changes: 4 additions & 2 deletions tests/fixtures/deploy_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ def minimum_data():
definition={
'to_do': 'stuff',
'why': 'reasons'
}
},
state=TaskStateEnum.PENDING
),
Task(
pipeline=pipeline,
Expand All @@ -56,7 +57,8 @@ def minimum_data():
definition={
'to_do': 'more stuff',
'why': 'reasons'
}
},
state=TaskStateEnum.PENDING
)
]

Expand Down
15 changes: 12 additions & 3 deletions tests/task_route_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ def test_task_creation(async_minimum, fastapi_testclient):
def test_task_update(async_minimum, fastapi_testclient):

task = fastapi_testclient.get('/tasks', headers=headers4ptest_one).json()[0]
assert task['status'] is None
assert task['status'] == TaskStateEnum.PENDING.value

task['status'] = TaskStateEnum.PENDING
task['status'] = TaskStateEnum.RUNNING
response = fastapi_testclient.put(
'/tasks',
json=task,
Expand Down Expand Up @@ -206,7 +206,16 @@ def test_get_tasks(async_minimum, async_tasks, fastapi_testclient):
)
assert response.status_code == status.HTTP_200_OK, 'Other optional argument works'
tasks = response.json()
assert len(tasks) == 10, 'Ten pending tasks selected'
# async_minimum provides 2 tasks, async_tasks provides 10
assert len(tasks) == 12, 'Twelve pending tasks selected'

response = fastapi_testclient.get(
'/tasks?status=RUNNING',
headers=headers4ptest_one
)
assert response.status_code == status.HTTP_200_OK, 'Other optional argument works'
tasks = response.json()
assert len(tasks) == 0, 'No running tasks selected'

response = fastapi_testclient.get(
'/tasks?pipeline_name="ptest one"&status=PENDING',
Expand Down

0 comments on commit f524120

Please sign in to comment.