Skip to content

Keep object key order when resolving a schema #481

Closed

Description

@marmeladema

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.

Would you be ok with that ?

Activity

  1. Julian commented on Oct 24, 2018

    @Julian
    Member

    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 :)

  2. marmeladema commented on Oct 24, 2018

    @marmeladema
    Author

    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).

  3. Julian commented on Oct 24, 2018

    @Julian
    Member
  4. Julian commented on Oct 24, 2018

    @Julian
    Member
  5. marmeladema commented on Oct 24, 2018

    @marmeladema
    Author

    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.

  6. Julian commented on Oct 25, 2018

    @Julian
    Member
  7. tobywf commented on Nov 13, 2018

    @tobywf

    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):

    def make_validator(schema, ..., timeout=TIMEOUT_IN_SECONDS):
        # ..., figure out base_uri etc
    
        def get_with_timeout(uri):
            return requests.get(uri, timeout=timeout).json()
    
        resolver = RefResolver(
            base_uri=base_uri,
            referrer=schema,
            handlers={"http": get_with_timeout, "https": get_with_timeout},
        )
        return Draft7Validator(schema, resolver=resolver)

    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.

    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:

    from __future__ import absolute_import, print_function, unicode_literals
    import requests
    from collections import OrderedDict
    
    url = "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.:

    def get_with_order(uri):
        return requests.get(uri).json(object_pairs_hook=OrderedDict)

    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.

  8. Julian commented on Nov 13, 2018

    @Julian
    Member

    (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.

    Definitely keen to hear if you have any use cases that wouldn't be solved by what's in python-jsonschema/referencing#4!

  9. marmeladema commented on Nov 13, 2018

    @marmeladema
    Author

    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.

  10. tobywf commented on Nov 13, 2018

    @tobywf

    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!

  11. ssbarnea commented on May 20, 2022

    @ssbarnea
  12. Julian commented on May 20, 2022

    @Julian
  13. ssbarnea commented on May 20, 2022

    @ssbarnea
  14. Julian commented on Nov 1, 2022

    @Julian
    Member

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    EnhancementSome new desired functionality

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions