Add referrer to handler's call

[ghost]

For handlers to resolve relative references, they need a referrer to be
passed in.

Looking at the code and tests especially I'm starting to doubt if this was the intent of the referrer argument. It can be used in this way, but what resolver.resolve() returns as first member of the tuple is a bit magical to me. However, we do need a way to be able to resolve relative references.

The code also introduces a default scheme, with a default conforming to what is normal in the URI resolver world.

[Julian]

Hi, will have to read this more carefully, but IIRC the intention was probably that handlers just always get called with absolute URIs.

This also couldn't get merged exactly as is, because it'd break backwards compatibility, but if it turns out this makes sense we can likely correct that part easily.

[ghost]

If this is part of the jsonschema spec (and I admit, I haven't read it), then you're right it's the wrong place to fix it. From a practical point of view, you'd need to pre-process documents that have relative references and it really wouldn't make sense anymore to use jsonschema's RefResolver class or the handler interface.
So perhaps it makes more sense to create a RelativeRefResolver class that has the API contract I'm proposing? That way there's no backwards compatibility issue. If you agree, I'll rework using that approach.

P.S. I'm guessing the PyPy failure has nothing to do with my PR.

[Julian]

From a practical point of view, you'd need to pre-process documents that have relative references

I don't think you need to preprocess do you?

The RefResolver should know what the base URI is, so it's just that each handler shouldn't have to deal with resolving relative references, the RefResolver before it calls the handler resolves the relative reference to its base URI.

(By the way I know close to 0 about URIs so apologies in advance if anything I have said, say, or will say makes negative amounts of sense :)

[Julian]

P.S. I'm guessing the PyPy failure has nothing to do with my PR.

It is actually, it's pointing out that urlsplit isn't a word, though sphinx-spelling's output is not exactly easy to read.

The fix there is to use a Sphinx reference (`urlsplit`), but let's make sure we agree first on whether this makes sense before too many specific comments on the implementation.

There's also #306 and to a lesser extent #434 open which might be relevant to have a look at.

[ghost]

The RefResolver should know what the base URI is, so it's just that each handler shouldn't have to deal with resolving relative references, the RefResolver before it calls the handler resolves the relative reference to its base URI.

Relative references are relative to the document they are specified in (the referrer). Which is why even with a base URI, you cannot translate correctly if you don't know the location of the referrer:

base = '/home/julian'
uri = '../common/schemas.yaml'
referrer = '/home/julian/endpoint/paths.yaml'

With just base and uri, you can only come up with /home/common/schemas.yaml as the absolute uri. Adding referrer to the mix as the document where the reference is made, you will see that the intended relative reference is /home/julian/common/schemas.yaml (since a double dot from a document refers to the parent of it's container, not it's container).

Now if we go to section 5.1.3 of RFC 3986, then one could argue that each RefResolver should be handed a different base uri, corresponding to the containing document. So there would be no global base, but referrer would be passed in as the base. Again, this can't be done as the handler context is unable to keep such state information without the help of jsonschema. At least, I don't see a way to do that, let alone do that thread safe.

[Julian]

Relative references are relative to the document they are specified in (the referrer). Which is why even with a base URI, you cannot translate correctly if you don't know the location of the referrer:

The RefResolver has both the base URI and the referring document, and the latter should contain its own URI.

It's still possible I'm missing something basic here certainly, but I'm 90% sure that every handler shouldn't be doing its own URL resolution.

A handler is the equivalent of "given a scheme, go fetch the resource that lives at a given fully, correctly resolved URL with that scheme".

All the actual resolution is (or was intended) to live external to the handler, since my understanding is that that shouldn't be scheme dependent. Is that understanding incorrect? (It's certainly possible that it isn't currently happening correctly, but yeah my inclination would be to correct that outside of "handlers" -- the handler should be being passed whatever URL needs to be resolved).

In theory, if more flexibility is required on how to dispatch particular URLs to handlers than just per-scheme, that's where something like python-jsonschema/referencing#4 comes in too.

[ghost]

So from your description I defer that with every reference, you should be creating a new RefResolver, using a somewhat static set of handlers and static base url, but variable referrers. This new RefResolver instance should translate the relative reference to an absolute URL (including scheme) and pass that on to the handler.

If the above is correct, then this code seems to adhere to that, except I'm still ending up with a RefResolver exception in very simple cases. So I need to investigate deeper where things go wrong.

[Julian]

@mes3yd lemme know what you find!

[ghost]

So finally got around to this: This turned out to be a documentation issue in the upstream library. When you pass a dict to the create_spec function (to obtain the result from a yaml format parsing), the base_url is passed in as optional. Technically it is, if you only have one file or abolute URLs. But when using relative URLs the base_url is required.

I am still not sure how the resolver handles this internally, since with only one base URL you need to somehow keep some state somewhere, but that's for another time.

Thanks Julian for the fast responses!

[Julian]

Glad you figured it out and yeah definitely let me know if you find anything further to address.

…

On Mon, Sep 10, 2018, 03:58 Melvyn Sopacua ***@***.***> wrote: So finally got around to this: This turned out to be a documentation issue in the upstream library. When you pass a dict to the create_spec function (to obtain the result from a yaml format parsing), the base_url is passed in as optional. Technically it is, if you only have one file or abolute URLs. But when using relative URLs the base_url is required. I am still not sure how the resolver handles this internally, since with only one base URL you need to somehow keep some state somewhere, but that's for another time. Thanks Julian for the fast responses! — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#445 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAUIXpp_7DENi_i-EJdnCbdVxDSaNebuks5uZhuygaJpZM4VwqcX> .