operator-framework / api

Contains the API definitions used by OLM and Marketplace
Apache License 2.0
29 stars 73 forks source link

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

Open jmccormick2001 opened 4 years ago

jmccormick2001 commented 4 years ago

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 { }

estroz commented 4 years ago

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

joelanford commented 4 years ago

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?

camilamacedo86 commented 2 years ago

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?

exdx commented 2 years ago

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.