You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Keep object key order when resolving a schema #481
As explained in the title, when resolving a schema on Python version (< 3.6) where dict implementation does not keep key insertion ordering, the object key order can be lost. This can be especially problematic for the "properties" object that is usually found in schemas.
I would be glad to provide a PR to fix this. My approach would be to provide a json_kwargs as RefResolver.init argument and forward it to json.loads in resolve_remote, so that users that need this functionality (at least me) could pass the object_pairs_hook argument with an OrderedDict value.
Hey -- RefResolver is the only place that really deals with deserialization yeah -- this is probably related to python-jsonschema/referencing#4, since I imagine a solution for the first would solve this too, though obviously that ticket is more general. I haven't yet fully thought through what a decent interface looks like there, but maybe have a look, since I suspect there's one decent solution that covers both use cases.
Some random brainstorming:
What I'd initially imagine is an argument for a deserializer, and that deserializer being injectable with your version of JSON loading.
But, even better I suspect would be to have <HTTP layer> wrapped by <HTTP to Python Object Layer>, and then only the latter gets passed to RefResolver.
There are after all at least 3 distinct tasks here:
Take URL, find handler that knows how to retrieve it
Take handler (e.g. HTTP), make request, return response (this is unfortunately right now either requests or the urllib2 module)
Take (e.g. HTTP) response, decide how to deserialize it based on configuration + context
And yeah how many objects that is, and whether any or all of them is RefResolver is what I still haven't fully thought through, so comments definitely welcome :)
The other issue is way more general than the problem i am facing right now. Also i am using the resolver locally with file:// scheme, no HTTP server involved.
But as a first step, we could add a deserializer argument to RefResolver and call it in resolve_remote. Default value would still be json.loads.
This way, you could solve the yaml problem and this issue at the same time.
The .json() method from the requests module will not be used anymore though, as all deserialization will use the same code path (which i think if safer anyway).
It is, yes, but I don't want 2 (which undoubtedly eventually becomes 3 and
4 and ..) when a decently designed interface solves both issues and seems
likely to solve future ones.
If such a solution exists that'd be worth doing, which is why I say it
needs thinking through before just adding arguments willy nilly.
On Wed, Oct 24, 2018, 09:29 marmeladema ***@***.***> wrote:
The other issue is way more general than the problem i am facing right
now. Also i am using the resolver locally with file:// scheme, no HTTP
server involved.
But as a first step, we could add a deserializer argument to RefResolver
and call it in resolve_remote. Default value would still be json.loads.
This way, you could solve the yaml problem and this issue at the same time.
The .json() method from the requests module will not be used anymore
though, as all deserialization will use the same code path (which i think
if safer anyway).
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#481 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAUIXi7J7cVqab3lW5zpqIilF4jfxYg2ks5uoGs2gaJpZM4X3hYQ>
.
And by the way, requests' json method cannot just be replaced with
json.loads unfortunately, which is almost the whole reason we use requests.
It implements what the json spec says is a correct unicode decode dance,
which json.loads does not.
On Wed, Oct 24, 2018, 09:32 Julian Berman ***@***.***> wrote:
It is, yes, but I don't want 2 (which undoubtedly eventually becomes 3 and
4 and ..) when a decently designed interface solves both issues and seems
likely to solve future ones.
If such a solution exists that'd be worth doing, which is why I say it
needs thinking through before just adding arguments willy nilly.
On Wed, Oct 24, 2018, 09:29 marmeladema ***@***.***> wrote:
> The other issue is way more general than the problem i am facing right
> now. Also i am using the resolver locally with file:// scheme, no HTTP
> server involved.
> But as a first step, we could add a deserializer argument to RefResolver
> and call it in resolve_remote. Default value would still be json.loads.
> This way, you could solve the yaml problem and this issue at the same
> time.
> The .json() method from the requests module will not be used anymore
> though, as all deserialization will use the same code path (which i think
> if safer anyway).
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <#481 (comment)>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAUIXi7J7cVqab3lW5zpqIilF4jfxYg2ks5uoGs2gaJpZM4X3hYQ>
> .
>
It is, yes, but I don't want 2 (which undoubtedly eventually becomes 3 and
4 and ..) when a decently designed interface solves both issues and seems
likely to solve future ones.
Do you think a PR as a "work-in-progress" base of discussion could help here ? Not meant to be merged of course. Just so that people can comment, and we can go toward finding a solution.
And by the way, requests' json method cannot just be replaced with json.loads unfortunately, which is almost the whole reason we use requests. It implements what the json spec says is a correct unicode decode dance, which json.loads does not.
We can still use the guess_json_utf function from the requests module to handle this dance properly.
On Wed, Oct 24, 2018, 09:44 marmeladema ***@***.***> wrote:
It is, yes, but I don't want 2 (which undoubtedly eventually becomes 3 and
4 and ..) when a decently designed interface solves both issues and seems
likely to solve future ones.
Do you think a PR as a "work-in-progress" base of discussion could help
here ? Not meant to be merged of course. Just so that people can comment,
and we can go toward finding a solution.
And by the way, requests' json method cannot just be replaced with
json.loads unfortunately, which is almost the whole reason we use requests.
It implements what the json spec says is a correct unicode decode dance,
which json.loads does not.
We can still use the guess_json_utf function from the requests module to
handle this dance properly.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#481 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAUIXmTkjdWyvZEZpI1AfjMgITVsK4j9ks5uoG6ngaJpZM4X3hYQ>
.
I had a similar use-case; I wanted to add a timeout to the request. Based on RefResolver.resolve_remote, I replaced all calls to Draft7Validator with a factory method that created a validator + RefResolver with custom handlers (they take precedence):
This seems to work fine, although if the API settles down, I would just inherit from RefResolver and override resolve_remote to avoid weird fall-through cases.
from __future__ importabsolute_import, print_function, unicode_literalsimportrequestsfromcollectionsimportOrderedDicturl="https://api.github.com/users/octocat/repos"requests.get(url).json(object_pairs_hook=OrderedDict)
So you could use also use a function and custom handlers, e.g.:
Sure, it isn't super clean or optimal, and personally, I'd dump legacy Python and move to Python 3, but maybe this unblocks you?
(Also, if there's a reason not use do this, I'd like to know)
For the broader discussion, being able to mess around with RefResolver would be super useful because it does a lot, but I can understand the reluctance of having to support old designs.
(Also, if there's a reason not use do this, I'd like to know)
This seems to work fine, although if the API settles down, I would just inherit from RefResolver and override resolve_remote to avoid weird fall-through cases.
Subclassing isn't a supported public API for any classes in jsonschema :), but otherwise this looks reasonable.
For the broader discussion, being able to mess around with RefResolver would be super useful because it does a lot, but I can understand the reluctance of having to support old designs.
It would not work for me as i don't not want to provide a handler for a specific protocol, i need ordering whatever the protocol. I can not even fake the handlers argument because it is transformed as dict in the constructor.
We (the library) no longer support <3.6 so I think this is closeable. If that's not the case and there's still what to address here feel free to follow up.
Activity
Julian commentedon Oct 24, 2018
Hey --
RefResolveris the only place that really deals with deserialization yeah -- this is probably related to python-jsonschema/referencing#4, since I imagine a solution for the first would solve this too, though obviously that ticket is more general. I haven't yet fully thought through what a decent interface looks like there, but maybe have a look, since I suspect there's one decent solution that covers both use cases.Some random brainstorming:
What I'd initially imagine is an argument for a deserializer, and that deserializer being injectable with your version of JSON loading.
But, even better I suspect would be to have
<HTTP layer>wrapped by<HTTP to Python Object Layer>, and then only the latter gets passed toRefResolver.There are after all at least 3 distinct tasks here:
And yeah how many objects that is, and whether any or all of them is
RefResolveris what I still haven't fully thought through, so comments definitely welcome :)marmeladema commentedon Oct 24, 2018
The other issue is way more general than the problem i am facing right now. Also i am using the resolver locally with
file://scheme, no HTTP server involved.But as a first step, we could add a deserializer argument to
RefResolverand call it inresolve_remote. Default value would still bejson.loads.This way, you could solve the yaml problem and this issue at the same time.
The .json() method from the requests module will not be used anymore though, as all deserialization will use the same code path (which i think if safer anyway).
Julian commentedon Oct 24, 2018
Julian commentedon Oct 24, 2018
marmeladema commentedon Oct 24, 2018
Do you think a PR as a "work-in-progress" base of discussion could help here ? Not meant to be merged of course. Just so that people can comment, and we can go toward finding a solution.
We can still use the
guess_json_utffunction from therequestsmodule to handle this dance properly.Julian commentedon Oct 25, 2018
tobywf commentedon Nov 13, 2018
I had a similar use-case; I wanted to add a timeout to the request. Based on RefResolver.resolve_remote, I replaced all calls to
Draft7Validatorwith a factory method that created a validator + RefResolver with custom handlers (they take precedence):This seems to work fine, although if the API settles down, I would just inherit from
RefResolverand overrideresolve_remoteto avoid weird fall-through cases.Your problem is similar. Luckily, on recent requests version (don't know how recent), you can specify extra keyword arguments for
.json()and still get all the sanity-preserving encoding handling:So you could use also use a function and custom
handlers, e.g.:Sure, it isn't super clean or optimal, and personally, I'd dump legacy Python and move to Python 3, but maybe this unblocks you?
(Also, if there's a reason not use do this, I'd like to know)
For the broader discussion, being able to mess around with RefResolver would be super useful because it does a lot, but I can understand the reluctance of having to support old designs.
Julian commentedon Nov 13, 2018
Subclassing isn't a supported public API for any classes in jsonschema :), but otherwise this looks reasonable.
Definitely keen to hear if you have any use cases that wouldn't be solved by what's in python-jsonschema/referencing#4!
marmeladema commentedon Nov 13, 2018
It would not work for me as i don't not want to provide a handler for a specific protocol, i need ordering whatever the protocol. I can not even fake the
handlersargument because it is transformed as dict in the constructor.tobywf commentedon Nov 13, 2018
Fair enough, was hoping one of those approaches helps, but guess it's a niche use-case. My understanding is JSON specifies objects are unordered.
Anyway, I'll add my thoughts to python-jsonschema/referencing#4 , didn't mean to hijack the issue!
Squashed 'json/' changes from 15ec577f..09fd353f
ssbarnea commentedon May 20, 2022
Julian commentedon May 20, 2022
ssbarnea commentedon May 20, 2022
Julian commentedon Nov 1, 2022
We (the library) no longer support <3.6 so I think this is closeable. If that's not the case and there's still what to address here feel free to follow up.