Skip to content

Commit

Permalink
Add support for ApprovedCondition for GateNodes (#688)
Browse files Browse the repository at this point in the history
* fix: resume signal node signalId & executionId

Signed-off-by: James <[email protected]>

* fix: for approvedcondition

Signed-off-by: James <[email protected]>

* fix: unit test

Signed-off-by: James <[email protected]>

* fix: unit test for pausedtaskscomponent

Signed-off-by: James <[email protected]>

---------

Signed-off-by: James <[email protected]>
  • Loading branch information
james-union authored Feb 21, 2023
1 parent 27d4344 commit 860e356
Show file tree
Hide file tree
Showing 12 changed files with 107 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ export const ExecutionDetailsActions = ({
<LaunchFormDialog
compiledNode={compiledNode}
initialParameters={initialParameters}
nodeId={nodeExecutionId.nodeId}
nodeExecutionId={nodeExecutionId}
showLaunchForm={showResumeForm}
setShowLaunchForm={setShowResumeForm}
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -137,7 +137,7 @@ export const NodeExecutionActions = ({
<LaunchFormDialog
compiledNode={compiledNode}
initialParameters={initialParameters}
nodeId={execution.id.nodeId}
nodeExecutionId={execution.id as NodeExecutionIdentifier}
showLaunchForm={showResumeForm}
setShowLaunchForm={setShowResumeForm}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,27 @@ export const LaunchFormActions: React.FC<LaunchFormActionsProps> = ({
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,
),
);
}
}
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -17,7 +18,7 @@ interface LaunchFormDialogProps {
showLaunchForm: boolean;
setShowLaunchForm: React.Dispatch<React.SetStateAction<boolean>>;
compiledNode?: CompiledNode;
nodeId?: string;
nodeExecutionId?: NodeExecutionIdentifier;
}

function getLaunchProps(id: ResourceIdentifier) {
Expand All @@ -35,7 +36,7 @@ export const LaunchFormDialog = ({
showLaunchForm,
setShowLaunchForm,
compiledNode,
nodeId,
nodeExecutionId,
}: LaunchFormDialogProps): JSX.Element => {
const onCancelLaunch = () => setShowLaunchForm(false);

Expand All @@ -58,10 +59,10 @@ export const LaunchFormDialog = ({
onClose={onCancelLaunch}
{...getLaunchProps(id)}
/>
) : compiledNode && nodeId ? (
) : compiledNode && nodeExecutionId ? (
<ResumeForm
initialParameters={initialParameters}
nodeId={nodeId}
nodeExecutionId={nodeExecutionId}
onClose={onCancelLaunch}
compiledNode={compiledNode}
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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 */
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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<ResumeSignalFormProps> = ({
compiledNode,
nodeId,
nodeExecutionId,
onClose,
}) => {
const { formInputsRef, state, service } = useResumeFormState({
compiledNode,
nodeId,
nodeExecutionId,
onClose,
});
const { nodeExecutionsById } = useContext(NodeExecutionsByIdContext);
const [nodeExecution, setNodeExecution] = useState<NodeExecution>(
nodeExecutionsById[nodeId],
nodeExecutionsById[nodeExecutionId.nodeId],
);
const styles = useStyles();
const baseState = state as BaseInterpretedLaunchState;
Expand All @@ -55,9 +56,9 @@ export const ResumeSignalForm: React.FC<ResumeSignalFormProps> = ({
}, [state.context.parsedInputs]);

useEffect(() => {
const newNodeExecution = nodeExecutionsById[nodeId];
const newNodeExecution = nodeExecutionsById[nodeExecutionId.nodeId];
setNodeExecution(newNodeExecution);
}, [nodeId]);
}, [nodeExecutionId.nodeId]);

return (
<>
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -99,6 +102,7 @@ export interface TaskLaunchContext extends BaseLaunchContext {

export interface TaskResumeContext extends BaseLaunchContext {
compiledNode?: CompiledNode;
nodeExecutionId?: NodeExecutionIdentifier;
}

export enum LaunchState {
Expand Down Expand Up @@ -190,6 +194,7 @@ export type BaseLaunchTypestate =
value: LaunchState.SUBMIT_SUCCEEDED;
context: BaseLaunchContext & {
resultExecutionId: WorkflowExecutionIdentifier;
nodeExecutionId?: NodeExecutionIdentifier;
};
}
| {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
};

Expand All @@ -60,7 +65,7 @@ const createMockCompiledWorkflowClosure = (
});

const createMockCompiledNode = (type?: Core.ILiteralType): CompiledNode => ({
id: mockNodeExecutionId,
id: mockScopeId,
metadata: {
name: 'my-signal-name',
timeout: '3600s',
Expand Down Expand Up @@ -112,7 +117,7 @@ describe('ResumeSignalForm', () => {
<ResumeSignalForm
onClose={onClose}
compiledNode={mockCompiledNode}
nodeId={mockNodeExecutionId}
nodeExecutionId={mockNodeExecutionId}
/>
</NodeExecutionsByIdContext.Provider>
</NodeExecutionDetailsContext.Provider>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
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 { SimpleType } from 'models/Common/types';
import { NodeExecutionIdentifier } from 'models/Execution/types';
import { CompiledNode } from 'models/Node/types';
import { RefObject, useMemo, useRef } from 'react';
import {
Expand All @@ -27,7 +28,7 @@ import {
interface ResumeFormProps extends BaseLaunchFormProps {
compiledNode: CompiledNode;
initialParameters?: TaskInitialLaunchParameters;
nodeId: string;
nodeExecutionId: NodeExecutionIdentifier;
}

async function loadInputs({ compiledNode }: TaskResumeContext) {
Expand All @@ -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[] = [
Expand Down Expand Up @@ -63,9 +72,12 @@ async function validate(formInputsRef: RefObject<LaunchFormInputsRef>) {
async function submit(
{ resumeSignalNode }: APIContextValue,
formInputsRef: RefObject<LaunchFormInputsRef>,
{ compiledNode }: TaskResumeContext,
{ 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) {
Expand All @@ -75,9 +87,11 @@ async function submit(
const literals = formInputsRef.current.getValues();

const response = await resumeSignalNode({
id: compiledNode?.gateNode?.signal
?.signalId as unknown as Core.SignalIdentifier,
value: literals['signal'],
id: {
signalId,
executionId: nodeExecutionId?.executionId,
},
value: isApprovedCondition ? { scalar: { primitive: { boolean: true } } } : literals['signal'],
});

return response;
Expand All @@ -99,6 +113,7 @@ function getServices(
*/
export function useResumeFormState({
compiledNode,
nodeExecutionId,
}: ResumeFormProps): ResumeFormState {
const apiContext = useAPIContext();
const formInputsRef = useRef<LaunchFormInputsRef>(null);
Expand All @@ -117,6 +132,7 @@ export function useResumeFormState({
services,
context: {
compiledNode,
nodeExecutionId,
},
});

Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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,
Expand All @@ -34,6 +35,7 @@ export const PausedTasksComponent: React.FC<PausedTasksComponentProps> = ({
pausedNodes,
initialIsVisible = false,
}) => {
const nodeExecutionsById = useContext(NodeExecutionsByIdContext);
const { compiledWorkflowClosure } = useNodeExecutionContext();
const [isVisible, setIsVisible] = useState(initialIsVisible);
const [showResumeForm, setShowResumeForm] = useState<boolean>(false);
Expand Down Expand Up @@ -108,7 +110,7 @@ export const PausedTasksComponent: React.FC<PausedTasksComponentProps> = ({
<LaunchFormDialog
compiledNode={compiledNode}
initialParameters={undefined}
nodeId={selectedNodeId}
nodeExecutionId={nodeExecutionsById[selectedNodeId].id}
showLaunchForm={showResumeForm}
setShowLaunchForm={setShowResumeForm}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ export const ReactFlowGateNode = ({ data }: RFNode) => {
<LaunchFormDialog
compiledNode={compiledNode}
initialParameters={undefined}
nodeId={scopedId}
nodeExecutionId={nodeExecutionsById[scopedId].id}
showLaunchForm={showResumeForm}
setShowLaunchForm={setShowResumeForm}
/>
Expand Down
Loading

0 comments on commit 860e356

Please sign in to comment.