Fixes initial population of values for string Collections in the launch form. #48
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mrmcduff
approved these changes
Feb 20, 2020
BobNisco
approved these changes
Feb 20, 2020
| /** Converts a value within a collection to the appropriate string | ||
| * representation. Some values require additional quotes. | ||
| */ | ||
| export function collectionChildToString(type: InputType, value: any) { |
There was a problem hiding this comment.
Could be worthwhile to add an explicit return type of string here so that future code changes do not accidentally return unexpected types
There was a problem hiding this comment.
That's a good idea. Code consuming this expects a string, so changes here would still result in a type error. But I can't guarantee that any new usage of this function would also have such a strict type expectation.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
flyteorg/flyte#181
When converting
Literalvalues toInputValues for use in the form, theCollectiontype is output as a JSON representation of an array. The existing implementation was blindly wrapping[]around values and comma-separating them. For Dates and strings, the resulting array string is not valid JSON and will trigger validation errors.It turned out to be easier to allow
JSON.stringifyto do this work for us, with the exception of integers. Since we use a special type for integers,stringifywould result in a quoted string value instead of a bare integer value.