Merge pull request #18551 from davelopez/task_monitor_consider_task_expiration

Add task expiration to persistent task monitor
This commit is contained in:
Marius van den Beek
2024-07-19 11:18:35 +02:00
committed by GitHub
9 changed files with 313 additions and 8 deletions
@@ -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");
});
});
@@ -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>
@@ -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,
};
}
@@ -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(
@@ -1,5 +1,5 @@
import { StorageSerializers, useLocalStorage } from "@vueuse/core";
import { computed } from "vue";
import { computed, watch } from "vue";
import type { TaskMonitor } from "./genericTaskMonitor";
@@ -80,6 +80,18 @@ export interface MonitoringData {
* The information about the task.
*/
request: MonitoringRequest;
/**
* The time when the task was started.
*/
startedAt: Date;
/**
* The status of the task when it was last checked.
* The meaning of the status string is up to the monitor implementation.
* In case of an error, this will be the error message.
*/
status?: string;
}
/**
@@ -93,6 +105,18 @@ export function usePersistentProgressTaskMonitor(
useMonitor: TaskMonitor,
monitoringData: MonitoringData | null = null
) {
const {
waitForTask,
isFinalState,
loadStatus,
isRunning,
isCompleted,
hasFailed,
requestHasFailed,
status,
expirationTime,
} = useMonitor;
const localStorageKey = getPersistentKey(request);
const currentMonitoringData = useLocalStorage<MonitoringData | null>(localStorageKey, monitoringData, {
@@ -103,7 +127,42 @@ export function usePersistentProgressTaskMonitor(
return Boolean(currentMonitoringData.value);
});
const { waitForTask, isRunning, isCompleted, hasFailed, requestHasFailed, status } = useMonitor;
const canExpire = computed(() => {
return Boolean(expirationTime);
});
const hasExpired = computed(() => {
if (!currentMonitoringData.value || !expirationTime) {
return false;
}
const now = new Date();
const startedAt = new Date(currentMonitoringData.value.startedAt);
const elapsedTimeInMs = now.getTime() - startedAt.getTime();
return elapsedTimeInMs > expirationTime!;
});
const expirationDate = computed(() => {
if (!currentMonitoringData.value || !expirationTime) {
return undefined;
}
const startedAt = new Date(currentMonitoringData.value.startedAt);
return new Date(startedAt.getTime() + expirationTime);
});
watch(
status,
(newStatus) => {
if (newStatus && currentMonitoringData.value) {
currentMonitoringData.value = {
...currentMonitoringData.value,
status: newStatus,
};
}
},
{ immediate: true }
);
async function start(monitoringData?: MonitoringData) {
if (monitoringData) {
@@ -114,6 +173,18 @@ export function usePersistentProgressTaskMonitor(
throw new Error("No monitoring data provided or stored. Cannot start monitoring progress.");
}
if (isFinalState(currentMonitoringData.value.status)) {
// The task has already finished no need to start monitoring again.
// Instead, reload the stored status to update the UI.
return loadStatus(currentMonitoringData.value.status!);
}
if (hasExpired.value) {
// The monitoring data has expired. Requesting the status again will likely
// return incorrect results. Reset the monitoring data to start fresh.
return;
}
return waitForTask(currentMonitoringData.value.taskId);
}
@@ -137,12 +208,59 @@ export function usePersistentProgressTaskMonitor(
* Clears the monitoring data in the local storage.
*/
reset,
/**
* The task is still running.
*/
isRunning,
/**
* The task has been completed successfully.
*/
isCompleted,
/**
* Indicates the task has failed and will not yield results.
*/
hasFailed,
/**
* If true, the status of the task cannot be determined because of a request error.
*/
requestHasFailed,
/**
* Indicates that there is monitoring data stored.
*/
hasMonitoringData,
/**
* The task ID stored in the monitoring data or undefined if no monitoring data is available.
*/
storedTaskId: currentMonitoringData.value?.taskId,
/**
* The current status of the task.
* The meaning of the status string is up to the monitor implementation.
* In case of an error, this will be the error message.
*/
status,
/**
* True if the monitoring data can expire.
*/
canExpire,
/**
* True if the monitoring data has expired.
* The monitoring data expires after the expiration time has passed since the task was started.
*/
hasExpired,
/**
* The expiration date for the monitoring data.
* After this date, the monitoring data is considered expired and should not be used.
*/
expirationDate,
};
}
@@ -60,4 +60,44 @@ describe("useShortTermStorageMonitor", () => {
expect(isCompleted.value).toBe(false);
expect(status.value).toBe("Request failed");
});
it("should load the status from the stored monitoring data", async () => {
const { loadStatus, isRunning, isCompleted, hasFailed, status } = useShortTermStorageMonitor();
const storedStatus = "READY";
loadStatus(storedStatus);
expect(status.value).toBe(storedStatus);
expect(isRunning.value).toBe(false);
expect(isCompleted.value).toBe(true);
expect(hasFailed.value).toBe(false);
});
describe("isFinalState", () => {
it("should indicate is final state when the task is completed", async () => {
const { waitForTask, isFinalState, isRunning, isCompleted, hasFailed, status } =
useShortTermStorageMonitor();
expect(isFinalState(status.value)).toBe(false);
waitForTask(COMPLETED_TASK_ID);
await flushPromises();
expect(isFinalState(status.value)).toBe(true);
expect(isRunning.value).toBe(false);
expect(isCompleted.value).toBe(true);
expect(hasFailed.value).toBe(false);
});
it("should indicate is final state when the task has failed", async () => {
const { waitForTask, isFinalState, isRunning, isCompleted, hasFailed, status } =
useShortTermStorageMonitor();
expect(isFinalState(status.value)).toBe(false);
waitForTask(REQUEST_FAILED_TASK_ID);
await flushPromises();
expect(isFinalState(status.value)).toBe(true);
expect(isRunning.value).toBe(false);
expect(isCompleted.value).toBe(false);
expect(hasFailed.value).toBe(true);
});
});
});
@@ -3,6 +3,7 @@ import { fetcher } from "@/api/schema";
import { useGenericMonitor } from "./genericTaskMonitor";
const DEFAULT_POLL_DELAY = 10000;
const DEFAULT_EXPIRATION_TIME = 1000 * 60 * 60 * 24; // 24 hours
const getTempStorageRequestReady = fetcher
.path("/api/short_term_storage/{storage_request_id}/ready")
@@ -27,5 +28,6 @@ export function useShortTermStorageMonitor() {
completedCondition: (status?: string) => status === READY_STATE,
failedCondition: (status?: string) => typeof status === "string" && !VALID_STATES.includes(status),
defaultPollDelay: DEFAULT_POLL_DELAY,
expirationTime: DEFAULT_EXPIRATION_TIME,
});
}
@@ -75,4 +75,42 @@ describe("useTaskMonitor", () => {
expect(isCompleted.value).toBe(false);
expect(status.value).toBe("Request failed");
});
it("should load the status from the stored monitoring data", async () => {
const { loadStatus, isRunning, isCompleted, hasFailed, status } = useTaskMonitor();
const storedStatus = "SUCCESS";
loadStatus(storedStatus);
expect(status.value).toBe(storedStatus);
expect(isRunning.value).toBe(false);
expect(isCompleted.value).toBe(true);
expect(hasFailed.value).toBe(false);
});
describe("isFinalState", () => {
it("should indicate is final state when the task is completed", async () => {
const { waitForTask, isFinalState, isRunning, isCompleted, hasFailed, status } = useTaskMonitor();
expect(isFinalState(status.value)).toBe(false);
waitForTask(COMPLETED_TASK_ID);
await flushPromises();
expect(isFinalState(status.value)).toBe(true);
expect(isRunning.value).toBe(false);
expect(isCompleted.value).toBe(true);
expect(hasFailed.value).toBe(false);
});
it("should indicate is final state when the task has failed", async () => {
const { waitForTask, isFinalState, isRunning, isCompleted, hasFailed, status } = useTaskMonitor();
expect(isFinalState(status.value)).toBe(false);
waitForTask(FAILED_TASK_ID);
await flushPromises();
expect(isFinalState(status.value)).toBe(true);
expect(isRunning.value).toBe(false);
expect(isCompleted.value).toBe(false);
expect(hasFailed.value).toBe(true);
});
});
});
+2
View File
@@ -5,6 +5,7 @@ import { useGenericMonitor } from "./genericTaskMonitor";
const SUCCESS_STATE = "SUCCESS";
const FAILURE_STATE = "FAILURE";
const DEFAULT_POLL_DELAY = 10000;
const DEFAULT_EXPIRATION_TIME = 1000 * 60 * 60 * 24; // 24 hours
const getTaskStatus = fetcher.path("/api/tasks/{task_id}/state").method("get").create();
@@ -21,5 +22,6 @@ export function useTaskMonitor() {
completedCondition: (status?: string) => status === SUCCESS_STATE,
failedCondition: (status?: string) => status === FAILURE_STATE,
defaultPollDelay: DEFAULT_POLL_DELAY,
expirationTime: DEFAULT_EXPIRATION_TIME,
});
}