Merge pull request #72 from kjsanger/model-avoid-null-status

Update Task model validation to prevent status being NULL

src/npg_porch/models/task.py

@@ -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
 
@@ -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()
@@ -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()
@@ -81,5 +84,5 @@ def __eq__(self, other):
                 truths.append(v == other_d[k])
         if all(truths):
             return True
-        else:
-            return False
+
+        return False

tests/data_access_test.py

@@ -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(
@@ -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
             )
         )
 
@@ -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
             )
         )
 
@@ -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
         )
     )
 
@@ -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
@@ -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
             )
         )
 

tests/fixtures/deploy_db.py

@@ -46,7 +46,8 @@ def minimum_data():
             definition={
                 'to_do': 'stuff',
                 'why': 'reasons'
-            }
+            },
+            state=TaskStateEnum.PENDING
         ),
         Task(
             pipeline=pipeline,
@@ -56,7 +57,8 @@ def minimum_data():
             definition={
                 'to_do': 'more stuff',
                 'why': 'reasons'
-            }
+            },
+            state=TaskStateEnum.PENDING
         )
     ]
 

tests/task_route_test.py

@@ -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(
@@ -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.
@@ -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,
@@ -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',