grafana / thema

A CUE-based framework for portable, evolvable schema
Apache License 2.0
229 stars 12 forks source link

feature request: add error return to `ValidateAny()` #156

Open mildwonkey opened 1 year ago

mildwonkey commented 1 year ago

ValidateAny returns a nil instance if no conforming schema is found but no error, so you end up with this perfectly-valid-but-not-very-go-ish code block:

i := lin.ValidateAny(dashdata)
if i == nil {
  fmt.Errorf("BADONG") //  if you get this ref we are friends
}

It would be nice to have something more idiomatic:

i, err := lin.ValidateAny(dashdata)
if err != nil {
  return err
}

I think this would be nice to have even if we don't do anything to polish the returned errors (yet 😉)

sdboyer commented 1 year ago

i had a similar thought when i initially wrote it. But i also had a conundrum: in a multi-schema lineage where all schemas fail validation, which error do we return? In what order? For that matter, in what order should ValidateAny() move through all the schemas in a lineage?

Now that we're further along and starting to think more properly about errors, i don't actually find the above questions as intimidating: some kind of multierror that contains all of the schemas' error responses, keyed by version. (If we see other use cases for "error across all schemas," we could later expose a type for this.) And then yes, put that all into an additional error return value.

That said, ValidateAny() has always been a bit of a hack. It's fully implementable externally, so is redundant bloat on the Lineage interface. It was a shortcut that let me create vmuxers without having to design some more proper indirection patterns to, at minimum, allow the caller control over iteration order.

So: i'm :+1: to this change and would happily accept a PR, but would like to see it moved into a standalone func. An initial PR could kick the can down the road on iteration order - adding an error return and moving the func is sufficient. i suspect we can use a variadic option pattern later to avoid another breaking change.