[Bug] d2g subcommand changes done to add support for YAML file. #6765
Conversation
- -f option with filename functionality done. - WIP: task def coding done, testing remaining due to incomplete test case(payload option missing) - WIP: Piped input coding remaining, need to detect file type passed.
anirudh-pandey
requested review from
lotas,
petemoore and
matt-boris
and removed request for
a team
for more information, see https://pre-commit.ci
| if len(filePath) >= 4 { | ||
| fileExtension = filePath[len(filePath)-4:] | ||
| } | ||
| input, err := userInput(filePath, fileExtension) |
There was a problem hiding this comment.
Rather than checking the file extension, it would be better to check the content. There are many variations for the file extension, including, but not limited to no extension, .yaml, .yml, .YAML, .YML, .json, .JSON. What matters really is only whether the content can be successfully interpreted as a particular format, and if it passes the payload validation (i.e. includes required fields, etc). For this, it is only necessary to attempt to parse the content in a particular format, and then decide based on whether the data was successfully parsed, whether the data is of the particular format you are trying.
| @@ -95,16 +102,23 @@ func convert(cmd *cobra.Command, args []string) (err error) { | |||
| return fmt.Errorf("failed to unmarshal generic worker task definition: %v", err) | |||
There was a problem hiding this comment.
This would need to be changed. If you aren't able to unmarshal, it suggests the input format is not json. At this point, knowing it is not json, the next task is to see if it is yaml.
| @@ -179,3 +207,8 @@ func validateJSON(input []byte, schema string) error { | |||
|
|
|||
| return nil | |||
| } | |||
|
|
|||
| func isYAMLExtension(fileExtension string) bool { | |||
There was a problem hiding this comment.
As mentioned above, this is not the best way to determine the format of the content. Furthermore, this function would consider a file called foo.notyaml to be a yaml file. The standard library provides filepath.Ext for this purpose which handles variable length extensions, but we shouldn't need to consider the filename, especially because the input can come from standard in, so we need a means to be able to deduce it without a filename.
|
Many thanks for your efforts so far @anirudh-pandey! I think there is still a little bit of work to be done, but let me know if my suggestions make sense to you or not. I'm happy to help. |
✅ Tasks Done:-
Github Bug/Issue: Fixes #6731