fix: Adjust task status behaviour

Author: Alan Christie

viewer/target_loader.py

@@ -4,7 +4,6 @@
 import math
 import os
 import tarfile
-import traceback
 import uuid
 from dataclasses import dataclass, field
 from enum import Enum
@@ -153,16 +152,20 @@ def log(self, level: Level, message: str) -> None:
 
     def final(self, message, upload_state=None):
         if upload_state:
+            # User has provided an over-ride for the upload state.
             self.upload_state = upload_state
+        elif self.upload_state == UploadState.PROCESSING:
+            self.upload_state = UploadState.SUCCESS
+            logger.info(message)
         else:
-            if self.upload_state == UploadState.PROCESSING:
-                self.upload_state = UploadState.SUCCESS
-                logger.info(message)
-            else:
-                self.upload_state = UploadState.FAILED
-                logger.error(message)
+            self.upload_state = UploadState.FAILED
+            logger.error(message)
 
+        # This is (expected to be) the last message for the upload.
+        # Add the user-supplied message and then add the string representation
+        # of the upload state.
         self.stack.append(UploadReportEntry(message=message))
+        self.stack.append(UploadReportEntry(message=self.upload_state.name))
         self._update_task(self.json())
 
     def json(self):
@@ -1175,10 +1178,6 @@ def process_bundle(self):
         # this is the last file to load. if any of the files missing, don't continue
         if not meta or not config or not xtalforms_yaml:
             msg = "Missing files in uploaded data, aborting"
-            self.report.final(
-                msg,
-                Level.FATAL,
-            )
             raise FileNotFoundError(msg)
 
         try:
@@ -1596,22 +1595,20 @@ def load_target(
                     raise IntegrityError(
                         f"Uploading {target_loader.data_bundle} failed"
                     )
-        except IntegrityError as exc:
-            logger.error(exc, exc_info=True)
-            target_loader.report.final(f"Uploading {target_loader.data_bundle} failed")
-            raise IntegrityError from exc
-
-        except (FileExistsError, FileNotFoundError, StopIteration) as exc:
-            raise Exception from exc
-
         except Exception as exc:
-            # catching and logging any other error
-            logger.error(exc, exc_info=True)
-            target_loader.report.log(Level.FATAL, traceback.format_exc())
-            raise Exception from exc
+            # Handle _any_ underlying problem.
+            # These are errors processing the data, which we handle gracefully.
+            # The task should _always_ end successfully.
+            # Any problem with the underlying data is transmitted in the report.
+            logger.debug(exc, exc_info=True)
+            target_loader.report.final(
+                f"Uploading {target_loader.data_bundle} failed",
+                upload_state=UploadState.SUCCESS,
+            )
+            return
 
         else:
-            # move to final location
+            # Move the uploaded file to its final location
             target_loader.abs_final_path.mkdir(parents=True)
             target_loader.raw_data.rename(target_loader.abs_final_path)
             Path(target_loader.bundle_path).rename(
@@ -1621,7 +1618,8 @@ def load_target(
             set_directory_permissions(target_loader.abs_final_path, 0o755)
 
             target_loader.report.final(
-                f"{target_loader.data_bundle} uploaded successfully"
+                f"{target_loader.data_bundle} uploaded successfully",
+                upload_state=UploadState.SUCCESS,
             )
             target_loader.experiment_upload.message = target_loader.report.json()
 

viewer/views.py

@@ -1578,14 +1578,12 @@ def create(self, request, *args, **kwargs):
 class TaskStatus(APIView):
     def get(self, request, task_id, *args, **kwargs):
         """Given a task_id (a string) we try to return the status of the task,
-        trying to handle unknown tasks as best we can. Celery is happy to accept any
-        string as a Task ID. To know it's a real task takes a lot of work, or you can
-        simply interpret a 'PENDING' state as "unknown task".
+        trying to handle unknown tasks as best we can.
         """
         # Unused arguments
         del request, args, kwargs
 
-        logger.info("task_id=%s", task_id)
+        logger.debug("task_id=%s", task_id)
 
         # task_id is a UUID, but Celery expects a string
         task_id_str = str(task_id)
@@ -1609,11 +1607,13 @@ def get(self, request, task_id, *args, **kwargs):
             elif isinstance(result.info, list):
                 messages = result.info
 
+        # The task is considered to have failed
+        # if the word 'FAILED' is in the last line of the message (regardless of case)
         task_status = "UNKNOWN"
-        if result.ready():
-            task_status = "SUCCESS" if result.successful() else "FAILED"
+        if result.ready() and messages:
+            task_status = "FAILED" if 'failed' in messages[-1].lower() else "SUCCESS"
+
         data = {
-            'id': result.id,
             'started': result.state != 'PENDING',
             'finished': result.ready(),
             'status': task_status,