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

fix: show info message for un-launchable workflows #84

Merged
merged 5 commits into from
Jul 15, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 20 additions & 11 deletions src/components/Launch/LaunchWorkflowForm/LaunchWorkflowForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@ import {
workflowSortFields
} from 'models';
import * as React from 'react';
import { formStrings } from './constants';
import { formStrings, unsupportedRequiredInputsString } from './constants';
import { InputValueCacheContext } from './inputValueCache';
import { LaunchWorkflowFormInputs } from './LaunchWorkflowFormInputs';
import { SearchableSelector } from './SearchableSelector';
import { useStyles } from './styles';
import { LaunchWorkflowFormProps } from './types';
import { UnsupportedRequiredInputsError } from './UnsupportedRequiredInputsError';
import { useLaunchWorkflowFormState } from './useLaunchWorkflowFormState';
import { workflowsToSearchableSelectorOptions } from './utils';

Expand Down Expand Up @@ -67,6 +68,11 @@ export const LaunchWorkflowForm: React.FC<LaunchWorkflowFormProps> = props => {
state.onSubmit();
};

const preventSubmit =
submissionState.loading ||
!state.inputLoadingState.hasLoaded ||
state.unsupportedRequiredInputs.length > 0;

return (
<InputValueCacheContext.Provider value={state.inputValueCache}>
<DialogTitle disableTypography={true} className={styles.header}>
Expand Down Expand Up @@ -114,12 +120,18 @@ export const LaunchWorkflowForm: React.FC<LaunchWorkflowFormProps> = props => {
{...state.inputLoadingState}
>
<section title={formStrings.inputs}>
<LaunchWorkflowFormInputs
key={state.formKey}
inputs={state.inputs}
ref={state.formInputsRef}
showErrors={state.showErrors}
/>
{state.unsupportedRequiredInputs.length > 0 ? (
<UnsupportedRequiredInputsError
inputs={state.unsupportedRequiredInputs}
/>
) : (
<LaunchWorkflowFormInputs
key={state.formKey}
inputs={state.inputs}
ref={state.formInputsRef}
showErrors={state.showErrors}
/>
)}
</section>
</WaitForData>
) : null}
Expand All @@ -143,10 +155,7 @@ export const LaunchWorkflowForm: React.FC<LaunchWorkflowFormProps> = props => {
</Button>
<Button
color="primary"
disabled={
submissionState.loading ||
!state.inputLoadingState.hasLoaded
}
disabled={preventSubmit}
id="launch-workflow-submit"
onClick={submit}
type="submit"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { makeStyles, Theme } from '@material-ui/core/styles';
import ErrorOutline from '@material-ui/icons/ErrorOutline';
import { NonIdealState } from 'components/common';
import { useCommonStyles } from 'components/common/styles';
import * as React from 'react';
import {
cannotLaunchWorkflowString,
requiredInputSuffix,
unsupportedRequiredInputsString
} from './constants';
import { ParsedInput } from './types';

const useStyles = makeStyles((theme: Theme) => ({
contentContainer: {
whiteSpace: 'pre-line',
textAlign: 'left'
},
errorContainer: {
marginBottom: theme.spacing(2)
}
}));

function formatLabel(label: string) {
return label.endsWith(requiredInputSuffix)
? label.substring(0, label.length - 1)
: label;
}

export interface UnsupportedRequiredInputsErrorProps {
inputs: ParsedInput[];
}
export const UnsupportedRequiredInputsError: React.FC<UnsupportedRequiredInputsErrorProps> = ({
inputs
}) => {
const styles = useStyles();
const commonStyles = useCommonStyles();
return (
<NonIdealState
className={styles.errorContainer}
icon={ErrorOutline}
size="medium"
title={cannotLaunchWorkflowString}
>
<div className={styles.contentContainer}>
<p>{unsupportedRequiredInputsString}</p>
<ul className={commonStyles.listUnstyled}>
{inputs.map(input => (
<li
key={input.name}
className={commonStyles.textMonospace}
>
{formatLabel(input.label)}
</li>
))}
</ul>
</div>
</NonIdealState>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
SimpleVariableKey
} from '../__mocks__/mockInputs';
import { LaunchWorkflowForm } from '../LaunchWorkflowForm';
import { binaryInputName, errorInputName } from '../test/constants';
import { useExecutionLaunchConfiguration } from '../useExecutionLaunchConfiguration';
import { getWorkflowInputs } from '../utils';

Expand Down Expand Up @@ -200,3 +201,10 @@ stories.add('Launched from Execution', () => {
} as Execution;
return renderForm({ mocks, execution });
});
stories.add('Unsupported Required Values', () => {
const mocks = generateMocks(mockSimpleVariables);
const parameters = mocks.mockLaunchPlan.closure!.expectedInputs.parameters;
parameters[binaryInputName].required = true;
parameters[errorInputName].required = true;
return renderForm({ mocks });
});
4 changes: 4 additions & 0 deletions src/components/Launch/LaunchWorkflowForm/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,7 @@ export const simpleTypeToInputType: { [k in SimpleType]: InputType } = {
[SimpleType.STRING]: InputType.String,
[SimpleType.STRUCT]: InputType.Struct
};

export const requiredInputSuffix = '*';
export const cannotLaunchWorkflowString = 'Workflow cannot be launched';
export const unsupportedRequiredInputsString = `This Workflow version contains one or more required inputs which are not supported by Flyte Console and do not have default values specified in the Workflow definition or the selected Launch Plan.\n\nYou can launch this Workflow version with the Flyte CLI or by selecting a Launch Plan which provides values for the unsupported inputs.\n\nThe required inputs are :`;
5 changes: 4 additions & 1 deletion src/components/Launch/LaunchWorkflowForm/getInputs.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { sortedObjectEntries } from 'common/utils';
import { LaunchPlan, Workflow } from 'models';
import { requiredInputSuffix } from './constants';
import { LiteralValueMap, ParsedInput } from './types';
import {
createInputCacheKey,
Expand Down Expand Up @@ -36,7 +37,9 @@ export function getInputs(
parameter.var.type
);
const typeLabel = formatLabelWithType(name, typeDefinition);
const label = required ? `${typeLabel}*` : typeLabel;
const label = required
? `${typeLabel}${requiredInputSuffix}`
: typeLabel;
const inputKey = createInputCacheKey(name, typeDefinition);
const defaultVaue =
parameter.default != null ? parameter.default : undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import { collectionChildToString } from '../utils';
import {
literalTestCases,
literalToInputTestCases,
supportedPrimitives,
unsupportedTypes,
validityTestCases
} from './testCases';

Expand Down Expand Up @@ -58,14 +60,7 @@ describe('literalToInputValue', () => {
})
);

[
InputType.Collection,
InputType.Datetime,
InputType.Duration,
InputType.Float,
InputType.Integer,
InputType.String
].map(type =>
supportedPrimitives.map(type =>
it(`should convert None value for ${type} to undefined`, () => {
expect(
literalToInputValue({ type }, literalNone())
Expand Down Expand Up @@ -176,16 +171,7 @@ describe('inputToLiteral', () => {
});

describe('Unsupported Types', () => {
[
InputType.Binary,
InputType.Blob,
InputType.Error,
InputType.Map,
InputType.None,
InputType.Schema,
InputType.Struct,
InputType.Unknown
].map(type =>
unsupportedTypes.map(type =>
it(`should return empty value for type: ${type}`, () => {
expect(
inputToLiteral(makeSimpleInput(type, '')).scalar
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,24 @@ type PrimitiveTestParams = [InputType, any, Core.IPrimitive];

const validDateString = '2019-01-10T00:00:00.000Z'; // Dec 1, 2019

export const supportedPrimitives = [
InputType.Boolean,
InputType.Datetime,
InputType.Duration,
InputType.Float,
InputType.Integer
];

export const unsupportedTypes = [
InputType.Binary,
InputType.Blob,
InputType.Error,
InputType.Map,
InputType.None,
InputType.Schema,
InputType.Struct
];

export const validityTestCases = {
boolean: {
invalid: ['randomString', {}, new Date()],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { InputType, InputTypeDefinition } from '../../types';
import { typeIsSupported } from '../utils';
import { supportedPrimitives, unsupportedTypes } from './testCases';

type TypeIsSupportedTestCase = [string, InputTypeDefinition, boolean];
describe('Launch/inputHelpers/utils', () => {
describe('typeIsSupported', () => {
const cases: TypeIsSupportedTestCase[] = [
...supportedPrimitives.map<TypeIsSupportedTestCase>(type => [
`supports type ${type}`,
{ type },
true
]),
...supportedPrimitives.map<TypeIsSupportedTestCase>(type => [
`supports 1-dimension collection of type ${type}`,
{ type: InputType.Collection, subtype: { type } },
true
]),
...supportedPrimitives.map<TypeIsSupportedTestCase>(type => [
`supports 2-dimension collection of type: ${type}`,
{
type: InputType.Collection,
subtype: { type: InputType.Collection, subtype: { type } }
},
true
]),
...unsupportedTypes.map<TypeIsSupportedTestCase>(type => [
`does NOT support type ${type}`,
{ type },
false
]),
...unsupportedTypes.map<TypeIsSupportedTestCase>(type => [
`does NOT support 1-dimension collection of type ${type}`,
{ type: InputType.Collection, subtype: { type } },
false
]),
...unsupportedTypes.map<TypeIsSupportedTestCase>(type => [
`does NOT support 2-dimension collection of type: ${type}`,
{
type: InputType.Collection,
subtype: { type: InputType.Collection, subtype: { type } }
},
false
])
];

cases.forEach(([description, value, expected]) =>
it(description, () => expect(typeIsSupported(value)).toBe(expected))
);
});
});
35 changes: 34 additions & 1 deletion src/components/Launch/LaunchWorkflowForm/inputHelpers/utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Core } from 'flyteidl';
import { get } from 'lodash';
import { InputType } from '../types';
import { InputType, InputTypeDefinition } from '../types';

/** Performs a deep get of `path` on the given `Core.ILiteral`. Will throw
* if the given property doesn't exist.
Expand All @@ -25,3 +25,36 @@ export function collectionChildToString(type: InputType, value: any) {
}
return type === InputType.Integer ? `${value}` : JSON.stringify(value);
}

/** Determines if a given input type, including all levels of nested types, is
* supported for use in the Launch form.
*/
export function typeIsSupported(typeDefinition: InputTypeDefinition): boolean {
const { type, subtype } = typeDefinition;
switch (type) {
case InputType.Map:
case InputType.None:
case InputType.Schema:
case InputType.Unknown:
return false;
case InputType.Boolean:
case InputType.Datetime:
case InputType.Duration:
case InputType.Float:
case InputType.Integer:
case InputType.String:
return true;
case InputType.Collection: {
if (!subtype) {
console.error(
'Unexpected missing subtype for collection input',
typeDefinition
);
return false;
}
return typeIsSupported(subtype);
}
default:
schottra marked this conversation as resolved.
Show resolved Hide resolved
return false;
}
}
1 change: 0 additions & 1 deletion src/components/Launch/LaunchWorkflowForm/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ export const useStyles = makeStyles((theme: Theme) => ({
width: '100%'
},
inputsSection: {
minHeight: theme.spacing(59),
padding: theme.spacing(2)
},
inputLabel: {
Expand Down
Loading