Merge pull request galaxyproject#18551 from davelopez/task_monitor_co…

…nsider_task_expiration Add task expiration to persistent task monitor
bgruening · Jul 19, 2024 · bf5f1c7 · bf5f1c7
2 parents d7d85fd + 0eb7eee
commit bf5f1c7
Show file tree

Hide file tree

Showing 9 changed files with 313 additions and 8 deletions.
diff --git a/client/src/components/Common/PersistentTaskProgressMonitorAlert.test.ts b/client/src/components/Common/PersistentTaskProgressMonitorAlert.test.ts
@@ -20,13 +20,18 @@ const FAKE_MONITOR_REQUEST: MonitoringRequest = {
     description: "Test description",
 };
 
+const FAKE_EXPIRATION_TIME = 1000;
+
 const FAKE_MONITOR: TaskMonitor = {
     waitForTask: jest.fn(),
     isRunning: ref(false),
     isCompleted: ref(false),
     hasFailed: ref(false),
     requestHasFailed: ref(false),
-    status: ref(""),
+    status: ref(),
+    expirationTime: FAKE_EXPIRATION_TIME,
+    isFinalState: jest.fn(),
+    loadStatus: jest.fn(),
 };
 
 const mountComponent = (
@@ -61,6 +66,7 @@ describe("PersistentTaskProgressMonitorAlert.vue", () => {
             taskId: "1",
             taskType: "task",
             request: FAKE_MONITOR_REQUEST,
+            startedAt: new Date(),
         };
         usePersistentProgressTaskMonitor(FAKE_MONITOR_REQUEST, useMonitor, existingMonitoringData);
 
@@ -85,6 +91,7 @@ describe("PersistentTaskProgressMonitorAlert.vue", () => {
             taskId: "1",
             taskType: "task",
             request: FAKE_MONITOR_REQUEST,
+            startedAt: new Date(),
         };
         usePersistentProgressTaskMonitor(FAKE_MONITOR_REQUEST, useMonitor, existingMonitoringData);
 
@@ -109,6 +116,7 @@ describe("PersistentTaskProgressMonitorAlert.vue", () => {
             taskId: "1",
             taskType: "task",
             request: FAKE_MONITOR_REQUEST,
+            startedAt: new Date(),
         };
         usePersistentProgressTaskMonitor(FAKE_MONITOR_REQUEST, useMonitor, existingMonitoringData);
 
@@ -138,6 +146,7 @@ describe("PersistentTaskProgressMonitorAlert.vue", () => {
             taskId: taskId,
             taskType: "short_term_storage",
             request: monitoringRequest,
+            startedAt: new Date(),
         };
         usePersistentProgressTaskMonitor(monitoringRequest, useMonitor, existingMonitoringData);
 
@@ -166,6 +175,7 @@ describe("PersistentTaskProgressMonitorAlert.vue", () => {
             taskId: "1",
             taskType: "task",
             request: FAKE_MONITOR_REQUEST,
+            startedAt: new Date(),
         };
         usePersistentProgressTaskMonitor(FAKE_MONITOR_REQUEST, useMonitor, existingMonitoringData);
 
@@ -180,4 +190,29 @@ describe("PersistentTaskProgressMonitorAlert.vue", () => {
         expect(completedAlert.exists()).toBe(true);
         expect(completedAlert.text()).not.toContain("Download here");
     });
+
+    it("should render a warning alert when the task has expired even if the status is running", () => {
+        const useMonitor = {
+            ...FAKE_MONITOR,
+            isRunning: ref(true),
+        };
+        const existingMonitoringData: MonitoringData = {
+            taskId: "1",
+            taskType: "task",
+            request: FAKE_MONITOR_REQUEST,
+            startedAt: new Date(Date.now() - FAKE_EXPIRATION_TIME * 2), // Make sure the task has expired
+        };
+        usePersistentProgressTaskMonitor(FAKE_MONITOR_REQUEST, useMonitor, existingMonitoringData);
+
+        const wrapper = mountComponent({
+            monitorRequest: FAKE_MONITOR_REQUEST,
+            useMonitor,
+        });
+
+        expect(wrapper.find(".d-flex").exists()).toBe(true);
+
+        const warningAlert = wrapper.find('[variant="warning"]');
+        expect(warningAlert.exists()).toBe(true);
+        expect(warningAlert.text()).toContain("The testing task has expired and the result is no longer available");
+    });
 });
diff --git a/client/src/components/Common/PersistentTaskProgressMonitorAlert.vue b/client/src/components/Common/PersistentTaskProgressMonitorAlert.vue
@@ -9,6 +9,8 @@ import { TaskMonitor } from "@/composables/genericTaskMonitor";
 import { MonitoringRequest, usePersistentProgressTaskMonitor } from "@/composables/persistentProgressMonitor";
 import { useShortTermStorage } from "@/composables/shortTermStorage";
 
+import UtcDate from "@/components/UtcDate.vue";
+
 library.add(faSpinner);
 
 interface Props {
@@ -46,8 +48,19 @@ const props = withDefaults(defineProps<Props>(), {
 
 const { getDownloadObjectUrl } = useShortTermStorage();
 
-const { hasMonitoringData, isRunning, isCompleted, hasFailed, requestHasFailed, storedTaskId, status, start, reset } =
-    usePersistentProgressTaskMonitor(props.monitorRequest, props.useMonitor);
+const {
+    hasMonitoringData,
+    isRunning,
+    isCompleted,
+    hasFailed,
+    requestHasFailed,
+    storedTaskId,
+    status,
+    hasExpired,
+    expirationDate,
+    start,
+    reset,
+} = usePersistentProgressTaskMonitor(props.monitorRequest, props.useMonitor);
 
 const downloadUrl = computed(() => {
     // We can only download the result if the task is completed and the task type is short_term_storage.
@@ -66,7 +79,12 @@ watch(
     () => props.taskId,
     (newTaskId, oldTaskId) => {
         if (newTaskId && newTaskId !== oldTaskId) {
-            start({ taskId: newTaskId, taskType: props.monitorRequest.taskType, request: props.monitorRequest });
+            start({
+                taskId: newTaskId,
+                taskType: props.monitorRequest.taskType,
+                request: props.monitorRequest,
+                startedAt: new Date(),
+            });
         }
     }
 );
@@ -89,7 +107,10 @@ function dismissAlert() {
 
 <template>
     <div v-if="hasMonitoringData" class="d-flex justify-content-end">
-        <BAlert v-if="isRunning" variant="info" show>
+        <BAlert v-if="hasExpired" variant="warning" show dismissible @dismissed="dismissAlert">
+            The {{ monitorRequest.action }} task has <b>expired</b> and the result is no longer available.
+        </BAlert>
+        <BAlert v-else-if="isRunning" variant="info" show>
             <b>{{ inProgressMessage }}</b>
             <FontAwesomeIcon :icon="faSpinner" class="mr-2" spin />
         </BAlert>
@@ -98,6 +119,10 @@ function dismissAlert() {
             <BLink v-if="downloadUrl" class="download-link" :href="downloadUrl">
                 <b>Download here</b>
             </BLink>
+            <br />
+            <span v-if="expirationDate">
+                This result will <b>expire <UtcDate :date="expirationDate.toISOString()" mode="elapsed" /></b>
+            </span>
         </BAlert>
         <BAlert v-else-if="hasFailed" variant="danger" show dismissible @dismissed="dismissAlert">
             <span>{{ failedMessage }}</span>

diff --git a/client/src/composables/genericTaskMonitor.ts b/client/src/composables/genericTaskMonitor.ts
@@ -43,6 +43,25 @@ export interface TaskMonitor {
      * In case of an error, this will be the error message.
      */
     status: Readonly<Ref<string | undefined>>;
+
+    /**
+     * Loads the status of the task from a stored value.
+     * @param storedStatus The status string to load.
+     */
+    loadStatus: (storedStatus: string) => void;
+
+    /**
+     * Determines if the status represents a final state.
+     * @param status The status string to check.
+     * @returns True if the status is a final state and is not expected to change.
+     */
+    isFinalState: (status?: string) => boolean;
+
+    /**
+     * If defined, the time (in milliseconds) after which the task should be considered expired.
+     * Requests to check the task status after this time should be avoided. As they are not guaranteed to return the correct status.
+     */
+    expirationTime?: number;
 }
 
 /**
@@ -66,6 +85,12 @@ export function useGenericMonitor(options: {
      * The delay can be overridden when calling `waitForTask`.
      */
     defaultPollDelay?: number;
+
+    /** Optional expiration time for the task in milliseconds.
+     * After this time, the task should be considered expired.
+     * Requests to check the task status after this time should be avoided.
+     */
+    expirationTime?: number;
 }): TaskMonitor {
     let timeout: NodeJS.Timeout | null = null;
     let pollDelay = options.defaultPollDelay ?? DEFAULT_POLL_DELAY;
@@ -78,6 +103,14 @@ export function useGenericMonitor(options: {
     const isCompleted = computed(() => options.completedCondition(status.value));
     const hasFailed = computed(() => options.failedCondition(status.value));
 
+    function isFinalState(status?: string) {
+        return options.completedCondition(status) || options.failedCondition(status);
+    }
+
+    function loadStatus(storedStatus: string) {
+        status.value = storedStatus;
+    }
+
     async function waitForTask(taskId: string, pollDelayInMs?: number) {
         pollDelay = pollDelayInMs ?? pollDelay;
         resetState();
@@ -130,10 +163,13 @@ export function useGenericMonitor(options: {
 
     return {
         waitForTask,
+        isFinalState,
+        loadStatus,
         isRunning: readonly(isRunning),
         isCompleted: readonly(isCompleted),
         hasFailed: readonly(hasFailed),
         requestHasFailed: readonly(requestHasFailed),
         status: readonly(status),
+        expirationTime: options.expirationTime,
     };
 }
diff --git a/client/src/composables/persistentProgressMonitor.test.ts b/client/src/composables/persistentProgressMonitor.test.ts
@@ -20,6 +20,8 @@ jest.mock("@vueuse/core", () => ({
 
 function useMonitorMock(): TaskMonitor {
     const isRunning = ref(false);
+    const status = ref();
+
     return {
         waitForTask: jest.fn().mockImplementation(() => {
             isRunning.value = true;
@@ -28,7 +30,12 @@ function useMonitorMock(): TaskMonitor {
         isCompleted: ref(false),
         hasFailed: ref(false),
         requestHasFailed: ref(false),
-        status: ref(""),
+        status,
+        expirationTime: 1000,
+        isFinalState: jest.fn(),
+        loadStatus(storedStatus) {
+            status.value = storedStatus;
+        },
     };
 }
 const mockUseMonitor = useMonitorMock();
@@ -55,6 +62,7 @@ describe("usePersistentProgressTaskMonitor", () => {
             taskId: "123",
             taskType: "task",
             request: MOCK_REQUEST,
+            startedAt: new Date(),
         };
 
         const { start, isRunning } = usePersistentProgressTaskMonitor(MOCK_REQUEST, mockUseMonitor, monitoringData);
@@ -75,6 +83,7 @@ describe("usePersistentProgressTaskMonitor", () => {
             taskId: "123",
             taskType: "task",
             request: MOCK_REQUEST,
+            startedAt: new Date(),
         };
 
         const { reset, hasMonitoringData } = usePersistentProgressTaskMonitor(