Outdated documentation for ValidateBytes

[matthiassommer]

The documentation presents examples such as
errs, err = rs.ValidateBytes(invalidPerson)

whereas the function definition is
func (s *Schema) ValidateBytes(ctx context.Context, data []byte) ([]KeyError, error) {

Can you provide me an example on how I should pass the context to this function? What is the benefit of this change?

For now, I go with

ctx := context.TODO()
errs, err := rs.ValidateBytes(ctx, metadataBytes)

[jc21]

Definitely interested to hear about "What is the benefit of this change?"

[b5]

Ah this is a great point, we'll get the documentation fixed up, but to speak to the benefit of the change:

we pass a context because the jsonschema spec states validation should be able to make web requests to fetch referenced schemas. Those requests can end up taking time (a slow server or a high-latency connection), and you might want to put a boundary on that request. Here's an example:

// create a context that times out after one second. whenever we create a context with values,
// we need to cancel it in all cases, so we defer a call to cancel
// we're passing the "background" context to "WithTimeout" because there's no deeper
// context to build on. context.Backround() has the same functionality as context.TODO()
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
defer cancel()

// call validation. If the root schema makes web requests, and those web requests
// take longer than one second, we'll get an error that is or wraps err.DeadlineExceeded
errs, err :=  rs.ValidateBytes(ctx, metadataBytes)

if err != nil {
  if errors.Is(err, context.DeadlineExceeded) {
    fmt.Println("request timed out!")
  }
  // handle other errors...
}

But if you don't want a deadline, the code you've written will work just fine! It's common practice to use context.Background() whenever you don't really need context cancellation. context.TODO() is generally reserved for code that's being upgraded to use contexts.

Hope that helps explain things! We'll mark this issue as closed when the docs are updated.

[Arqu]

The docs have been updated as part of #77. However I definitely think that the above higlights the rationale a lot more than just the code sample.