From d0d58ef258764305d2d64b9c47a86d6444c46ab2 Mon Sep 17 00:00:00 2001 From: Morgan Wowk Date: Mon, 2 Mar 2026 16:41:46 -0800 Subject: [PATCH] chore: Prevent unnecessary API calls to get container details --- src/components/shared/AnnouncementBanners.tsx | 3 +- .../FlowCanvas/TaskNode/TaskOverview/logs.tsx | 52 +++++++------------ .../TaskNode/TaskOverview/tests/logs.test.tsx | 6 ++- src/components/shared/TaskDetails/Details.tsx | 1 + .../shared/TaskDetails/ExecutionDetails.tsx | 8 ++- src/services/executionService.ts | 7 ++- src/utils/executionStatus.ts | 17 ++++++ 7 files changed, 56 insertions(+), 38 deletions(-) diff --git a/src/components/shared/AnnouncementBanners.tsx b/src/components/shared/AnnouncementBanners.tsx index 529d1e11e..152f24a63 100644 --- a/src/components/shared/AnnouncementBanners.tsx +++ b/src/components/shared/AnnouncementBanners.tsx @@ -1,8 +1,9 @@ +import "@/config/announcements"; + import { useState } from "react"; import { InfoBox } from "@/components/shared/InfoBox"; import { BlockStack } from "@/components/ui/layout"; -import "@/config/announcements"; import { getStorage } from "@/utils/typedStorage"; interface DismissedAnnouncementsStorage { diff --git a/src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/logs.tsx b/src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/logs.tsx index 51df2a94e..118e6ac9b 100644 --- a/src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/logs.tsx +++ b/src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/logs.tsx @@ -7,6 +7,7 @@ import { Link } from "@/components/ui/link"; import { Spinner } from "@/components/ui/spinner"; import { useBackend } from "@/providers/BackendProvider"; import { getBackendStatusString } from "@/utils/backend"; +import { CONTAINER_STATUSES_PRE_LAUNCH } from "@/utils/executionStatus"; const LogDisplay = ({ logs, @@ -52,15 +53,14 @@ const LogDisplay = ({ ); }; +/** + * Statuses where the container is running and actively producing logs. + * Used to decide whether to poll for new log content. + */ const isStatusActivelyLogging = (status?: string): boolean => { - if (!status) { - return false; - } switch (status) { case "RUNNING": case "PENDING": - case "QUEUED": - case "WAITING_FOR_UPSTREAM": case "CANCELLING": return true; default: @@ -68,24 +68,14 @@ const isStatusActivelyLogging = (status?: string): boolean => { } }; +/** + * Returns true if the container may have logs worth fetching. + */ const shouldStatusHaveLogs = (status?: string): boolean => { if (!status) { return false; } - - if (isStatusActivelyLogging(status)) { - return true; - } - - switch (status) { - case "FAILED": - case "SYSTEM_ERROR": - case "SUCCEEDED": - case "CANCELLED": - return true; - default: - return false; - } + return !CONTAINER_STATUSES_PRE_LAUNCH.has(status); }; const getLogs = async (executionId: string, backendUrl: string) => { @@ -104,8 +94,9 @@ const Logs = ({ }) => { const { backendUrl, configured, available } = useBackend(); - const [isLogging, setIsLogging] = useState(!!executionId); - const [shouldHaveLogs, setShouldHaveLogs] = useState(!!executionId); + const shouldFetch = !!executionId && shouldStatusHaveLogs(status); + const shouldPoll = shouldFetch && isStatusActivelyLogging(status); + const [logs, setLogs] = useState<{ log_text?: string; system_error_exception_full?: string; @@ -113,18 +104,11 @@ const Logs = ({ const { data, isLoading, error, refetch } = useQuery({ queryKey: ["logs", executionId], queryFn: () => getLogs(String(executionId), backendUrl), - enabled: isLogging, - refetchInterval: 5000, + enabled: shouldFetch, + refetchInterval: shouldPoll ? 5000 : false, refetchIntervalInBackground: false, }); - useEffect(() => { - if (status) { - setIsLogging(isStatusActivelyLogging(status)); - setShouldHaveLogs(shouldStatusHaveLogs(status)); - } - }, [status]); - useEffect(() => { if (data && !error) { setLogs({ @@ -139,8 +123,10 @@ const Logs = ({ }, [data, error]); useEffect(() => { - refetch(); - }, [backendUrl, refetch]); + if (shouldFetch) { + refetch(); + } + }, [backendUrl, refetch, shouldFetch]); if (!configured) { return ( @@ -150,7 +136,7 @@ const Logs = ({ ); } - if (!shouldHaveLogs) { + if (!shouldFetch && !logs) { return ( Logs are available only for active, queued and completed executions. diff --git a/src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/tests/logs.test.tsx b/src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/tests/logs.test.tsx index 0af199f17..0c96229a5 100644 --- a/src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/tests/logs.test.tsx +++ b/src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/tests/logs.test.tsx @@ -87,18 +87,20 @@ describe("OpenLogsInNewWindowLink", () => { const statusesThatShouldHaveLogs: ContainerExecutionStatus[] = [ "RUNNING", "PENDING", - "QUEUED", - "WAITING_FOR_UPSTREAM", "CANCELLING", "FAILED", "SYSTEM_ERROR", "SUCCEEDED", + // CANCELLED may have logs when the task was cancelled mid-run; the + // backend uploads logs before termination in that path. "CANCELLED", ]; const statusesThatShouldNotHaveLogs: ContainerExecutionStatus[] = [ "INVALID", "UNINITIALIZED", + "QUEUED", + "WAITING_FOR_UPSTREAM", "SKIPPED", ]; diff --git a/src/components/shared/TaskDetails/Details.tsx b/src/components/shared/TaskDetails/Details.tsx index 47067a0ad..bdc6df057 100644 --- a/src/components/shared/TaskDetails/Details.tsx +++ b/src/components/shared/TaskDetails/Details.tsx @@ -125,6 +125,7 @@ const TaskDetailsInternal = ({ )} diff --git a/src/components/shared/TaskDetails/ExecutionDetails.tsx b/src/components/shared/TaskDetails/ExecutionDetails.tsx index f3c1fbe0c..b3df8ef2d 100644 --- a/src/components/shared/TaskDetails/ExecutionDetails.tsx +++ b/src/components/shared/TaskDetails/ExecutionDetails.tsx @@ -18,12 +18,14 @@ import { InfoBox } from "../InfoBox"; interface ExecutionDetailsProps { executionId: string; componentSpec?: ComponentSpec; + status?: string; className?: string; } export const ExecutionDetails = ({ executionId, componentSpec, + status, className, }: ExecutionDetailsProps) => { const { backendUrl } = useBackend(); @@ -35,7 +37,11 @@ export const ExecutionDetails = ({ data: containerState, isLoading: isLoadingContainerState, error: containerStateError, - } = useFetchContainerExecutionState(executionId, backendUrl); + } = useFetchContainerExecutionState( + isSubgraph ? undefined : executionId, + backendUrl, + status, + ); const getExecutionItems = () => { const items: AttributeProps[] = [ diff --git a/src/services/executionService.ts b/src/services/executionService.ts index 48d49a29a..aee3b522b 100644 --- a/src/services/executionService.ts +++ b/src/services/executionService.ts @@ -13,6 +13,7 @@ import { TWENTY_FOUR_HOURS_IN_MS, } from "@/utils/constants"; import { + CONTAINER_STATUSES_PRE_LAUNCH, flattenExecutionStatusStats, getOverallExecutionStatusFromStats, } from "@/utils/executionStatus"; @@ -69,11 +70,15 @@ const fetchContainerExecutionState = async ( export const useFetchContainerExecutionState = ( executionId: string | undefined, backendUrl: string, + status?: string, ) => { + const shouldFetch = + !!executionId && (!status || !CONTAINER_STATUSES_PRE_LAUNCH.has(status)); + return useQuery({ queryKey: ["container-execution-state", executionId], queryFn: () => fetchContainerExecutionState(executionId!, backendUrl), - enabled: !!executionId, + enabled: shouldFetch, refetchOnWindowFocus: false, }); }; diff --git a/src/utils/executionStatus.ts b/src/utils/executionStatus.ts index c850532a5..5f2e269b7 100644 --- a/src/utils/executionStatus.ts +++ b/src/utils/executionStatus.ts @@ -42,6 +42,23 @@ export const EXECUTION_STATUS_BG_COLORS: Record = { UNINITIALIZED: "bg-yellow-400", }; +/** + * Statuses where the container was never launched. + * Used to skip fetches to /container_state and /container_log — both of which + * require a container execution record that won't exist for these statuses. + * + * CANCELLED is excluded: a task cancelled mid-run (while PENDING or RUNNING) + * will have a container execution record and uploaded logs, and the frontend + * cannot distinguish that from a pre-launch cancellation by status alone. + */ +export const CONTAINER_STATUSES_PRE_LAUNCH = new Set([ + "INVALID", + "UNINITIALIZED", + "QUEUED", + "WAITING_FOR_UPSTREAM", + "SKIPPED", +]); + /** * Statuses considered "in progress" (not terminal). */