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

Update Task model validation to prevent status being NULL #72

Merged
merged 2 commits into from
Jul 12, 2024
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
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

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
24 changes: 16 additions & 8 deletions tests/data_access_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ async def test_create_task(db_accessor):

task = Task(
pipeline=saved_pipeline,
task_input={'test': True}
task_input={'test': True},
status=TaskStateEnum.PENDING
)

(saved_task, created) = await db_accessor.create_task(
Expand Down Expand Up @@ -144,7 +145,8 @@ async def test_claim_tasks(db_accessor):
token_id=1,
task=Task(
task_input={'number': i + 1},
pipeline=pipeline
pipeline=pipeline,
status=TaskStateEnum.PENDING
)
)

Expand Down Expand Up @@ -179,14 +181,16 @@ async def test_multi_claim_tasks(db_accessor):
token_id=1,
task=Task(
task_input={'number': i + 1},
pipeline=pipeline
pipeline=pipeline,
status=TaskStateEnum.PENDING
)
)
await db_accessor.create_task(
token_id=2,
task=Task(
task_input={'number': i + 1},
pipeline=other_pipeline
pipeline=other_pipeline,
status=TaskStateEnum.PENDING
)
)

Expand All @@ -208,7 +212,8 @@ async def test_update_tasks(db_accessor):
token_id=1,
task=Task(
task_input={'number': 1},
pipeline=saved_pipeline
pipeline=saved_pipeline,
status=TaskStateEnum.PENDING
)
)

Expand All @@ -219,11 +224,13 @@ async def test_update_tasks(db_accessor):

events = await db_accessor.get_events_for_task(modified_task)
assert len(events) == 2, 'Task was created, and then updated'
events[1].change == 'Task changed, new status DONE'
assert events[1].change == 'Task changed, new status DONE'

# Try to change a task that doesn't exist
with pytest.raises(NoResultFound):
await db_accessor.update_task(1, Task(task_input={'number': None}, pipeline=saved_pipeline))
await db_accessor.update_task(1, Task(task_input={'number': None},
pipeline=saved_pipeline,
status=TaskStateEnum.PENDING))

# Try modifying something we're not allowed to
saved_task.task_input_id = None
Expand Down Expand Up @@ -254,7 +261,8 @@ async def test_get_tasks(db_accessor):
token_id=1,
task=Task(
task_input={'number': i + 1},
pipeline=pipeline
pipeline=pipeline,
status=TaskStateEnum.PENDING
)
)

Expand Down
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
29 changes: 20 additions & 9 deletions tests/task_route_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@ def test_task_creation(async_minimum, fastapi_testclient):

# Create a task with a sparse pipeline definition
task_one = Task(
pipeline = {
pipeline={
'name': 'ptest one'
},
task_input = {
task_input={
'number': 1
}
},
status=TaskStateEnum.PENDING,
)

response = fastapi_testclient.post(
Expand All @@ -46,12 +47,13 @@ def test_task_creation(async_minimum, fastapi_testclient):
assert response.json() == response_obj

task_two = Task(
pipeline = {
pipeline={
'name': 'ptest none'
},
task_input = {
task_input={
'number': 1
}
},
status=TaskStateEnum.PENDING,
)
# The token is valid, but for a different pipeline. It is impossible
# to have a valid token for a pipeline that does not exist.
Expand All @@ -67,9 +69,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 +208,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