diff --git a/package.json b/package.json index 27d2a174d..e59efb99e 100644 --- a/package.json +++ b/package.json @@ -82,7 +82,7 @@ "@commitlint/cli": "^8.3.5", "@commitlint/config-conventional": "^8.3.4", "@date-io/moment": "1.3.9", - "@flyteorg/flyteidl": "0.24.11", + "@flyteorg/flyteidl": "1.1.0", "@material-ui/core": "^4.0.0", "@material-ui/icons": "^4.0.0", "@material-ui/pickers": "^3.2.2", diff --git a/src/components/Executions/ExecutionDetails/ExecutionMetadataExtra.tsx b/src/components/Executions/ExecutionDetails/ExecutionMetadataExtra.tsx index 8ff819ffd..6d2d847b8 100644 --- a/src/components/Executions/ExecutionDetails/ExecutionMetadataExtra.tsx +++ b/src/components/Executions/ExecutionDetails/ExecutionMetadataExtra.tsx @@ -6,6 +6,7 @@ import { Execution } from 'models/Execution/types'; import * as React from 'react'; import { getLaunchPlan } from 'models/Launch/api'; import { LaunchPlanSpec } from 'models/Launch/types'; +import { dashedValueString } from 'common/constants'; import { ExecutionMetadataLabels } from './constants'; const useStyles = makeStyles((theme: Theme) => { @@ -39,6 +40,7 @@ export const ExecutionMetadataExtra: React.FC<{ maxParallelism, rawOutputDataConfig, securityContext, + interruptible, } = execution.spec; const [launchPlanSpec, setLaunchPlanSpec] = React.useState>({}); @@ -67,6 +69,10 @@ export const ExecutionMetadataExtra: React.FC<{ label: ExecutionMetadataLabels.parallelism, value: maxParallelism, }, + { + label: ExecutionMetadataLabels.interruptible, + value: interruptible ? (interruptible.value ? 'true' : 'false') : dashedValueString, + }, ]; return ( diff --git a/src/components/Executions/ExecutionDetails/RelaunchExecutionForm.tsx b/src/components/Executions/ExecutionDetails/RelaunchExecutionForm.tsx index af71e92ba..f1bdaa2a2 100644 --- a/src/components/Executions/ExecutionDetails/RelaunchExecutionForm.tsx +++ b/src/components/Executions/ExecutionDetails/RelaunchExecutionForm.tsx @@ -35,6 +35,7 @@ function useRelaunchWorkflowFormState({ execution }: RelaunchExecutionFormProps) annotations, authRole, securityContext, + interruptible, }, } = execution; @@ -59,6 +60,7 @@ function useRelaunchWorkflowFormState({ execution }: RelaunchExecutionFormProps) annotations, authRole, securityContext, + interruptible, }; }, }, @@ -74,7 +76,7 @@ function useRelaunchTaskFormState({ execution }: RelaunchExecutionFormProps) { defaultValue: {} as TaskInitialLaunchParameters, doFetch: async (execution) => { const { - spec: { authRole, launchPlan: taskId }, + spec: { authRole, launchPlan: taskId, interruptible }, } = execution; const task = await apiContext.getTask(taskId); const inputDefinitions = getTaskInputs(task); @@ -85,7 +87,7 @@ function useRelaunchTaskFormState({ execution }: RelaunchExecutionFormProps) { }, apiContext, ); - return { authRole, values, taskId }; + return { authRole, values, taskId, interruptible }; }, }, execution, diff --git a/src/components/Executions/ExecutionDetails/constants.ts b/src/components/Executions/ExecutionDetails/constants.ts index 8ce69f759..4bc046263 100644 --- a/src/components/Executions/ExecutionDetails/constants.ts +++ b/src/components/Executions/ExecutionDetails/constants.ts @@ -10,6 +10,7 @@ export enum ExecutionMetadataLabels { rawOutputPrefix = 'Raw Output Prefix', parallelism = 'Parallelism', securityContextDefault = 'default', + interruptible = 'Interruptible override', } export const tabs = { diff --git a/src/components/Executions/ExecutionDetails/test/ExecutionMetadata.test.tsx b/src/components/Executions/ExecutionDetails/test/ExecutionMetadata.test.tsx index 0b20ca28b..858217bf3 100644 --- a/src/components/Executions/ExecutionDetails/test/ExecutionMetadata.test.tsx +++ b/src/components/Executions/ExecutionDetails/test/ExecutionMetadata.test.tsx @@ -1,5 +1,6 @@ import { render } from '@testing-library/react'; import { dashedValueString } from 'common/constants'; +import { Protobuf } from 'flyteidl'; import { Execution, WorkflowExecutionIdentifier } from 'models/Execution/types'; import { createMockExecution } from 'models/__mocks__/executionsData'; import * as React from 'react'; @@ -11,6 +12,7 @@ import { ExecutionMetadata } from '../ExecutionMetadata'; const clusterTestId = `metadata-${ExecutionMetadataLabels.cluster}`; const startTimeTestId = `metadata-${ExecutionMetadataLabels.time}`; const durationTestId = `metadata-${ExecutionMetadataLabels.duration}`; +const interruptibleTestId = `metadata-${ExecutionMetadataLabels.interruptible}`; jest.mock('models/Launch/api', () => ({ getLaunchPlan: jest.fn(() => Promise.resolve({ spec: {} })), @@ -75,4 +77,22 @@ describe('ExecutionMetadata', () => { Routes.ExecutionDetails.makeUrl(referenceExecution), ); }); + + it('shows true if execution was marked as interruptible', () => { + execution.spec.interruptible = Protobuf.BoolValue.create({ value: true }); + const { getByTestId } = renderMetadata(); + expect(getByTestId(interruptibleTestId)).toHaveTextContent('true'); + }); + + it('shows false if execution was not marked as interruptible', () => { + execution.spec.interruptible = Protobuf.BoolValue.create({ value: false }); + const { getByTestId } = renderMetadata(); + expect(getByTestId(interruptibleTestId)).toHaveTextContent('false'); + }); + + it('shows dashes if no interruptible value is found in execution spec', () => { + delete execution.spec.interruptible; + const { getByTestId } = renderMetadata(); + expect(getByTestId(interruptibleTestId)).toHaveTextContent(dashedValueString); + }); }); diff --git a/src/components/Executions/ExecutionDetails/test/RelaunchExecutionForm.test.tsx b/src/components/Executions/ExecutionDetails/test/RelaunchExecutionForm.test.tsx index 0a19fe144..ad5be24b4 100644 --- a/src/components/Executions/ExecutionDetails/test/RelaunchExecutionForm.test.tsx +++ b/src/components/Executions/ExecutionDetails/test/RelaunchExecutionForm.test.tsx @@ -9,7 +9,7 @@ import { } from 'components/Launch/LaunchForm/utils'; import { mockSimpleVariables } from 'components/Launch/LaunchForm/__mocks__/mockInputs'; import { primitiveLiteral } from 'components/Launch/LaunchForm/__mocks__/utils'; -import { Admin } from 'flyteidl'; +import { Admin, Protobuf } from 'flyteidl'; import { LiteralMap, ResourceType, Variable } from 'models/Common/types'; import { getExecutionData } from 'models/Execution/api'; import { Execution, ExecutionData } from 'models/Execution/types'; @@ -155,6 +155,39 @@ describe('RelaunchExecutionForm', () => { }), }); }); + + it('should not set interruptible value if not provided', async () => { + delete execution.spec.interruptible; + const { getByText } = renderForm(); + await waitFor(() => expect(getByText(mockContentString))); + checkLaunchFormProps({ + initialParameters: expect.objectContaining({ + interruptible: undefined, + }), + }); + }); + + it('should have correct interruptible value if override is enabled', async () => { + execution.spec.interruptible = Protobuf.BoolValue.create({ value: true }); + const { getByText } = renderForm(); + await waitFor(() => expect(getByText(mockContentString))); + checkLaunchFormProps({ + initialParameters: expect.objectContaining({ + interruptible: Protobuf.BoolValue.create({ value: true }), + }), + }); + }); + + it('should have correct interruptible value if override is disabled', async () => { + execution.spec.interruptible = Protobuf.BoolValue.create({ value: false }); + const { getByText } = renderForm(); + await waitFor(() => expect(getByText(mockContentString))); + checkLaunchFormProps({ + initialParameters: expect.objectContaining({ + interruptible: Protobuf.BoolValue.create({ value: false }), + }), + }); + }); }); describe('Launch form with full inputs', () => { @@ -256,5 +289,38 @@ describe('RelaunchExecutionForm', () => { }), }); }); + + it('should not set interruptible value if not provided', async () => { + delete execution.spec.interruptible; + const { getByText } = renderForm(); + await waitFor(() => expect(getByText(mockContentString))); + checkLaunchFormProps({ + initialParameters: expect.objectContaining({ + interruptible: undefined, + }), + }); + }); + + it('should have correct interruptible value if override is enabled', async () => { + execution.spec.interruptible = Protobuf.BoolValue.create({ value: true }); + const { getByText } = renderForm(); + await waitFor(() => expect(getByText(mockContentString))); + checkLaunchFormProps({ + initialParameters: expect.objectContaining({ + interruptible: Protobuf.BoolValue.create({ value: true }), + }), + }); + }); + + it('should have correct interruptible value if override is disabled', async () => { + execution.spec.interruptible = Protobuf.BoolValue.create({ value: false }); + const { getByText } = renderForm(); + await waitFor(() => expect(getByText(mockContentString))); + checkLaunchFormProps({ + initialParameters: expect.objectContaining({ + interruptible: Protobuf.BoolValue.create({ value: false }), + }), + }); + }); }); }); diff --git a/src/components/Launch/LaunchForm/LaunchInterruptibleInput.tsx b/src/components/Launch/LaunchForm/LaunchInterruptibleInput.tsx new file mode 100644 index 000000000..23412c8b3 --- /dev/null +++ b/src/components/Launch/LaunchForm/LaunchInterruptibleInput.tsx @@ -0,0 +1,112 @@ +import { makeStyles, Theme, Typography } from '@material-ui/core'; +import FormControlLabel from '@material-ui/core/FormControlLabel'; +import Checkbox from '@material-ui/core/Checkbox'; +import * as React from 'react'; +import { Protobuf } from 'flyteidl'; +import { useStyles } from './styles'; +import { LaunchInterruptibleInputRef } from './types'; +import { formStrings } from './constants'; + +export const useInterruptibleStyles = makeStyles((theme: Theme) => ({ + labelIndeterminate: { + color: theme.palette.grey[500], + }, +})); + +const isValueValid = (value: any) => { + return value !== undefined && value !== null; +}; + +interface LaunchInterruptibleInputProps { + initialValue?: Protobuf.IBoolValue | null; +} + +export const LaunchInterruptibleInputImpl: React.ForwardRefRenderFunction< + LaunchInterruptibleInputRef, + LaunchInterruptibleInputProps +> = (props, ref) => { + // interruptible stores the override to enable/disable the setting for an execution + const [interruptible, setInterruptible] = React.useState(false); + // indeterminate tracks whether the interruptible flag is unspecified/indeterminate (true) or an override has been selected (false) + const [indeterminate, setIndeterminate] = React.useState(true); + + React.useEffect(() => { + if (isValueValid(props.initialValue) && isValueValid(props.initialValue!.value)) { + setInterruptible(() => props.initialValue!.value!); + setIndeterminate(() => false); + } else { + setInterruptible(() => false); + setIndeterminate(() => true); + } + }, [props.initialValue?.value]); + + const handleInputChange = React.useCallback(() => { + if (indeterminate) { + setInterruptible(() => true); + setIndeterminate(() => false); + } else if (interruptible) { + setInterruptible(() => false); + setIndeterminate(() => false); + } else { + setInterruptible(() => false); + setIndeterminate(() => true); + } + }, [interruptible, indeterminate]); + + React.useImperativeHandle( + ref, + () => ({ + getValue: () => { + if (indeterminate) { + return null; + } + + return Protobuf.BoolValue.create({ value: interruptible }); + }, + validate: () => true, + }), + [interruptible, indeterminate], + ); + + const styles = useStyles(); + const interruptibleStyles = useInterruptibleStyles(); + + const getInterruptibleLabel = () => { + if (indeterminate) { + return ( + {`${formStrings.interruptible} (no override)`} + ); + } else if (interruptible) { + return {`${formStrings.interruptible} (enabled)`}; + } + return {`${formStrings.interruptible} (disabled)`}; + }; + + return ( +
+
+ Override interruptible flag + + Overrides the interruptible flag of a workflow for a single execution, allowing it to be + forced on or off. If no value was selected, the workflow's default will be used. + +
+
+ + } + label={getInterruptibleLabel()} + /> +
+
+ ); +}; + +export const LaunchInterruptibleInput = React.forwardRef(LaunchInterruptibleInputImpl); diff --git a/src/components/Launch/LaunchForm/LaunchTaskForm.tsx b/src/components/Launch/LaunchForm/LaunchTaskForm.tsx index ce90a8817..a280544fa 100644 --- a/src/components/Launch/LaunchForm/LaunchTaskForm.tsx +++ b/src/components/Launch/LaunchForm/LaunchTaskForm.tsx @@ -7,6 +7,7 @@ import { LaunchFormHeader } from './LaunchFormHeader'; import { LaunchFormInputs } from './LaunchFormInputs'; import { LaunchState } from './launchMachine'; import { LaunchRoleInput } from './LaunchRoleInput'; +import { LaunchInterruptibleInput } from './LaunchInterruptibleInput'; import { SearchableSelector } from './SearchableSelector'; import { useStyles } from './styles'; import { BaseInterpretedLaunchState, BaseLaunchService, LaunchTaskFormProps } from './types'; @@ -15,8 +16,14 @@ import { isEnterInputsState } from './utils'; /** Renders the form for initiating a Launch request based on a Task */ export const LaunchTaskForm: React.FC = (props) => { - const { formInputsRef, roleInputRef, state, service, taskSourceSelectorState } = - useLaunchTaskFormState(props); + const { + formInputsRef, + roleInputRef, + interruptibleInputRef, + state, + service, + taskSourceSelectorState, + } = useLaunchTaskFormState(props); const styles = useStyles(); const baseState = state as BaseInterpretedLaunchState; const baseService = service as BaseLaunchService; @@ -63,6 +70,10 @@ export const LaunchTaskForm: React.FC = (props) => { /> ) : null} + diff --git a/src/components/Launch/LaunchForm/LaunchWorkflowForm.tsx b/src/components/Launch/LaunchForm/LaunchWorkflowForm.tsx index 6f29c54b7..2a1027542 100644 --- a/src/components/Launch/LaunchForm/LaunchWorkflowForm.tsx +++ b/src/components/Launch/LaunchForm/LaunchWorkflowForm.tsx @@ -13,6 +13,7 @@ import { useLaunchWorkflowFormState } from './useLaunchWorkflowFormState'; import { isEnterInputsState } from './utils'; import { LaunchRoleInput } from './LaunchRoleInput'; import { LaunchFormAdvancedInputs } from './LaunchFormAdvancedInputs'; +import { LaunchInterruptibleInput } from './LaunchInterruptibleInput'; /** Renders the form for initiating a Launch request based on a Workflow */ export const LaunchWorkflowForm: React.FC = (props) => { @@ -20,6 +21,7 @@ export const LaunchWorkflowForm: React.FC = (props) => formInputsRef, roleInputRef, advancedOptionsRef, + interruptibleInputRef, state, service, workflowSourceSelectorState, @@ -105,6 +107,10 @@ export const LaunchWorkflowForm: React.FC = (props) => /> ) : null} + diff --git a/src/components/Launch/LaunchForm/constants.ts b/src/components/Launch/LaunchForm/constants.ts index 2e023c1d5..81a326c21 100644 --- a/src/components/Launch/LaunchForm/constants.ts +++ b/src/components/Launch/LaunchForm/constants.ts @@ -10,6 +10,7 @@ export const formStrings = { title: 'Create New Execution', workflowVersion: 'Workflow Version', launchPlan: 'Launch Plan', + interruptible: 'Interruptible', }; export const AuthRoleStrings: { [k in AuthRoleTypes]: AuthRoleMeta } = { diff --git a/src/components/Launch/LaunchForm/launchMachine.ts b/src/components/Launch/LaunchForm/launchMachine.ts index 113e07d83..d9fd0c309 100644 --- a/src/components/Launch/LaunchForm/launchMachine.ts +++ b/src/components/Launch/LaunchForm/launchMachine.ts @@ -1,4 +1,4 @@ -import { Admin, Core } from 'flyteidl'; +import { Admin, Core, Protobuf } from 'flyteidl'; import { Identifier, NamedEntityIdentifier } from 'models/Common/types'; import { WorkflowExecutionIdentifier } from 'models/Execution/types'; import { LaunchPlan } from 'models/Launch/types'; @@ -82,6 +82,7 @@ export interface WorkflowLaunchContext extends BaseLaunchContext { labels?: Admin.ILabels | null; annotations?: Admin.IAnnotations | null; securityContext?: Core.ISecurityContext | null; + interruptible?: Protobuf.IBoolValue | null; } export interface TaskLaunchContext extends BaseLaunchContext { @@ -89,6 +90,7 @@ export interface TaskLaunchContext extends BaseLaunchContext { preferredTaskId?: Identifier; taskVersion?: Identifier; taskVersionOptions?: Task[]; + interruptible?: Protobuf.IBoolValue | null; } export enum LaunchState { diff --git a/src/components/Launch/LaunchForm/test/LaunchTaskForm.test.tsx b/src/components/Launch/LaunchForm/test/LaunchTaskForm.test.tsx index 9afaacd41..769c85287 100644 --- a/src/components/Launch/LaunchForm/test/LaunchTaskForm.test.tsx +++ b/src/components/Launch/LaunchForm/test/LaunchTaskForm.test.tsx @@ -11,7 +11,7 @@ import { import { APIContext } from 'components/data/apiContext'; import { mockAPIContextValue } from 'components/data/__mocks__/apiContext'; import { muiTheme } from 'components/Theme/muiTheme'; -import { Core } from 'flyteidl'; +import { Core, Protobuf } from 'flyteidl'; import { cloneDeep } from 'lodash'; import { RequestConfig } from 'models/AdminEntity/types'; import { Identifier, NamedEntityIdentifier, Variable } from 'models/Common/types'; @@ -594,5 +594,140 @@ describe('LaunchForm: Task', () => { expect(queryByText(cannotLaunchTaskString)).toBeNull(); }); }); + + describe('Interruptible', () => { + it('should render checkbox', async () => { + const { getByLabelText } = renderForm(); + const inputElement = await waitFor(() => + getByLabelText(formStrings.interruptible, { exact: false }), + ); + expect(inputElement).toBeInTheDocument(); + expect(inputElement).not.toBeChecked(); + }); + + it('should use initial values when provided', async () => { + const initialParameters: TaskInitialLaunchParameters = { + taskId: mockTask.id, + interruptible: Protobuf.BoolValue.create({ value: true }), + }; + + const { getByLabelText } = renderForm({ + initialParameters, + }); + + const inputElement = await waitFor(() => + getByLabelText(formStrings.interruptible, { exact: false }), + ); + expect(inputElement).toBeInTheDocument(); + expect(inputElement).toBeChecked(); + }); + + it('should cycle between states correctly', async () => { + const { getByLabelText } = renderForm(); + + let inputElement = await waitFor(() => + getByLabelText(`${formStrings.interruptible} (no override)`, { exact: true }), + ); + expect(inputElement).toBeInTheDocument(); + expect(inputElement).not.toBeChecked(); + expect(inputElement).toHaveAttribute('data-indeterminate', 'true'); + + fireEvent.click(inputElement); + inputElement = await waitFor(() => + getByLabelText(`${formStrings.interruptible} (enabled)`, { exact: true }), + ); + expect(inputElement).toBeInTheDocument(); + expect(inputElement).toBeChecked(); + expect(inputElement).toHaveAttribute('data-indeterminate', 'false'); + + fireEvent.click(inputElement); + inputElement = await waitFor(() => + getByLabelText(`${formStrings.interruptible} (disabled)`, { exact: true }), + ); + expect(inputElement).toBeInTheDocument(); + expect(inputElement).not.toBeChecked(); + expect(inputElement).toHaveAttribute('data-indeterminate', 'false'); + + fireEvent.click(inputElement); + inputElement = await waitFor(() => + getByLabelText(`${formStrings.interruptible} (no override)`, { exact: true }), + ); + expect(inputElement).toBeInTheDocument(); + expect(inputElement).not.toBeChecked(); + expect(inputElement).toHaveAttribute('data-indeterminate', 'true'); + }); + + it('should submit without interruptible override set', async () => { + const { container, getByLabelText } = renderForm(); + + const inputElement = await waitFor(() => + getByLabelText(formStrings.interruptible, { exact: false }), + ); + expect(inputElement).toBeInTheDocument(); + expect(inputElement).not.toBeChecked(); + expect(inputElement).toHaveAttribute('data-indeterminate', 'true'); + + await fillInputs(container); + fireEvent.click(getSubmitButton(container)); + + await waitFor(() => + expect(mockCreateWorkflowExecution).toHaveBeenCalledWith( + expect.objectContaining({ + interruptible: null, + }), + ), + ); + }); + + it('should submit with interruptible override enabled', async () => { + const initialParameters: TaskInitialLaunchParameters = { + interruptible: Protobuf.BoolValue.create({ value: true }), + }; + const { container, getByLabelText } = renderForm({ initialParameters }); + + const inputElement = await waitFor(() => + getByLabelText(formStrings.interruptible, { exact: false }), + ); + expect(inputElement).toBeInTheDocument(); + expect(inputElement).toBeChecked(); + expect(inputElement).toHaveAttribute('data-indeterminate', 'false'); + + await fillInputs(container); + fireEvent.click(getSubmitButton(container)); + + await waitFor(() => + expect(mockCreateWorkflowExecution).toHaveBeenCalledWith( + expect.objectContaining({ + interruptible: Protobuf.BoolValue.create({ value: true }), + }), + ), + ); + }); + + it('should submit with interruptible override disabled', async () => { + const initialParameters: TaskInitialLaunchParameters = { + interruptible: Protobuf.BoolValue.create({ value: false }), + }; + const { container, getByLabelText } = renderForm({ initialParameters }); + + const inputElement = await waitFor(() => + getByLabelText(formStrings.interruptible, { exact: false }), + ); + expect(inputElement).toBeInTheDocument(); + expect(inputElement).not.toBeChecked(); + expect(inputElement).toHaveAttribute('data-indeterminate', 'false'); + + await fillInputs(container); + fireEvent.click(getSubmitButton(container)); + + await waitFor(() => + expect(mockCreateWorkflowExecution).toHaveBeenCalledWith( + expect.objectContaining({ + interruptible: Protobuf.BoolValue.create({ value: false }), + }), + ), + ); + }); + }); }); }); diff --git a/src/components/Launch/LaunchForm/test/LaunchWorkflowForm.test.tsx b/src/components/Launch/LaunchForm/test/LaunchWorkflowForm.test.tsx index 5f46b2915..579431f8f 100644 --- a/src/components/Launch/LaunchForm/test/LaunchWorkflowForm.test.tsx +++ b/src/components/Launch/LaunchForm/test/LaunchWorkflowForm.test.tsx @@ -11,7 +11,7 @@ import { import { APIContext } from 'components/data/apiContext'; import { mockAPIContextValue } from 'components/data/__mocks__/apiContext'; import { muiTheme } from 'components/Theme/muiTheme'; -import { Core } from 'flyteidl'; +import { Core, Protobuf } from 'flyteidl'; import { cloneDeep, get } from 'lodash'; import * as Long from 'long'; import { RequestConfig } from 'models/AdminEntity/types'; @@ -754,5 +754,150 @@ describe('LaunchForm: Workflow', () => { expect(queryByText(cannotLaunchWorkflowString)).toBeNull(); }); }); + + describe('Interruptible', () => { + it('should render checkbox', async () => { + const { getByLabelText } = renderForm(); + const inputElement = await waitFor(() => + getByLabelText(formStrings.interruptible, { exact: false }), + ); + expect(inputElement).toBeInTheDocument(); + expect(inputElement).not.toBeChecked(); + }); + + it('should use initial values when provided', async () => { + const initialParameters: WorkflowInitialLaunchParameters = { + workflowId: mockWorkflowVersions[2].id, + interruptible: Protobuf.BoolValue.create({ value: true }), + }; + + const { getByLabelText } = renderForm({ + initialParameters, + }); + + const inputElement = await waitFor(() => + getByLabelText(formStrings.interruptible, { exact: false }), + ); + expect(inputElement).toBeInTheDocument(); + expect(inputElement).toBeChecked(); + }); + + it('should cycle between states correctly when clicked', async () => { + const { getByLabelText } = renderForm(); + + let inputElement = await waitFor(() => + getByLabelText(`${formStrings.interruptible} (no override)`, { exact: true }), + ); + expect(inputElement).toBeInTheDocument(); + expect(inputElement).not.toBeChecked(); + expect(inputElement).toHaveAttribute('data-indeterminate', 'true'); + + fireEvent.click(inputElement); + inputElement = await waitFor(() => + getByLabelText(`${formStrings.interruptible} (enabled)`, { exact: true }), + ); + expect(inputElement).toBeInTheDocument(); + expect(inputElement).toBeChecked(); + expect(inputElement).toHaveAttribute('data-indeterminate', 'false'); + + fireEvent.click(inputElement); + inputElement = await waitFor(() => + getByLabelText(`${formStrings.interruptible} (disabled)`, { exact: true }), + ); + expect(inputElement).toBeInTheDocument(); + expect(inputElement).not.toBeChecked(); + expect(inputElement).toHaveAttribute('data-indeterminate', 'false'); + + fireEvent.click(inputElement); + inputElement = await waitFor(() => + getByLabelText(`${formStrings.interruptible} (no override)`, { exact: true }), + ); + expect(inputElement).toBeInTheDocument(); + expect(inputElement).not.toBeChecked(); + expect(inputElement).toHaveAttribute('data-indeterminate', 'true'); + }); + + it('should submit without interruptible override set', async () => { + const { container, getByLabelText } = renderForm(); + + const inputElement = await waitFor(() => + getByLabelText(formStrings.interruptible, { exact: false }), + ); + expect(inputElement).toBeInTheDocument(); + expect(inputElement).not.toBeChecked(); + expect(inputElement).toHaveAttribute('data-indeterminate', 'true'); + + const integerInput = getByLabelText(integerInputName, { + exact: false, + }); + fireEvent.change(integerInput, { target: { value: '123' } }); + await waitFor(() => expect(integerInput).toBeValid()); + fireEvent.click(getSubmitButton(container)); + + await waitFor(() => + expect(mockCreateWorkflowExecution).toHaveBeenCalledWith( + expect.objectContaining({ + interruptible: null, + }), + ), + ); + }); + + it('should submit with interruptible override enabled', async () => { + const initialParameters: WorkflowInitialLaunchParameters = { + interruptible: Protobuf.BoolValue.create({ value: true }), + }; + const { container, getByLabelText } = renderForm({ initialParameters }); + + const inputElement = await waitFor(() => + getByLabelText(formStrings.interruptible, { exact: false }), + ); + expect(inputElement).toBeInTheDocument(); + expect(inputElement).toBeChecked(); + expect(inputElement).toHaveAttribute('data-indeterminate', 'false'); + + const integerInput = getByLabelText(integerInputName, { + exact: false, + }); + fireEvent.change(integerInput, { target: { value: '123' } }); + fireEvent.click(getSubmitButton(container)); + + await waitFor(() => + expect(mockCreateWorkflowExecution).toHaveBeenCalledWith( + expect.objectContaining({ + interruptible: Protobuf.BoolValue.create({ value: true }), + }), + ), + ); + }); + + it('should submit with interruptible override disabled', async () => { + const initialParameters: WorkflowInitialLaunchParameters = { + interruptible: Protobuf.BoolValue.create({ value: false }), + }; + const { container, getByLabelText } = renderForm({ initialParameters }); + + const inputElement = await waitFor(() => + getByLabelText(formStrings.interruptible, { exact: false }), + ); + expect(inputElement).toBeInTheDocument(); + expect(inputElement).not.toBeChecked(); + expect(inputElement).toHaveAttribute('data-indeterminate', 'false'); + + const integerInput = getByLabelText(integerInputName, { + exact: false, + }); + fireEvent.change(integerInput, { target: { value: '123' } }); + fireEvent.click(getSubmitButton(container)); + + await waitFor(() => + expect(mockCreateWorkflowExecution).toHaveBeenCalledWith( + expect.objectContaining({ + interruptible: Protobuf.BoolValue.create({ value: false }), + }), + ), + ); + }); + }); }); }); diff --git a/src/components/Launch/LaunchForm/types.ts b/src/components/Launch/LaunchForm/types.ts index 60d4d5289..b3ace5b7d 100644 --- a/src/components/Launch/LaunchForm/types.ts +++ b/src/components/Launch/LaunchForm/types.ts @@ -1,4 +1,4 @@ -import { Admin, Core } from 'flyteidl'; +import { Admin, Core, Protobuf } from 'flyteidl'; import { BlobDimensionality, Identifier, @@ -60,6 +60,7 @@ export interface WorkflowInitialLaunchParameters extends BaseInitialLaunchParame rawOutputDataConfig?: Admin.IRawOutputDataConfig | null; labels?: Admin.ILabels | null; annotations?: Admin.IAnnotations | null; + interruptible?: Protobuf.IBoolValue | null; } export interface LaunchWorkflowFormProps extends BaseLaunchFormProps { @@ -71,6 +72,7 @@ export interface TaskInitialLaunchParameters extends BaseInitialLaunchParameters taskId?: Identifier; authRole?: Admin.IAuthRole; securityContext?: Core.ISecurityContext; + interruptible?: Protobuf.IBoolValue | null; } export interface LaunchTaskFormProps extends BaseLaunchFormProps { taskId: NamedEntityIdentifier; @@ -93,6 +95,12 @@ export interface LaunchRoleInputRef { getValue(): LaunchRoles; validate(): boolean; } + +export interface LaunchInterruptibleInputRef { + getValue(): Protobuf.IBoolValue | null | undefined; + validate(): boolean; +} + export interface LaunchAdvancedOptionsRef { getValues(): Admin.IExecutionSpec; validate(): boolean; @@ -119,6 +127,7 @@ export interface LaunchWorkflowFormState { advancedOptionsRef: React.RefObject; formInputsRef: React.RefObject; roleInputRef: React.RefObject; + interruptibleInputRef: React.RefObject; state: State; service: Interpreter; workflowSourceSelectorState: WorkflowSourceSelectorState; @@ -127,6 +136,7 @@ export interface LaunchWorkflowFormState { export interface LaunchTaskFormState { formInputsRef: React.RefObject; roleInputRef: React.RefObject; + interruptibleInputRef: React.RefObject; state: State; service: Interpreter; taskSourceSelectorState: TaskSourceSelectorState; diff --git a/src/components/Launch/LaunchForm/useLaunchTaskFormState.ts b/src/components/Launch/LaunchForm/useLaunchTaskFormState.ts index d3badd3db..383baa78c 100644 --- a/src/components/Launch/LaunchForm/useLaunchTaskFormState.ts +++ b/src/components/Launch/LaunchForm/useLaunchTaskFormState.ts @@ -20,6 +20,7 @@ import { import { validate as baseValidate } from './services'; import { LaunchFormInputsRef, + LaunchInterruptibleInputRef, LaunchRoleInputRef, LaunchTaskFormProps, LaunchTaskFormState, @@ -92,6 +93,7 @@ async function loadInputs( async function validate( formInputsRef: RefObject, roleInputRef: RefObject, + _interruptibleInputRef: RefObject, ) { if (roleInputRef.current === null) { throw new Error('Unexpected empty role input ref'); @@ -107,6 +109,7 @@ async function submit( { createWorkflowExecution }: APIContextValue, formInputsRef: RefObject, roleInputRef: RefObject, + interruptibleInputRef: RefObject, { referenceExecutionId, taskVersion }: TaskLaunchContext, ) { if (!taskVersion) { @@ -121,6 +124,7 @@ async function submit( const { securityContext } = roleInputRef.current?.getValue(); const literals = formInputsRef.current.getValues(); + const interruptible = interruptibleInputRef.current?.getValue(); const launchPlanId = taskVersion; const { domain, project } = taskVersion; @@ -131,6 +135,7 @@ async function submit( project, referenceExecutionId, inputs: { literals }, + interruptible, }); const newExecutionId = response.id as WorkflowExecutionIdentifier; if (!newExecutionId) { @@ -144,12 +149,13 @@ function getServices( apiContext: APIContextValue, formInputsRef: RefObject, roleInputRef: RefObject, + interruptibleInputRef: RefObject, ) { return { loadTaskVersions: partial(loadTaskVersions, apiContext), loadInputs: partial(loadInputs, apiContext), - submit: partial(submit, apiContext, formInputsRef, roleInputRef), - validate: partial(validate, formInputsRef, roleInputRef), + submit: partial(submit, apiContext, formInputsRef, roleInputRef, interruptibleInputRef), + validate: partial(validate, formInputsRef, roleInputRef, interruptibleInputRef), }; } @@ -167,15 +173,17 @@ export function useLaunchTaskFormState({ authRole: defaultAuthRole, taskId: preferredTaskId, values: defaultInputValues, + interruptible, } = initialParameters; const apiContext = useAPIContext(); const formInputsRef = useRef(null); const roleInputRef = useRef(null); + const interruptibleInputRef = useRef(null); const services = useMemo( - () => getServices(apiContext, formInputsRef, roleInputRef), - [apiContext, formInputsRef, roleInputRef], + () => getServices(apiContext, formInputsRef, roleInputRef, interruptibleInputRef), + [apiContext, formInputsRef, roleInputRef, interruptibleInputRef], ); const [state, sendEvent, service] = useMachine< @@ -191,6 +199,7 @@ export function useLaunchTaskFormState({ preferredTaskId, referenceExecutionId, sourceId, + interruptible, }, }); @@ -239,6 +248,7 @@ export function useLaunchTaskFormState({ return { formInputsRef, roleInputRef, + interruptibleInputRef: interruptibleInputRef, state, service, taskSourceSelectorState, diff --git a/src/components/Launch/LaunchForm/useLaunchWorkflowFormState.ts b/src/components/Launch/LaunchForm/useLaunchWorkflowFormState.ts index cb3f6c968..ac2834e83 100644 --- a/src/components/Launch/LaunchForm/useLaunchWorkflowFormState.ts +++ b/src/components/Launch/LaunchForm/useLaunchWorkflowFormState.ts @@ -25,6 +25,7 @@ import { LaunchWorkflowFormState, ParsedInput, LaunchRoles, + LaunchInterruptibleInputRef, } from './types'; import { useWorkflowSourceSelectorState } from './useWorkflowSourceSelectorState'; import { getUnsupportedRequiredInputs } from './utils'; @@ -154,6 +155,7 @@ async function submit( formInputsRef: RefObject, roleInputRef: RefObject, advancedOptionsRef: RefObject, + interruptibleInputRef: RefObject, { launchPlan, referenceExecutionId, workflowVersion }: WorkflowLaunchContext, ) { if (!launchPlan) { @@ -170,6 +172,7 @@ async function submit( const literals = formInputsRef.current.getValues(); const { disableAll, labels, annotations, maxParallelism, rawOutputDataConfig } = advancedOptionsRef.current?.getValues() || {}; + const interruptible = interruptibleInputRef.current?.getValue(); const launchPlanId = launchPlan.id; const { domain, project } = workflowVersion; @@ -185,6 +188,7 @@ async function submit( project, referenceExecutionId, inputs: { literals }, + interruptible, }); const newExecutionId = response.id as WorkflowExecutionIdentifier; if (!newExecutionId) { @@ -197,7 +201,8 @@ async function submit( async function validate( formInputsRef: RefObject, roleInputRef: RefObject, - advancedOptionsRef: RefObject, + _advancedOptionsRef: RefObject, + _interruptibleInputRef: RefObject, ) { if (roleInputRef.current === null) { throw new Error('Unexpected empty role input ref'); @@ -214,13 +219,31 @@ function getServices( formInputsRef: RefObject, roleInputRef: RefObject, advancedOptionsRef: RefObject, + interruptibleInputRef: RefObject, ) { return { loadWorkflowVersions: partial(loadWorkflowVersions, apiContext), loadLaunchPlans: partial(loadLaunchPlans, apiContext), loadInputs: partial(loadInputs, apiContext), - submit: partial(submit, apiContext, formInputsRef, roleInputRef, advancedOptionsRef), - validate: partial(validate, formInputsRef, roleInputRef, advancedOptionsRef), + // lodash's partial function only has typings for one function and 4 arguments, the added interruptibleInputRef + // caused a typing issue (although the underlying implemententation would've worked fine). Thus, the call + // to partial has been replaced with an arrow function passing all other values, leading to similar results. + submit: (launchContext: WorkflowLaunchContext) => + submit( + apiContext, + formInputsRef, + roleInputRef, + advancedOptionsRef, + interruptibleInputRef, + launchContext, + ), + validate: partial( + validate, + formInputsRef, + roleInputRef, + advancedOptionsRef, + interruptibleInputRef, + ), }; } @@ -245,16 +268,25 @@ export function useLaunchWorkflowFormState({ labels, annotations, securityContext, + interruptible, } = initialParameters; const apiContext = useAPIContext(); const formInputsRef = useRef(null); const roleInputRef = useRef(null); const advancedOptionsRef = useRef(null); + const interruptibleInputRef = useRef(null); const services = useMemo( - () => getServices(apiContext, formInputsRef, roleInputRef, advancedOptionsRef), - [apiContext, formInputsRef, roleInputRef, advancedOptionsRef], + () => + getServices( + apiContext, + formInputsRef, + roleInputRef, + advancedOptionsRef, + interruptibleInputRef, + ), + [apiContext, formInputsRef, roleInputRef, advancedOptionsRef, interruptibleInputRef], ); const [state, sendEvent, service] = useMachine< @@ -277,6 +309,7 @@ export function useLaunchWorkflowFormState({ rawOutputDataConfig, labels, annotations, + interruptible, }, }); @@ -387,6 +420,7 @@ export function useLaunchWorkflowFormState({ advancedOptionsRef, formInputsRef, roleInputRef, + interruptibleInputRef: interruptibleInputRef, state, service, workflowSourceSelectorState, diff --git a/src/models/Execution/api.ts b/src/models/Execution/api.ts index 19ff2e9c8..07ced0629 100644 --- a/src/models/Execution/api.ts +++ b/src/models/Execution/api.ts @@ -1,4 +1,4 @@ -import { Admin, Core } from 'flyteidl'; +import { Admin, Core, Protobuf } from 'flyteidl'; import { getAdminEntity, postAdminEntity } from 'models/AdminEntity/AdminEntity'; import { defaultListExecutionChildrenConfig, @@ -88,6 +88,7 @@ export interface CreateWorkflowExecutionArguments { launchPlanId: Identifier; project: string; referenceExecutionId?: WorkflowExecutionIdentifier; + interruptible?: Protobuf.IBoolValue | null; } /** Submits a request to create a new `WorkflowExecution` using the provided @@ -106,6 +107,7 @@ export const createWorkflowExecution = ( launchPlanId: launchPlan, project, referenceExecutionId: referenceExecution, + interruptible, }: CreateWorkflowExecutionArguments, config?: RequestConfig, ) => { @@ -136,6 +138,10 @@ export const createWorkflowExecution = ( spec.rawOutputDataConfig = rawOutputDataConfig; } + if (interruptible !== undefined && interruptible !== null) { + spec.interruptible = interruptible; + } + return postAdminEntity( { data: { diff --git a/yarn.lock b/yarn.lock index 0f48caca1..7a35b0b22 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1731,10 +1731,10 @@ minimatch "^3.0.4" strip-json-comments "^3.1.1" -"@flyteorg/flyteidl@0.24.11": - version "0.24.11" - resolved "https://registry.yarnpkg.com/@flyteorg/flyteidl/-/flyteidl-0.24.11.tgz#a1a6e17a0cd9303cf335fae50162d66c6a0ed980" - integrity sha512-PBRei4Yu5wHp8Fa683EW2xYllyApNcZOZwEh7f1tf68tnhrrvV455HBKez2U1qcqFg179qTy7VNX1zvjGCMmuA== +"@flyteorg/flyteidl@1.1.0": + version "1.1.0" + resolved "https://registry.yarnpkg.com/@flyteorg/flyteidl/-/flyteidl-1.1.0.tgz#9fcf06d878616439939e932c5a35357d3d7eea04" + integrity sha512-p07Vd6rxmASKJvfF56ArVyuZLjxTJPnAQsTs3rDJL74gd9Ixs4cEDa/A4+iMYSNl42iTVPvAkM4movXDD1Hmgg== "@gar/promisify@^1.0.1": version "1.1.3"