From 3f8c354b27470db71731ff9d6ac9a15f74c15ece Mon Sep 17 00:00:00 2001 From: Ahmed Awan Date: Wed, 7 Aug 2024 15:25:15 -0500 Subject: [PATCH 1/2] [24.1] Optimize `useInvocationGraph` for Invocation view This ensures the graph is only loaded onto the editor once (initially). After that, we update the `steps` ref for each step _only when_ the `step_jobs_summary` changes. Fixes https://github.com/galaxyproject/galaxy/issues/18660 --- client/src/composables/useInvocationGraph.ts | 243 +++++++++++-------- 1 file changed, 143 insertions(+), 100 deletions(-) diff --git a/client/src/composables/useInvocationGraph.ts b/client/src/composables/useInvocationGraph.ts index f9416c471119..fab2f5226dbe 100644 --- a/client/src/composables/useInvocationGraph.ts +++ b/client/src/composables/useInvocationGraph.ts @@ -10,7 +10,12 @@ import { } from "@fortawesome/free-solid-svg-icons"; import Vue, { computed, type Ref, ref } from "vue"; -import { stepJobsSummaryFetcher, type StepJobSummary, type WorkflowInvocationElementView } from "@/api/invocations"; +import { + type InvocationStep, + stepJobsSummaryFetcher, + type StepJobSummary, + type WorkflowInvocationElementView, +} from "@/api/invocations"; import { isWorkflowInput } from "@/components/Workflow/constants"; import { fromSimple } from "@/components/Workflow/Editor/modules/model"; import { getWorkflowFull } from "@/components/Workflow/workflows.services"; @@ -73,8 +78,11 @@ export function useInvocationGraph( library.add(faCheckCircle, faClock, faExclamationTriangle, faForward, faPause, faSpinner, faTrash); const steps = ref<{ [index: string]: GraphStep }>({}); + const stepsPopulated = ref(false); const storeId = computed(() => `invocation-${invocation.value.id}`); + const lastStepsJobsSummary = ref([]); + /** The full invocation mapped onto the original workflow */ const invocationGraph = ref(null); @@ -105,118 +113,153 @@ export function useInvocationGraph( } // get the job summary for each step in the invocation - const { data: stepJobsSummary } = await stepJobsSummaryFetcher({ invocation_id: invocation.value.id }); - - /** The original steps of the workflow */ - const originalSteps: Record = { ...loadedWorkflow.value.steps }; - - // for each step in the workflow, store the state and status of jobs - for (let i = 0; i < Object.keys(originalSteps).length; i++) { - /** An invocation graph step */ - const graphStepFromWfStep = { ...originalSteps[i] } as GraphStep; - - /** The type of the step (subworkflow, input, tool, etc.) */ - let type; - if (graphStepFromWfStep.type === "subworkflow") { - type = "subworkflow"; - } else if (isWorkflowInput(graphStepFromWfStep.type)) { - type = "input"; + const { data: stepsJobsSummary } = await stepJobsSummaryFetcher({ invocation_id: invocation.value.id }); + + // if the steps have not been populated or the job states have changed, update the steps + // TODO: What if the state of something not in the stepsJobsSummary has changed? (e.g.: subworkflows...) + if ( + !stepsPopulated.value || + JSON.stringify(stepsJobsSummary) !== JSON.stringify(lastStepsJobsSummary.value) + ) { + updateSteps(stepsJobsSummary); + + // Load the invocation graph into the editor the first time + if (!stepsPopulated.value) { + invocationGraph.value!.steps = { ...steps.value }; + await fromSimple(storeId.value, invocationGraph.value as any); + stepsPopulated.value = true; } + } + } catch (e) { + rethrowSimple(e); + } + } - /** The raw invocation step */ - const invocationStep = invocation.value.steps[i]; - - if (type !== "input") { - // there is an invocation step for this workflow step - if (invocationStep) { - /** The `populated_state` for this graph step. (This may or may not be used to - * derive the `state` for this invocation graph step) */ - let populatedState; - - if (type === "subworkflow") { - // if the step is a subworkflow, get the populated state from the invocation step - populatedState = invocationStep.state || undefined; - - /* TODO: - Note that subworkflows are often in the `scheduled` state regardless of whether - their output is successful or not. One good way to visually show if a subworkflow was - successful is to set `graphStepFromWfStep.state = subworkflow.output?.state`. - */ - } + /** Update the steps of the invocation graph with the step job summaries, or initialize the steps + * if they haven't been populated yet. + * @param stepsJobsSummary - The job summary for each step in the invocation + * */ + function updateSteps(stepsJobsSummary: StepJobSummary[]) { + /** Initialize with the original steps of the workflow, else update the existing graph steps */ + const fullSteps: Record = !stepsPopulated.value + ? { ...loadedWorkflow.value.steps } + : steps.value; + + // for each step, store the state and status of jobs + for (let i = 0; i < Object.keys(fullSteps).length; i++) { + /** An invocation graph step (initialized with the original workflow step) */ + let graphStepFromWfStep; + if (!steps.value[i]) { + graphStepFromWfStep = { ...fullSteps[i] } as GraphStep; + } else { + graphStepFromWfStep = steps.value[i] as GraphStep; + } - // First, try setting the state of the graph step based on its jobs' states or the populated state - else { - /** The step job summary for the invocation step (based on its job id) */ - const invocationStepSummary = stepJobsSummary.find((stepJobSummary: StepJobSummary) => { - if (stepJobSummary.model === "ImplicitCollectionJobs") { - return stepJobSummary.id === invocationStep.implicit_collection_jobs_id; - } else { - return stepJobSummary.id === invocationStep.job_id; - } - }); - - if (invocationStepSummary) { - // the step is not a subworkflow, get the populated state from the invocation step summary - populatedState = invocationStepSummary.populated_state; - - if (invocationStepSummary.states) { - const statesForThisStep = Object.keys(invocationStepSummary.states); - // set the state of the graph step based on the job states for this step - graphStepFromWfStep.state = getStepStateFromJobStates(statesForThisStep); - } - // now store the job states for this step in the graph step - graphStepFromWfStep.jobs = invocationStepSummary.states; - } else { - // TODO: There is no summary for this step's `job_id`; what does this mean? - graphStepFromWfStep.state = "waiting"; - } + /** The raw invocation step */ + const invocationStep = invocation.value.steps[i]; + + if (!isWorkflowInput(graphStepFromWfStep.type)) { + let invocationStepSummary: StepJobSummary | undefined; + if (invocationStep) { + invocationStepSummary = stepsJobsSummary.find((stepJobSummary: StepJobSummary) => { + if (stepJobSummary.model === "ImplicitCollectionJobs") { + return stepJobSummary.id === invocationStep.implicit_collection_jobs_id; + } else { + return stepJobSummary.id === invocationStep.job_id; } + }); + } + updateStep(graphStepFromWfStep, invocationStep, invocationStepSummary); + } - // If the state still hasn't been set, set it based on the populated state - if (!graphStepFromWfStep.state) { - if (populatedState === "scheduled" || populatedState === "ready") { - graphStepFromWfStep.state = "queued"; - } else if (populatedState === "resubmitted") { - graphStepFromWfStep.state = "new"; - } else if (populatedState === "failed") { - graphStepFromWfStep.state = "error"; - } else if (populatedState === "deleting") { - graphStepFromWfStep.state = "deleted"; - } else if (populatedState && !["stop", "stopped"].includes(populatedState)) { - graphStepFromWfStep.state = populatedState as GraphStep["state"]; - } - } - } + // add the graph step to the steps object if it doesn't exist yet + if (!steps.value[i]) { + Vue.set(steps.value, i, graphStepFromWfStep); + } + } - // there is no invocation step for this workflow step, it is probably queued - else { - graphStepFromWfStep.state = "queued"; - } + lastStepsJobsSummary.value = stepsJobsSummary; + } + + /** + * Store the state and status of jobs for the graph step. + * @param graphStep - Invocation graph step + * @param invocationStep - The invocation step for the workflow step + * @param invocationStepSummary - The step job summary for the invocation step (based on its job id) + */ + function updateStep( + graphStep: GraphStep, + invocationStep: InvocationStep | undefined, + invocationStepSummary: StepJobSummary | undefined + ) { + // there is an invocation step for this workflow step + if (invocationStep) { + /** The `populated_state` for this graph step. (This may or may not be used to + * derive the `state` for this invocation graph step) */ + let populatedState; + + if (graphStep.type === "subworkflow") { + // if the step is a subworkflow, get the populated state from the invocation step + populatedState = invocationStep.state || undefined; + + /* TODO: + Note that subworkflows are often in the `scheduled` state regardless of whether + their output is successful or not. One good way to visually show if a subworkflow was + successful is to set `graphStep.state = subworkflow.output?.state`. + */ + } + + // First, try setting the state of the graph step based on its jobs' states or the populated state + else { + if (invocationStepSummary) { + // the step is not a subworkflow, get the populated state from the invocation step summary + populatedState = invocationStepSummary.populated_state; - /** Setting the header class for the graph step */ - graphStepFromWfStep.headerClass = { - "node-header-invocation": true, - [`header-${graphStepFromWfStep.state}`]: !!graphStepFromWfStep.state, - }; - // TODO: maybe a different one for inputs? Currently they have no state either. - - /** Setting the header icon for the graph step */ - if (graphStepFromWfStep.state) { - graphStepFromWfStep.headerIcon = iconClasses[graphStepFromWfStep.state]?.icon; - graphStepFromWfStep.headerIconSpin = iconClasses[graphStepFromWfStep.state]?.spin; + if (invocationStepSummary.states) { + const statesForThisStep = Object.keys(invocationStepSummary.states); + // set the state of the graph step based on the job states for this step + graphStep.state = getStepStateFromJobStates(statesForThisStep); } + // now store the job states for this step in the graph step + graphStep.jobs = invocationStepSummary.states; + } else { + // TODO: There is no summary for this step's `job_id`; what does this mean? + graphStep.state = "waiting"; } + } - // update the invocation graph steps object - Vue.set(steps.value, i, graphStepFromWfStep); + // If the state still hasn't been set, set it based on the populated state + if (!graphStep.state) { + if (populatedState === "scheduled" || populatedState === "ready") { + graphStep.state = "queued"; + } else if (populatedState === "resubmitted") { + graphStep.state = "new"; + } else if (populatedState === "failed") { + graphStep.state = "error"; + } else if (populatedState === "deleting") { + graphStep.state = "deleted"; + } else if (populatedState && !["stop", "stopped"].includes(populatedState)) { + graphStep.state = populatedState as GraphStep["state"]; + } } + } - invocationGraph.value!.steps = { ...steps.value }; + // there is no invocation step for this workflow step, it is probably queued + else { + graphStep.state = "queued"; + } - // Load the invocation graph into the editor every time - await fromSimple(storeId.value, invocationGraph.value as any); - } catch (e) { - rethrowSimple(e); + /** Setting the header class for the graph step */ + graphStep.headerClass = { + "node-header-invocation": true, + [`header-${graphStep.state}`]: !!graphStep.state, + }; + // TODO: maybe a different one for inputs? Currently they have no state either. + + /** Setting the header icon for the graph step */ + if (graphStep.state) { + graphStep.headerIcon = iconClasses[graphStep.state]?.icon; + graphStep.headerIconSpin = iconClasses[graphStep.state]?.spin; } } From 8856b83ee8f3ccf023b4fa1d227b737b907a104d Mon Sep 17 00:00:00 2001 From: Ahmed Awan Date: Wed, 7 Aug 2024 16:03:08 -0500 Subject: [PATCH 2/2] only update individual step when state or jobs change --- client/src/composables/useInvocationGraph.ts | 60 ++++++++++++-------- 1 file changed, 35 insertions(+), 25 deletions(-) diff --git a/client/src/composables/useInvocationGraph.ts b/client/src/composables/useInvocationGraph.ts index fab2f5226dbe..be5ea6e008de 100644 --- a/client/src/composables/useInvocationGraph.ts +++ b/client/src/composables/useInvocationGraph.ts @@ -8,7 +8,7 @@ import { faSpinner, faTrash, } from "@fortawesome/free-solid-svg-icons"; -import Vue, { computed, type Ref, ref } from "vue"; +import { computed, type Ref, ref, set } from "vue"; import { type InvocationStep, @@ -174,7 +174,7 @@ export function useInvocationGraph( // add the graph step to the steps object if it doesn't exist yet if (!steps.value[i]) { - Vue.set(steps.value, i, graphStepFromWfStep); + set(steps.value, i, graphStepFromWfStep); } } @@ -182,7 +182,7 @@ export function useInvocationGraph( } /** - * Store the state and status of jobs for the graph step. + * Store the state, jobs and class for the graph step based on the invocation step and its job summary. * @param graphStep - Invocation graph step * @param invocationStep - The invocation step for the workflow step * @param invocationStepSummary - The step job summary for the invocation step (based on its job id) @@ -192,6 +192,9 @@ export function useInvocationGraph( invocationStep: InvocationStep | undefined, invocationStepSummary: StepJobSummary | undefined ) { + /** The new state for the graph step */ + let newState = graphStep.state; + // there is an invocation step for this workflow step if (invocationStep) { /** The `populated_state` for this graph step. (This may or may not be used to @@ -218,48 +221,55 @@ export function useInvocationGraph( if (invocationStepSummary.states) { const statesForThisStep = Object.keys(invocationStepSummary.states); // set the state of the graph step based on the job states for this step - graphStep.state = getStepStateFromJobStates(statesForThisStep); + newState = getStepStateFromJobStates(statesForThisStep); + } + // now store the job states for this step in the graph step, if they changed since the last time + if (JSON.stringify(graphStep.jobs) !== JSON.stringify(invocationStepSummary.states)) { + set(graphStep, "jobs", invocationStepSummary.states); } - // now store the job states for this step in the graph step - graphStep.jobs = invocationStepSummary.states; } else { // TODO: There is no summary for this step's `job_id`; what does this mean? - graphStep.state = "waiting"; + newState = "waiting"; } } // If the state still hasn't been set, set it based on the populated state - if (!graphStep.state) { + if (!newState) { if (populatedState === "scheduled" || populatedState === "ready") { - graphStep.state = "queued"; + newState = "queued"; } else if (populatedState === "resubmitted") { - graphStep.state = "new"; + newState = "new"; } else if (populatedState === "failed") { - graphStep.state = "error"; + newState = "error"; } else if (populatedState === "deleting") { - graphStep.state = "deleted"; + newState = "deleted"; } else if (populatedState && !["stop", "stopped"].includes(populatedState)) { - graphStep.state = populatedState as GraphStep["state"]; + newState = populatedState as GraphStep["state"]; } } } // there is no invocation step for this workflow step, it is probably queued else { - graphStep.state = "queued"; + newState = "queued"; } - /** Setting the header class for the graph step */ - graphStep.headerClass = { - "node-header-invocation": true, - [`header-${graphStep.state}`]: !!graphStep.state, - }; - // TODO: maybe a different one for inputs? Currently they have no state either. - - /** Setting the header icon for the graph step */ - if (graphStep.state) { - graphStep.headerIcon = iconClasses[graphStep.state]?.icon; - graphStep.headerIconSpin = iconClasses[graphStep.state]?.spin; + // if the state has changed, update the graph step + if (graphStep.state !== newState) { + graphStep.state = newState; + + /** Setting the header class for the graph step */ + graphStep.headerClass = { + "node-header-invocation": true, + [`header-${graphStep.state}`]: !!graphStep.state, + }; + // TODO: maybe a different one for inputs? Currently they have no state either. + + /** Setting the header icon for the graph step */ + if (graphStep.state) { + graphStep.headerIcon = iconClasses[graphStep.state]?.icon; + graphStep.headerIconSpin = iconClasses[graphStep.state]?.spin; + } } }