Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add interruptible override to launch forms #chore #417

Merged
merged 9 commits into from
May 3, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -39,6 +40,7 @@ export const ExecutionMetadataExtra: React.FC<{
maxParallelism,
rawOutputDataConfig,
securityContext,
interruptible,
} = execution.spec;

const [launchPlanSpec, setLaunchPlanSpec] = React.useState<Partial<LaunchPlanSpec>>({});
Expand Down Expand Up @@ -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 (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ function useRelaunchWorkflowFormState({ execution }: RelaunchExecutionFormProps)
annotations,
authRole,
securityContext,
interruptible,
},
} = execution;

Expand All @@ -59,6 +60,7 @@ function useRelaunchWorkflowFormState({ execution }: RelaunchExecutionFormProps)
annotations,
authRole,
securityContext,
interruptible,
};
},
},
Expand All @@ -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);
Expand All @@ -85,7 +87,7 @@ function useRelaunchTaskFormState({ execution }: RelaunchExecutionFormProps) {
},
apiContext,
);
return { authRole, values, taskId };
return { authRole, values, taskId, interruptible };
},
},
execution,
Expand Down
1 change: 1 addition & 0 deletions src/components/Executions/ExecutionDetails/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export enum ExecutionMetadataLabels {
rawOutputPrefix = 'Raw Output Prefix',
parallelism = 'Parallelism',
securityContextDefault = 'default',
interruptible = 'Interruptible override',
}

export const tabs = {
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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: {} })),
Expand Down Expand Up @@ -75,4 +77,24 @@ 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(execution.spec.metadata.systemMetadata?.executionCluster).toBeDefined();
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);
});
});
99 changes: 99 additions & 0 deletions src/components/Launch/LaunchForm/LaunchInterruptibleInput.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { 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';

const isValueValid = (value: any) => {
return value !== undefined && value !== null;
};

export interface LaunchInterruptibleInputProps {
MorpheusXAUT marked this conversation as resolved.
Show resolved Hide resolved
initialValue?: Protobuf.IBoolValue | null;
}

export const LaunchInterruptibleInputImpl: React.ForwardRefRenderFunction<
LaunchInterruptibleInputRef,
LaunchInterruptibleInputProps
> = (props, ref) => {
const [interruptible, setInterruptible] = React.useState(false);
const [indeterminate, setIndeterminate] = React.useState(true);
MorpheusXAUT marked this conversation as resolved.
Show resolved Hide resolved

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 getInterruptibleLabel = (): string => {
if (indeterminate) {
return `${formStrings.interruptible} (no override)`;
} else if (interruptible) {
return `${formStrings.interruptible} (enabled)`;
}
return `${formStrings.interruptible} (disabled)`;
};

return (
<section>
<header className={styles.sectionHeader}>
<Typography variant="h6">Override interruptible flag</Typography>
<Typography variant="body2">
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.
</Typography>
</header>
<section title={formStrings.interruptible}>
<FormControlLabel
control={
<Checkbox
checked={interruptible}
indeterminate={indeterminate}
onChange={handleInputChange}
/>
}
label={getInterruptibleLabel()}
/>
</section>
</section>
);
};

export const LaunchInterruptibleInput = React.forwardRef(LaunchInterruptibleInputImpl);
15 changes: 13 additions & 2 deletions src/components/Launch/LaunchForm/LaunchTaskForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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<LaunchTaskFormProps> = (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;
Expand Down Expand Up @@ -63,6 +70,10 @@ export const LaunchTaskForm: React.FC<LaunchTaskFormProps> = (props) => {
/>
) : null}
<LaunchFormInputs key={formKey} ref={formInputsRef} state={baseState} variant="task" />
<LaunchInterruptibleInput
initialValue={state.context.interruptible}
ref={interruptibleInputRef}
/>
</DialogContent>
<LaunchFormActions state={baseState} service={baseService} onClose={props.onClose} />
</>
Expand Down
6 changes: 6 additions & 0 deletions src/components/Launch/LaunchForm/LaunchWorkflowForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@ 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<LaunchWorkflowFormProps> = (props) => {
const {
formInputsRef,
roleInputRef,
advancedOptionsRef,
interruptibleInputRef,
state,
service,
workflowSourceSelectorState,
Expand Down Expand Up @@ -105,6 +107,10 @@ export const LaunchWorkflowForm: React.FC<LaunchWorkflowFormProps> = (props) =>
/>
) : null}
<LaunchFormAdvancedInputs ref={advancedOptionsRef} state={state} />
<LaunchInterruptibleInput
initialValue={state.context.interruptible}
ref={interruptibleInputRef}
/>
</AccordionDetails>
</Accordion>
</DialogContent>
Expand Down
1 change: 1 addition & 0 deletions src/components/Launch/LaunchForm/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 } = {
Expand Down
4 changes: 3 additions & 1 deletion src/components/Launch/LaunchForm/launchMachine.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -82,13 +82,15 @@ 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 {
defaultAuthRole?: Admin.IAuthRole;
preferredTaskId?: Identifier;
taskVersion?: Identifier;
taskVersionOptions?: Task[];
interruptible?: Protobuf.IBoolValue | null;
}

export enum LaunchState {
Expand Down
30 changes: 29 additions & 1 deletion src/components/Launch/LaunchForm/test/LaunchTaskForm.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -594,5 +594,33 @@ 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();
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -754,5 +754,33 @@ 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();
});
});
});
});
Loading