Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ref v2 Spec #3

Closed
6 tasks
Julian opened this issue Jul 19, 2017 · 3 comments
Closed
6 tasks

Ref v2 Spec #3

Julian opened this issue Jul 19, 2017 · 3 comments
Labels
enhancement New feature or request

Comments

@Julian
Copy link
Member

Julian commented Jul 19, 2017

This is some random notes on a new version of the ref resolution API. Feel free to leave comments, although some of the below might be overly terse chicken scratch

  • It should probably be easier to pre-resolve all refs (maybe that should even be the default) rather than lazy/forced resolution
  • Keep in mind async use cases
  • Too many methods bruh. Just 2? Resolver().resolve(ref) + Resolver().for_scope(url)?
  • Take yarl.URLs, not strs
  • What should be the thing for users who want to resolve relative to files on disk?
  • Move the resolver argument to extend
@bsmithers

This comment has been minimized.

@Julian

This comment has been minimized.

@agoose77
Copy link

agoose77 commented Jul 5, 2018

I've just been thinking about the problem of following $id declarations during ref resolution and came across this issue.

I don't think that we should recursively dereference schemas. Whilst it's certainly a simpler approach (at least, conceptually), I think the extra cost should be avoided (especially if there are schemas with lazy reference paths (though I'm not sure if currently jsonschema does this, as even in the case of oneOf we need to ensure only one subschema validates (and hence we have to dereference it anyway)).

I think a better approach would be to follow the $id uri from within the resolution process, such as the approach taken here. I'm not convinced that we could (or should) just use this repository (for a start, it doesn't support the newer $id keyword, but the approach is valid enough (we already use context managers to track the current scope).

I'll give it some thought.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants