From c328cdfbbbe33232eacbb1921f7576c488e267aa Mon Sep 17 00:00:00 2001 From: James Date: Tue, 17 Jan 2023 13:51:20 -0500 Subject: [PATCH 1/4] fix: resume signal node signalId & executionId Signed-off-by: James --- .../ExecutionDetailsActions.tsx | 2 +- .../Tables/NodeExecutionActions.tsx | 4 ++-- .../Launch/LaunchForm/LaunchFormActions.tsx | 23 +++++++++++++++++-- .../Launch/LaunchForm/LaunchFormDialog.tsx | 9 ++++---- .../Launch/LaunchForm/ResumeForm.tsx | 3 ++- .../Launch/LaunchForm/ResumeSignalForm.tsx | 17 +++++++------- .../Launch/LaunchForm/launchMachine.ts | 7 +++++- .../Launch/LaunchForm/useResumeFormState.ts | 14 +++++++---- .../ReactFlow/PausedTasksComponent.tsx | 6 +++-- .../ReactFlow/customNodeComponents.tsx | 2 +- 10 files changed, 60 insertions(+), 27 deletions(-) diff --git a/packages/console/src/components/Executions/ExecutionDetails/ExecutionDetailsActions.tsx b/packages/console/src/components/Executions/ExecutionDetails/ExecutionDetailsActions.tsx index a7aaa7c92..112eabcb6 100644 --- a/packages/console/src/components/Executions/ExecutionDetails/ExecutionDetailsActions.tsx +++ b/packages/console/src/components/Executions/ExecutionDetails/ExecutionDetailsActions.tsx @@ -162,7 +162,7 @@ export const ExecutionDetailsActions = ({ diff --git a/packages/console/src/components/Executions/Tables/NodeExecutionActions.tsx b/packages/console/src/components/Executions/Tables/NodeExecutionActions.tsx index 9a05906eb..4863e6af2 100644 --- a/packages/console/src/components/Executions/Tables/NodeExecutionActions.tsx +++ b/packages/console/src/components/Executions/Tables/NodeExecutionActions.tsx @@ -1,5 +1,5 @@ import { IconButton, Tooltip } from '@material-ui/core'; -import { NodeExecution } from 'models/Execution/types'; +import { NodeExecution, NodeExecutionIdentifier } from 'models/Execution/types'; import * as React from 'react'; import InputsAndOutputsIcon from '@material-ui/icons/Tv'; import PlayCircleOutlineIcon from '@material-ui/icons/PlayCircleOutline'; @@ -137,7 +137,7 @@ export const NodeExecutionActions = ({ diff --git a/packages/console/src/components/Launch/LaunchForm/LaunchFormActions.tsx b/packages/console/src/components/Launch/LaunchForm/LaunchFormActions.tsx index 54e9a3911..1cce84b40 100644 --- a/packages/console/src/components/Launch/LaunchForm/LaunchFormActions.tsx +++ b/packages/console/src/components/Launch/LaunchForm/LaunchFormActions.tsx @@ -54,8 +54,27 @@ export const LaunchFormActions: React.FC = ({ Routes.ExecutionDetails.makeUrl(newState.context.resultExecutionId), ); } - if ((newState.context as TaskResumeContext).compiledNode) { - onCancel(); + const context = newState.context as TaskResumeContext; + if (context.compiledNode) { + // only for resume + if (context.nodeExecutionId) { + // this cancels the modal + onCancel(); + // this reloads the page + history.push( + Routes.ExecutionDetails.makeUrl( + context.nodeExecutionId.executionId, + ), + ); + } + } else { + if (newState.context.resultExecutionId) { + history.push( + Routes.ExecutionDetails.makeUrl( + newState.context.resultExecutionId, + ), + ); + } } } }); diff --git a/packages/console/src/components/Launch/LaunchForm/LaunchFormDialog.tsx b/packages/console/src/components/Launch/LaunchForm/LaunchFormDialog.tsx index 4c69a0e48..55a394d59 100644 --- a/packages/console/src/components/Launch/LaunchForm/LaunchFormDialog.tsx +++ b/packages/console/src/components/Launch/LaunchForm/LaunchFormDialog.tsx @@ -7,6 +7,7 @@ import { WorkflowInitialLaunchParameters, } from 'components/Launch/LaunchForm/types'; import { CompiledNode } from 'models/Node/types'; +import { NodeExecutionIdentifier } from 'models/Execution/types'; import { ResumeForm } from './ResumeForm'; interface LaunchFormDialogProps { @@ -17,7 +18,7 @@ interface LaunchFormDialogProps { showLaunchForm: boolean; setShowLaunchForm: React.Dispatch>; compiledNode?: CompiledNode; - nodeId?: string; + nodeExecutionId?: NodeExecutionIdentifier; } function getLaunchProps(id: ResourceIdentifier) { @@ -35,7 +36,7 @@ export const LaunchFormDialog = ({ showLaunchForm, setShowLaunchForm, compiledNode, - nodeId, + nodeExecutionId, }: LaunchFormDialogProps): JSX.Element => { const onCancelLaunch = () => setShowLaunchForm(false); @@ -58,10 +59,10 @@ export const LaunchFormDialog = ({ onClose={onCancelLaunch} {...getLaunchProps(id)} /> - ) : compiledNode && nodeId ? ( + ) : compiledNode && nodeExecutionId ? ( diff --git a/packages/console/src/components/Launch/LaunchForm/ResumeForm.tsx b/packages/console/src/components/Launch/LaunchForm/ResumeForm.tsx index 5e02c03aa..4274db2b3 100644 --- a/packages/console/src/components/Launch/LaunchForm/ResumeForm.tsx +++ b/packages/console/src/components/Launch/LaunchForm/ResumeForm.tsx @@ -1,3 +1,4 @@ +import { NodeExecutionIdentifier } from 'models/Execution/types'; import { CompiledNode } from 'models/Node/types'; import * as React from 'react'; import { useState } from 'react'; @@ -11,7 +12,7 @@ import { BaseLaunchFormProps, TaskInitialLaunchParameters } from './types'; interface ResumeFormProps extends BaseLaunchFormProps { compiledNode: CompiledNode; initialParameters?: TaskInitialLaunchParameters; - nodeId: string; + nodeExecutionId: NodeExecutionIdentifier; } /** Renders the form for requesting a resume request on a gate node */ diff --git a/packages/console/src/components/Launch/LaunchForm/ResumeSignalForm.tsx b/packages/console/src/components/Launch/LaunchForm/ResumeSignalForm.tsx index 9355fbdab..4ec5c6142 100644 --- a/packages/console/src/components/Launch/LaunchForm/ResumeSignalForm.tsx +++ b/packages/console/src/components/Launch/LaunchForm/ResumeSignalForm.tsx @@ -1,7 +1,8 @@ import { DialogContent, Typography } from '@material-ui/core'; import { getCacheKey } from 'components/Cache/utils'; -import React, { useState, useContext, useEffect, useMemo } from 'react'; -import { NodeExecution } from 'models/Execution/types'; +import * as React from 'react'; +import { useState, useContext, useEffect, useMemo } from 'react'; +import { NodeExecution, NodeExecutionIdentifier } from 'models/Execution/types'; import { NodeExecutionsByIdContext } from 'components/Executions/contexts'; import { useNodeExecutionData } from 'components/hooks/useNodeExecution'; import { LiteralMapViewer } from 'components/Literals/LiteralMapViewer'; @@ -24,23 +25,23 @@ import { LaunchFormActions } from './LaunchFormActions'; export interface ResumeSignalFormProps extends BaseLaunchFormProps { compiledNode: CompiledNode; initialParameters?: TaskInitialLaunchParameters; - nodeId: string; + nodeExecutionId: NodeExecutionIdentifier; } /** Renders the form for requesting a resume request on a gate node */ export const ResumeSignalForm: React.FC = ({ compiledNode, - nodeId, + nodeExecutionId, onClose, }) => { const { formInputsRef, state, service } = useResumeFormState({ compiledNode, - nodeId, + nodeExecutionId, onClose, }); const { nodeExecutionsById } = useContext(NodeExecutionsByIdContext); const [nodeExecution, setNodeExecution] = useState( - nodeExecutionsById[nodeId], + nodeExecutionsById[nodeExecutionId.nodeId], ); const styles = useStyles(); const baseState = state as BaseInterpretedLaunchState; @@ -55,9 +56,9 @@ export const ResumeSignalForm: React.FC = ({ }, [state.context.parsedInputs]); useEffect(() => { - const newNodeExecution = nodeExecutionsById[nodeId]; + const newNodeExecution = nodeExecutionsById[nodeExecutionId.nodeId]; setNodeExecution(newNodeExecution); - }, [nodeId]); + }, [nodeExecutionId.nodeId]); return ( <> diff --git a/packages/console/src/components/Launch/LaunchForm/launchMachine.ts b/packages/console/src/components/Launch/LaunchForm/launchMachine.ts index 2248f690b..cc90abccf 100644 --- a/packages/console/src/components/Launch/LaunchForm/launchMachine.ts +++ b/packages/console/src/components/Launch/LaunchForm/launchMachine.ts @@ -1,6 +1,9 @@ import { Admin, Core, Protobuf } from '@flyteorg/flyteidl-types'; import { Identifier, NamedEntityIdentifier } from 'models/Common/types'; -import { WorkflowExecutionIdentifier } from 'models/Execution/types'; +import { + NodeExecutionIdentifier, + WorkflowExecutionIdentifier, +} from 'models/Execution/types'; import { LaunchPlan } from 'models/Launch/types'; import { CompiledNode } from 'models/Node/types'; import { Task } from 'models/Task/types'; @@ -99,6 +102,7 @@ export interface TaskLaunchContext extends BaseLaunchContext { export interface TaskResumeContext extends BaseLaunchContext { compiledNode?: CompiledNode; + nodeExecutionId?: NodeExecutionIdentifier; } export enum LaunchState { @@ -190,6 +194,7 @@ export type BaseLaunchTypestate = value: LaunchState.SUBMIT_SUCCEEDED; context: BaseLaunchContext & { resultExecutionId: WorkflowExecutionIdentifier; + nodeExecutionId?: NodeExecutionIdentifier; }; } | { diff --git a/packages/console/src/components/Launch/LaunchForm/useResumeFormState.ts b/packages/console/src/components/Launch/LaunchForm/useResumeFormState.ts index 4f1b7c9a8..253f677ed 100644 --- a/packages/console/src/components/Launch/LaunchForm/useResumeFormState.ts +++ b/packages/console/src/components/Launch/LaunchForm/useResumeFormState.ts @@ -1,8 +1,8 @@ import { useMachine } from '@xstate/react'; import { defaultStateMachineConfig } from 'components/common/constants'; import { APIContextValue, useAPIContext } from 'components/data/apiContext'; -import { Core } from '@flyteorg/flyteidl-types'; import { partial } from 'lodash'; +import { NodeExecutionIdentifier } from 'models/Execution/types'; import { CompiledNode } from 'models/Node/types'; import { RefObject, useMemo, useRef } from 'react'; import { @@ -27,7 +27,7 @@ import { interface ResumeFormProps extends BaseLaunchFormProps { compiledNode: CompiledNode; initialParameters?: TaskInitialLaunchParameters; - nodeId: string; + nodeExecutionId: NodeExecutionIdentifier; } async function loadInputs({ compiledNode }: TaskResumeContext) { @@ -63,7 +63,7 @@ async function validate(formInputsRef: RefObject) { async function submit( { resumeSignalNode }: APIContextValue, formInputsRef: RefObject, - { compiledNode }: TaskResumeContext, + { compiledNode, nodeExecutionId }: TaskResumeContext, ) { if (!compiledNode?.gateNode?.signal?.signalId) { throw new Error('SignalId is empty'); @@ -75,8 +75,10 @@ async function submit( const literals = formInputsRef.current.getValues(); const response = await resumeSignalNode({ - id: compiledNode?.gateNode?.signal - ?.signalId as unknown as Core.SignalIdentifier, + id: { + signalId: compiledNode?.gateNode?.signal?.signalId, + executionId: nodeExecutionId?.executionId, + }, value: literals['signal'], }); @@ -99,6 +101,7 @@ function getServices( */ export function useResumeFormState({ compiledNode, + nodeExecutionId, }: ResumeFormProps): ResumeFormState { const apiContext = useAPIContext(); const formInputsRef = useRef(null); @@ -117,6 +120,7 @@ export function useResumeFormState({ services, context: { compiledNode, + nodeExecutionId, }, }); diff --git a/packages/console/src/components/flytegraph/ReactFlow/PausedTasksComponent.tsx b/packages/console/src/components/flytegraph/ReactFlow/PausedTasksComponent.tsx index b016f9d28..a9f0d04a9 100644 --- a/packages/console/src/components/flytegraph/ReactFlow/PausedTasksComponent.tsx +++ b/packages/console/src/components/flytegraph/ReactFlow/PausedTasksComponent.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import { useState } from 'react'; +import { useState, useContext } from 'react'; import { Badge, Button, withStyles } from '@material-ui/core'; import { TaskNames } from 'components/Executions/ExecutionDetails/Timeline/TaskNames'; import { dNode } from 'models/Graph/types'; @@ -9,6 +9,7 @@ import { COLOR_SPECTRUM } from 'components/Theme/colorSpectrum'; import { nodeExecutionPhaseConstants } from 'components/Executions/constants'; import { LaunchFormDialog } from 'components/Launch/LaunchForm/LaunchFormDialog'; import { useNodeExecutionContext } from 'components/Executions/contextProvider/NodeExecutionDetails'; +import { NodeExecutionsByIdContext } from 'components/Executions/contexts'; import { graphButtonContainer, graphButtonStyle, @@ -34,6 +35,7 @@ export const PausedTasksComponent: React.FC = ({ pausedNodes, initialIsVisible = false, }) => { + const nodeExecutionsById = useContext(NodeExecutionsByIdContext); const { compiledWorkflowClosure } = useNodeExecutionContext(); const [isVisible, setIsVisible] = useState(initialIsVisible); const [showResumeForm, setShowResumeForm] = useState(false); @@ -108,7 +110,7 @@ export const PausedTasksComponent: React.FC = ({ diff --git a/packages/console/src/components/flytegraph/ReactFlow/customNodeComponents.tsx b/packages/console/src/components/flytegraph/ReactFlow/customNodeComponents.tsx index 3e356d0ba..738f45a20 100644 --- a/packages/console/src/components/flytegraph/ReactFlow/customNodeComponents.tsx +++ b/packages/console/src/components/flytegraph/ReactFlow/customNodeComponents.tsx @@ -275,7 +275,7 @@ export const ReactFlowGateNode = ({ data }: RFNode) => { From 7e4a961cde16e5e94a8fed1e6cfc16d460a3beb1 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 17 Jan 2023 20:44:53 -0500 Subject: [PATCH 2/4] fix: for approvedcondition Signed-off-by: James --- .../Launch/LaunchForm/useResumeFormState.ts | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/packages/console/src/components/Launch/LaunchForm/useResumeFormState.ts b/packages/console/src/components/Launch/LaunchForm/useResumeFormState.ts index 253f677ed..a7420164b 100644 --- a/packages/console/src/components/Launch/LaunchForm/useResumeFormState.ts +++ b/packages/console/src/components/Launch/LaunchForm/useResumeFormState.ts @@ -2,6 +2,7 @@ import { useMachine } from '@xstate/react'; import { defaultStateMachineConfig } from 'components/common/constants'; import { APIContextValue, useAPIContext } from 'components/data/apiContext'; import { partial } from 'lodash'; +import { SimpleType } from 'models/Common/types'; import { NodeExecutionIdentifier } from 'models/Execution/types'; import { CompiledNode } from 'models/Node/types'; import { RefObject, useMemo, useRef } from 'react'; @@ -36,6 +37,14 @@ async function loadInputs({ compiledNode }: TaskResumeContext) { } const signalType = compiledNode.gateNode?.signal?.type; if (!signalType) { + if (compiledNode.gateNode?.approve?.signalId) { + // for approvedCondition + + return { + parsedInputs: [], + unsupportedRequiredInputs: [], + }; + } throw new Error('Failed to load inputs: missing signal.type'); } const parsedInputs: ParsedInput[] = [ @@ -65,7 +74,10 @@ async function submit( formInputsRef: RefObject, { compiledNode, nodeExecutionId }: TaskResumeContext, ) { - if (!compiledNode?.gateNode?.signal?.signalId) { + const signalId = + compiledNode?.gateNode?.signal?.signalId || compiledNode?.gateNode?.approve?.signalId; + const isApprovedCondition = !!compiledNode?.gateNode?.approve?.signalId; + if (!signalId) { throw new Error('SignalId is empty'); } if (formInputsRef.current === null) { @@ -76,10 +88,10 @@ async function submit( const response = await resumeSignalNode({ id: { - signalId: compiledNode?.gateNode?.signal?.signalId, + signalId, executionId: nodeExecutionId?.executionId, }, - value: literals['signal'], + value: isApprovedCondition ? { scalar: { primitive: { boolean: true } } } : literals['signal'], }); return response; From dd6c69d0507a589d9ead07e6fe3a3f50d26afb18 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 17 Jan 2023 21:09:48 -0500 Subject: [PATCH 3/4] fix: unit test Signed-off-by: James --- .../LaunchForm/test/ResumeSignalForm.test.tsx | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/console/src/components/Launch/LaunchForm/test/ResumeSignalForm.test.tsx b/packages/console/src/components/Launch/LaunchForm/test/ResumeSignalForm.test.tsx index a4e019c93..f21a350bb 100644 --- a/packages/console/src/components/Launch/LaunchForm/test/ResumeSignalForm.test.tsx +++ b/packages/console/src/components/Launch/LaunchForm/test/ResumeSignalForm.test.tsx @@ -24,22 +24,27 @@ import { NodeExecutionDetailsContext } from 'components/Executions/contextProvid import { signalInputName } from './constants'; import { ResumeSignalForm } from '../ResumeSignalForm'; -const mockNodeExecutionId = 'n0'; +const mockScopeId = 'n0'; const mockNodeId = 'node0'; +const mockExecutionId = { domain: 'domain', name: 'name', project: 'project' }; +const mockNodeExecutionId = { + nodeId: mockScopeId, + executionId: mockExecutionId, +}; const mockNodeExecutionsById = { - [mockNodeExecutionId]: { + [mockScopeId]: { closure: { createdAt: dateToTimestamp(new Date()), outputUri: '', phase: NodeExecutionPhase.UNDEFINED, }, id: { - executionId: { domain: 'domain', name: 'name', project: 'project' }, + executionId: mockExecutionId, nodeId: mockNodeId, }, inputUri: '', - scopedId: mockNodeExecutionId, + scopedId: mockScopeId, }, }; @@ -60,7 +65,7 @@ const createMockCompiledWorkflowClosure = ( }); const createMockCompiledNode = (type?: Core.ILiteralType): CompiledNode => ({ - id: mockNodeExecutionId, + id: mockScopeId, metadata: { name: 'my-signal-name', timeout: '3600s', @@ -112,7 +117,7 @@ describe('ResumeSignalForm', () => { From 22c8b9cc9a5dc755164597057d8fad509c6ef9d4 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 17 Jan 2023 22:06:59 -0500 Subject: [PATCH 4/4] fix: unit test for pausedtaskscomponent Signed-off-by: James --- .../test/PausedTasksComponent.test.tsx | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/packages/console/src/components/flytegraph/ReactFlow/test/PausedTasksComponent.test.tsx b/packages/console/src/components/flytegraph/ReactFlow/test/PausedTasksComponent.test.tsx index 70a6c4d7f..48c00659f 100644 --- a/packages/console/src/components/flytegraph/ReactFlow/test/PausedTasksComponent.test.tsx +++ b/packages/console/src/components/flytegraph/ReactFlow/test/PausedTasksComponent.test.tsx @@ -3,6 +3,9 @@ import { fireEvent, render } from '@testing-library/react'; import { NodeExecutionDetailsContext } from 'components/Executions/contextProvider/NodeExecutionDetails'; import { mockWorkflowId } from 'mocks/data/fixtures/types'; import { dTypes } from 'models/Graph/types'; +import { NodeExecutionsByIdContext } from 'components/Executions/contexts'; +import { NodeExecutionPhase } from 'models/Execution/enums'; +import { dateToTimestamp } from 'common/utils'; import { PausedTasksComponent } from '../PausedTasksComponent'; const pausedNodes = [ @@ -24,6 +27,22 @@ const pausedNodes = [ }, ]; +const nodeExecutionsById = { + n1: { + closure: { + createdAt: dateToTimestamp(new Date()), + outputUri: '', + phase: NodeExecutionPhase.UNDEFINED, + }, + id: { + executionId: { domain: 'domain', name: 'name', project: 'project' }, + nodeId: 'n1', + }, + inputUri: '', + scopedId: 'n1', + }, +}; + const compiledWorkflowClosure = { primary: { connections: { @@ -77,7 +96,9 @@ describe('flytegraph > ReactFlow > PausedTasksComponent', () => { compiledWorkflowClosure, }} > - + + + , );