diff --git a/AGENTS.md b/AGENTS.md index 3c776cb5..6f1ae5fe 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -82,7 +82,7 @@ The job status is derived from its tasks via `post_process()`. The evaluation pr 1. Any PENDING task → job becomes PENDING (unless job is CANCELING) 2. Any IN_PROGRESS task → job becomes IN_PROGRESS (unless job is CANCELING) 3. If job was `CANCELING` → job becomes `CANCELED` -4. Otherwise the job is finished and its final status is computed from the combination of `SUCCESS`, `WARNING`, and `FAILURE` tasks +4. Otherwise the job is finished and its final status is computed from the combination of `SUCCESS`, `WARNING`, and `FAILURE` tasks. If none of those are present but canceled tasks are, the job becomes `CANCELED` ### Job Actions diff --git a/adit/core/models.py b/adit/core/models.py index 8d587cfa..4b5a1106 100644 --- a/adit/core/models.py +++ b/adit/core/models.py @@ -291,10 +291,12 @@ def post_process(self, suppress_email=False) -> bool: has_success = self.tasks.filter(status=DicomTask.Status.SUCCESS).exists() has_warning = self.tasks.filter(status=DicomTask.Status.WARNING).exists() has_failure = self.tasks.filter(status=DicomTask.Status.FAILURE).exists() + has_canceled = self.tasks.filter(status=DicomTask.Status.CANCELED).exists() + # An "All tasks ..." message would be untrue when some tasks were canceled instead. if has_success and not has_warning and not has_failure: self.status = DicomJob.Status.SUCCESS - self.message = "All tasks succeeded." + self.message = "Some tasks were canceled." if has_canceled else "All tasks succeeded." elif has_success and has_failure or has_warning and has_failure: self.status = DicomJob.Status.FAILURE self.message = "Some tasks failed." @@ -303,12 +305,17 @@ def post_process(self, suppress_email=False) -> bool: self.message = "Some tasks have warnings." elif has_warning: self.status = DicomJob.Status.WARNING - self.message = "All tasks have warnings." + self.message = ( + "Some tasks have warnings." if has_canceled else "All tasks have warnings." + ) elif has_failure: self.status = DicomJob.Status.FAILURE - self.message = "All tasks failed." + self.message = "Some tasks failed." if has_canceled else "All tasks failed." + elif has_canceled: + self.status = DicomJob.Status.CANCELED + self.message = "All tasks were canceled." else: - # at least one of success, warnings or failures must be > 0 + # at least one of success, warnings, failures or cancellations must be > 0 raise AssertionError(f"Invalid task status list of {self}.") self.end = timezone.now() diff --git a/adit/core/tests/test_models.py b/adit/core/tests/test_models.py index 47a3f809..0ead0f54 100644 --- a/adit/core/tests/test_models.py +++ b/adit/core/tests/test_models.py @@ -168,6 +168,75 @@ def test_job_post_process_canceling_status(self): assert job.status == DicomJob.Status.CANCELED assert job.end is None + @pytest.mark.django_db + def test_job_post_process_all_tasks_canceled(self): + # Status is PENDING, not CANCELING, so this reaches the final evaluation, where + # an all-canceled job raised AssertionError (e.g. all tasks killed from the admin). + job = ExampleTransferJobFactory.create(status=DicomJob.Status.PENDING) + + ExampleTransferTaskFactory.create(job=job, status=DicomTask.Status.CANCELED) + ExampleTransferTaskFactory.create(job=job, status=DicomTask.Status.CANCELED) + + result = job.post_process() + job.refresh_from_db() + + assert result is True + assert job.status == DicomJob.Status.CANCELED + assert job.message == "All tasks were canceled." + assert job.end is not None + + @pytest.mark.django_db + def test_job_post_process_canceled_task_does_not_mask_failure(self): + # The canceled branch is last in the chain, so it must not shadow a failure. + job = ExampleTransferJobFactory.create(status=DicomJob.Status.PENDING) + + ExampleTransferTaskFactory.create(job=job, status=DicomTask.Status.CANCELED) + ExampleTransferTaskFactory.create(job=job, status=DicomTask.Status.FAILURE) + + job.post_process() + job.refresh_from_db() + + assert job.status == DicomJob.Status.FAILURE + + @pytest.mark.django_db + def test_job_post_process_success_and_canceled_does_not_claim_all_succeeded(self): + job = ExampleTransferJobFactory.create(status=DicomJob.Status.PENDING) + + ExampleTransferTaskFactory.create(job=job, status=DicomTask.Status.SUCCESS) + ExampleTransferTaskFactory.create(job=job, status=DicomTask.Status.CANCELED) + + job.post_process() + job.refresh_from_db() + + assert job.status == DicomJob.Status.SUCCESS + assert job.message == "Some tasks were canceled." + + @pytest.mark.django_db + def test_job_post_process_failure_and_canceled_does_not_claim_all_failed(self): + job = ExampleTransferJobFactory.create(status=DicomJob.Status.PENDING) + + ExampleTransferTaskFactory.create(job=job, status=DicomTask.Status.FAILURE) + ExampleTransferTaskFactory.create(job=job, status=DicomTask.Status.CANCELED) + + job.post_process() + job.refresh_from_db() + + assert job.status == DicomJob.Status.FAILURE + assert job.message == "Some tasks failed." + + @pytest.mark.django_db + def test_job_post_process_warning_and_canceled_does_not_claim_all_warned(self): + job = ExampleTransferJobFactory.create(status=DicomJob.Status.PENDING) + + ExampleTransferTaskFactory.create(job=job, status=DicomTask.Status.WARNING) + ExampleTransferTaskFactory.create(job=job, status=DicomTask.Status.CANCELED) + + job.post_process() + job.refresh_from_db() + + assert job.status == DicomJob.Status.WARNING + assert job.message == "Some tasks have warnings." + @pytest.mark.django_db @time_machine.travel("2025-01-15 14:30:00+00:00") def test_job_timezone_correctness(self):