api icon indicating copy to clipboard operation
api copied to clipboard

feature request - add an error sub-type field for more precise error checking

Open jmccormick2001 opened this issue 6 years ago • 4 comments

feature request...

if a user of this api wanted to specifically check a CSV for 'example annotations' it would be nice to have a predefined sub-type for that specific validation check...

for example:

if err.Type == errors.CSVFileNotValid && err.SubType == errors.ExampleAnnotationsNotFound { }

jmccormick2001 avatar Dec 03 '19 22:12 jmccormick2001

This is a somewhat complex feature to implement because errors.Error contains a predefined set of error types, to which we would have to add types for each field being validated, for all manifest types this library validates (ex. errors.ExampleAnnotationsNotFound for ClusterServiceVersions). That isn't a scalable approach.

However I agree we need to have some way to either check error types for field values or subdivide validators further.

/cc @njhale @kevinrizza

estroz avatar Dec 03 '19 23:12 estroz

If we go down this path, would using Go 1.13's support for error wrapping and using errors.As() and/or errors.Is() help us?

joelanford avatar Dec 04 '19 03:12 joelanford

I think we can close this one as outdated. See that if the annotation not be informed we ignore it. Also, we added the check: https://github.com/operator-framework/api/pull/207 you can see an example of this called in SDK https://github.com/operator-framework/operator-sdk/pull/5495.

@exdx @dinhxuanvu WDYT?

camilamacedo86 avatar Feb 14 '22 11:02 camilamacedo86

This is somewhat outdated specifically in regards to the annotation validation, considering the linked PR where validation was done to ensure the annotation is valid JSON. More so, as we move away from the CSV and moving the related annotations into a properties.yaml file this type of check would be less useful.

More broadly, the ask for more defined errors is useful though. Not sure if we should close just yet.

exdx avatar Feb 14 '22 19:02 exdx