fix: union value handling in launch form (#812)

* fix: updating collections of unions Signed-off-by: Carina Ursu <[email protected]> * fix: pass the correct initial value Signed-off-by: Carina Ursu <[email protected]> * fix: handle union non standard inputs Signed-off-by: Carina Ursu <[email protected]> * chore: pkg Signed-off-by: Carina Ursu <[email protected]> --------- Signed-off-by: Carina Ursu <[email protected]>
flyteorg · Aug 8, 2023 · 662429c · 662429c
1 parent 8013196
commit 662429c
Show file tree

Hide file tree

Showing 10 changed files with 128 additions and 52 deletions.
diff --git a/packages/console/package.json b/packages/console/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@flyteorg/console",
-  "version": "0.0.47",
+  "version": "0.0.48",
   "description": "Flyteconsole main app module",
   "main": "./dist/index.js",
   "module": "./lib/index.js",

diff --git a/packages/console/src/components/Launch/LaunchForm/LaunchFormComponents/CollectionList.tsx b/packages/console/src/components/Launch/LaunchForm/LaunchFormComponents/CollectionList.tsx
@@ -38,7 +38,7 @@ export interface CollectionListProps {
 /** Handles rendering of the input component for a Collection of SimpleType values */
 export const CollectionList: FC<CollectionListProps> = props => {
   const { defaultValue, updateCollection, inputProps } = props;
-  const { value, typeDefinition } = inputProps;
+  const { value, typeDefinition, initialValue } = inputProps;
 
   const collectionInputs = value as InputValue[];
 
@@ -75,6 +75,7 @@ export const CollectionList: FC<CollectionListProps> = props => {
           {getComponentForInput(
             {
               ...inputProps,
+              initialValue: initialValue?.[index],
               value: inputValue,
               label: `${inputProps.description}[${index}]`,
               onChange: val => onChange(val, index),

diff --git a/packages/console/src/components/Launch/LaunchForm/LaunchFormComponents/UnionInput.tsx b/packages/console/src/components/Launch/LaunchForm/LaunchFormComponents/UnionInput.tsx
@@ -6,7 +6,7 @@ import {
   UnionValue,
   InputValue,
 } from '../types';
-import { formatType, getInputDefintionForLiteralType } from '../utils';
+import { formatType } from '../utils';
 
 import { getHelperForInput } from '../inputHelpers/getHelperForInput';
 import {
@@ -48,19 +48,23 @@ const getInitialInputValue = (props: InputProps): UnionValue => {
 
   if (props.hasCollectionParent && Array.isArray(props.initialValue)) {
     const collectionValues = props.initialValue.map(literal => {
-      const unionValue = literal.scalar.union;
+      const unionValue = unionHelper.fromLiteral(
+        literal,
+        props.typeDefinition,
+      ) as UnionValue;
 
-      return {
-        value: unionValue.value,
-        typeDefinition: getInputDefintionForLiteralType(unionValue.type as any),
-      };
+      return unionValue;
     });
 
-    const subtype = collectionValues?.[0].typeDefinition;
+    const subtype =
+      collectionValues?.[0]?.typeDefinition ||
+      props.typeDefinition.listOfSubTypes?.[0];
     const value = collectionHelper.fromLiteral(
       {
         collection: {
-          literals: collectionValues.map(v => v.value),
+          literals: props.initialValue?.map(l =>
+            l?.scalar?.union ? l?.scalar?.union?.value : l,
+          ),
         },
       } as any,
       {

diff --git a/packages/console/src/components/Launch/LaunchForm/constants.ts b/packages/console/src/components/Launch/LaunchForm/constants.ts
@@ -52,6 +52,24 @@ export const simpleTypeToInputType: { [k in SimpleType]: InputType } = {
   [SimpleType.STRUCT]: InputType.Struct,
 };
 
+/** Maps nested `SimpleType`s to our flattened `InputType` enum. */
+export const primitiveToInputType: {
+  [k in
+    | 'integer'
+    | 'floatValue'
+    | 'stringValue'
+    | 'boolean'
+    | 'datetime'
+    | 'duration']: InputType;
+} = {
+  integer: InputType.Integer,
+  boolean: InputType.Boolean,
+  datetime: InputType.Datetime,
+  duration: InputType.Duration,
+  floatValue: InputType.Float,
+  stringValue: InputType.String,
+};
+
 export const defaultBlobValue: BlobValue = {
   uri: '',
   dimensionality: BlobDimensionality.SINGLE,

diff --git a/packages/console/src/components/Launch/LaunchForm/inputHelpers/union.ts b/packages/console/src/components/Launch/LaunchForm/inputHelpers/union.ts
@@ -1,11 +1,55 @@
 import { Core } from '@flyteorg/flyteidl-types';
 import { isObject } from 'lodash';
+import { Literal } from 'models';
 import { InputTypeDefinition, InputValue, UnionValue } from '../types';
 import { getHelperForInput } from './getHelperForInput';
 import { ConverterInput, InputHelper, InputValidatorParams } from './types';
 import t from '../../../common/strings';
 import { getInputDefintionForLiteralType } from '../utils';
 
+export function isScalarType(value: any): value is Literal {
+  return isObject(value) && !value.type && !!(value as any).scalar;
+}
+
+function getUnionValueFromUnknownLiteral(
+  literal: Literal,
+  inputTypeDefinition: InputTypeDefinition,
+) {
+  let currentLiteral = literal;
+  if (isScalarType(currentLiteral)) {
+    currentLiteral = currentLiteral?.scalar as any as Literal;
+  }
+
+  if (currentLiteral) {
+    const def = getInputDefintionForLiteralType(currentLiteral as any);
+    const inputDefinition =
+      inputTypeDefinition?.listOfSubTypes?.find(s => s.type === def.type) ||
+      def;
+    const helper = getHelperForInput(inputDefinition.type);
+    const value = helper.fromLiteral(literal, inputDefinition);
+    const unionValue = {
+      value,
+      typeDefinition: inputDefinition,
+    } as UnionValue;
+    return unionValue;
+  }
+
+  const { listOfSubTypes } = inputTypeDefinition;
+  const values = listOfSubTypes?.map(subtype => {
+    const helper = getHelperForInput(subtype.type);
+    try {
+      const value = helper.fromLiteral(literal, subtype);
+
+      return { value, typeDefinition: subtype } as UnionValue;
+    } catch {
+      // no-op
+    }
+    return { value: undefined, typeDefinition: subtype };
+  });
+
+  return values?.filter(v => v.value)?.[0] || values?.[0];
+}
+
 function fromLiteral(
   literal: Core.ILiteral,
   inputTypeDefinition: InputTypeDefinition,
@@ -15,39 +59,26 @@ function fromLiteral(
     throw new Error(t('missingUnionListOfSubType'));
   }
 
-  const localLiteral = literal?.scalar?.union || (literal as any);
-  const inputDef =
-    (localLiteral?.type &&
-      getInputDefintionForLiteralType(localLiteral.type as any)) ||
-    localLiteral?.typeDefinition;
+  const localLiteral = literal?.scalar?.union;
+  if (localLiteral?.type) {
+    const inputDef = getInputDefintionForLiteralType(localLiteral.type as any);
 
-  // Unpack nested variant of union data value
-  const literalValue = localLiteral?.value || literal;
+    // Unpack nested variant of union data value
+    const literalValue = localLiteral?.value || literal;
 
-  if (inputDef) {
     const helper = getHelperForInput(inputDef.type);
     try {
       const value = helper.fromLiteral(literalValue, inputDef);
       return { value, typeDefinition: inputDef } as UnionValue;
     } catch {
-      // no-op, continue executing
+      return { value: undefined, typeDefinition: inputDef };
     }
+  } else {
+    return getUnionValueFromUnknownLiteral(
+      literal as Literal,
+      inputTypeDefinition,
+    ) as any;
   }
-
-  // else try to guess the type
-  const values = listOfSubTypes.map(subtype => {
-    const helper = getHelperForInput(subtype.type);
-    try {
-      const value = helper.fromLiteral(literalValue, subtype);
-
-      return { value, typeDefinition: subtype } as UnionValue;
-    } catch {
-      // no-op
-    }
-    return { value: undefined, typeDefinition: subtype };
-  });
-
-  return values?.filter(v => v.value)?.[0] || values?.[0];
 }
 
 function toLiteral({
@@ -105,13 +136,10 @@ export const unionHelper: InputHelper = {
     const { listOfSubTypes } = typeDefinition;
     const selectedSubType = listOfSubTypes?.[0];
     const subtypeHelper = getHelperForInput(selectedSubType?.type!);
+
     return {
-      scalar: {
-        union: {
-          value: subtypeHelper.typeDefinitionToDefaultValue(selectedSubType!),
-          type: typeDefinition.literalType,
-        },
-      },
+      value: subtypeHelper.typeDefinitionToDefaultValue(selectedSubType!),
+      typeDefinition: selectedSubType,
     };
   },
 };
diff --git a/packages/console/src/components/Launch/LaunchForm/types.ts b/packages/console/src/components/Launch/LaunchForm/types.ts
@@ -208,7 +208,7 @@ export enum InputType {
 }
 
 export interface InputTypeDefinition {
-  literalType: LiteralType;
+  literalType: Partial<LiteralType>;
   type: InputType;
   subtype?: InputTypeDefinition;
   listOfSubTypes?: InputTypeDefinition[];

diff --git a/packages/console/src/components/Launch/LaunchForm/utils.ts b/packages/console/src/components/Launch/LaunchForm/utils.ts
@@ -7,7 +7,11 @@ import { Task } from 'models/Task/types';
 import { Workflow } from 'models/Workflow/types';
 import moment from 'moment';
 import { LiteralValueMap } from 'components/Launch/LaunchForm/types';
-import { simpleTypeToInputType, typeLabels } from './constants';
+import {
+  primitiveToInputType,
+  simpleTypeToInputType,
+  typeLabels,
+} from './constants';
 import { inputToLiteral } from './inputHelpers/inputHelpers';
 import { typeIsSupported } from './inputHelpers/utils';
 import { LaunchState } from './launchMachine';
@@ -156,14 +160,16 @@ export function convertFormInputsToLiterals(
  * a type annotation and converting input values.
  */
 export function getInputDefintionForLiteralType(
-  literalType: LiteralType,
+  literalType: Partial<LiteralType>,
 ): InputTypeDefinition {
   const result: InputTypeDefinition = {
     literalType,
     type: InputType.Unknown,
   };
 
-  if (literalType.blob) {
+  if (literalType.noneType) {
+    result.type = InputType.None;
+  } else if (literalType.blob) {
     result.type = InputType.Blob;
   } else if (literalType.collectionType) {
     result.type = InputType.Collection;
@@ -191,6 +197,13 @@ export function getInputDefintionForLiteralType(
     literalType.structure?.tag === 'none'
   ) {
     result.type = simpleTypeToInputType[literalType.simple];
+  } else if ((literalType as any).primitive) {
+    const primitive = (literalType as any).primitive;
+
+    result.type =
+      primitiveToInputType[
+        primitive.value || Object.keys(primitive || {})?.[0]
+      ];
   }
   return result;
 }

diff --git a/packages/console/src/models/Common/types.ts b/packages/console/src/models/Common/types.ts
@@ -9,6 +9,8 @@ import { Admin, Core, Protobuf } from '@flyteorg/flyteidl-types';
 /* eslint-disable @typescript-eslint/no-redeclare */
 export type SimpleType = Core.SimpleType;
 export const SimpleType = Core.SimpleType;
+export type NoneType = Core.Void;
+export const NoneType = Core.Void;
 export type EnumType = Core.EnumType;
 export const EnumType = Core.EnumType;
 export type FixedRateUnit = Admin.FixedRateUnit;
@@ -23,6 +25,12 @@ export type MessageFormat = Core.TaskLog.MessageFormat;
 export const MessageFormat = Core.TaskLog.MessageFormat;
 export type StructuredDatasetType = Core.StructuredDatasetType;
 export const StructuredDatasetType = Core.StructuredDatasetType;
+export type UnionType = Core.UnionType;
+export const UnionType = Core.UnionType;
+export type AnnotationType = Core.TypeAnnotation;
+export const AnnotationType = Core.TypeAnnotation;
+export type StructureType = Core.TypeStructure;
+export const StructureType = Core.TypeStructure;
 export type DatasetColumn = Core.StructuredDatasetType.DatasetColumn;
 export const DatasetColumn = Core.StructuredDatasetType.DatasetColumn;
 /* eslint-enable @typescript-eslint/no-redeclare */
@@ -152,14 +160,18 @@ export interface TypedInterface extends Core.ITypedInterface {
 }
 
 export interface LiteralType extends Core.ILiteralType {
-  blob?: BlobType;
+  simple?: SimpleType;
+  schema?: SchemaType;
   collectionType?: LiteralType;
   mapValueType?: LiteralType;
-  metadata?: ProtobufStruct;
-  schema?: SchemaType;
-  simple?: SimpleType;
-  structuredDatasetType?: StructuredDatasetType;
+  blob?: BlobType;
   enumType?: EnumType;
+  structuredDatasetType?: StructuredDatasetType;
+  unionType?: UnionType;
+  metadata?: ProtobufStruct;
+  annotationType?: AnnotationType;
+  structure: StructureType;
+  noneType?: NoneType;
 }
 
 export interface Variable extends Core.IVariable {

diff --git a/website/package.json b/website/package.json
@@ -37,7 +37,7 @@
   },
   "dependencies": {
     "@flyteorg/common": "^0.0.4",
-    "@flyteorg/console": "^0.0.47",
+    "@flyteorg/console": "^0.0.48",
     "long": "^4.0.0",
     "protobufjs": "~6.11.3",
     "react-ga4": "^1.4.1",

diff --git a/yarn.lock b/yarn.lock
@@ -2032,7 +2032,7 @@ __metadata:
   resolution: "@flyteconsole/client-app@workspace:website"
   dependencies:
     "@flyteorg/common": ^0.0.4
-    "@flyteorg/console": ^0.0.47
+    "@flyteorg/console": ^0.0.48
     "@types/long": ^3.0.32
     long: ^4.0.0
     protobufjs: ~6.11.3
@@ -2071,7 +2071,7 @@ __metadata:
   languageName: unknown
   linkType: soft
 
-"@flyteorg/console@^0.0.47, @flyteorg/console@workspace:packages/console":
+"@flyteorg/console@^0.0.48, @flyteorg/console@workspace:packages/console":
   version: 0.0.0-use.local
   resolution: "@flyteorg/console@workspace:packages/console"
   dependencies: