Fix YamlToObject file splitting. #1298
Conversation
I wonder how we got away with splitting on *any* three dashes for so long.
porridge
requested review from
alenkacz,
gerred,
kensipe,
nfnt and
zen-dog
as code owners
| @@ -14,7 +14,7 @@ import ( | |||
| // If the type is not known in the scheme, it tries to parse it as Unstructured | |||
| // TODO(av) could we use something else than a global scheme here? Should we somehow inject it? | |||
| func YamlToObject(yaml string) (objs []runtime.Object, err error) { | |||
| sepYamlfiles := strings.Split(yaml, "---") | |||
| sepYamlfiles := strings.Split(yaml, "\n---\n") | |||
There was a problem hiding this comment.
I'm wondering why we have to split the file here. Isn't this something the YAML decoder should take care of? Looking at the documentation of the decoder it looks like this is a supported use case. The Decode functions unmarshals the "next object from the underlying stream". I didn't test this though, but maybe a for-loop that calls Decode would do the splitting?
Not yours, but also wondering why we're using a YAMLOrJSONDecoder when there's a YAMLDecoder?
There was a problem hiding this comment.
What Jan said ^^ There is a splitYamlDocument method that should handle multiple YAML documents. Funny enough, the separator it uses is defined as const yamlSeparator = "\n---" so only one \n.
There was a problem hiding this comment.
why we're using a YAMLOrJSONDecoder when there's a YAMLDecoder?
Turns out YAMLDecoder is not a decoder, it's a ReadCloser 😆
Or at least not in the sense that YAMLOrJSONDecoder provides a Decode function.
|
Since I'll be away next week, let me merge this now, and I'll investigate the suggested improvements in a subsequent PR. |
I wonder how we got away with splitting on *any* three dashes for so long. Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>
I wonder how we got away with splitting on *any* three dashes for so long. Signed-off-by: Thomas Runyon <runyontr@gmail.com>
What this PR does / why we need it:
I wonder how we got away with splitting on any three dashes for so
long.
Fixes #1297