-
Notifications
You must be signed in to change notification settings - Fork 59
Compiler Enum Support and updated flyteidl #minor #276
Conversation
Signed-off-by: Ketan Umare <[email protected]>
Signed-off-by: Ketan Umare <[email protected]>
Signed-off-by: Ketan Umare <[email protected]>
Signed-off-by: Ketan Umare <[email protected]>
return true | ||
} | ||
} | ||
// If t is an enum, it can be created from a string as Enums as just constrained String aliases |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this a runtime error then if the string isn't a valid enum value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but our entry points will not allow bad values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@katrogan thats actually a good point. Can we restrict this in flyteadmin? Where we construct the launchplan?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't the launch plan have to reference the enum type to be valid?
@@ -33,6 +33,20 @@ func (t trivialChecker) CastsFrom(upstreamType *flyte.LiteralType) bool { | |||
if isVoid(upstreamType) { | |||
return true | |||
} | |||
|
|||
// If upstream is an enum, it can be consumed as a string downstream | |||
if upstreamType.GetEnumType() != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also add a check to the static binding to fail if the string isn't in the list...
TL;DR
Support for ENUM types in flytecompiler and flytepropeller update to support it
Type
Are all requirements met?
Complete description
https://docs.google.com/document/d/1rf0hMKbPebJ7jGYpsTDAbJ4ka58KIKD7EgWAEBIH06Q/edit#
Tracking Issue
flyteorg/flyte#590
Follow-up issue
NA