Ref resolution scope

[gazpachoking]

Okay, I think this resolves 😜 the issue mentioned in #66. It at least passes the tests I have at json-schema-org/JSON-Schema-Test-Suite#34

@Julian, how do you feel about this method?

[Julian]

So is resolve a useful function anymore? It's never called by us anymore and it seems anyone resolving a ref would always need the new scope too. Is that right?

[gazpachoking]

Yeah, seems right. I mostly left it in as the path of least resistance to see if unit tests were still passing. :P It can certainly come out though.

[Julian]

So would something that looks like:

with ref_resolver.resolving(ref) as resolved:
    self.validate(instance, resolved)

work instead of both methods? Will that work for whatever other cases we'll
need to deal with?
On Feb 20, 2013 2:14 PM, "Chase Sterling" notifications@github.com wrote:

Yeah, seems right. I mostly left it in as the path of least resistance to
see if unit tests were still passing. :P It can certainly come out though.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/68#issuecomment-13850448.

[gazpachoking]

Hehe, that sounds an awful lot like my first implementation :P I do like it though. We may need to keep the in_scope method to deal with id keyword from iter_errors method.

[gazpachoking]

How about in unit tests, are we going to use the resolving context manager still?

[gazpachoking]

I guess it only makes sense.

[gazpachoking]

I implemented your idea, and updated the tests. Seem better now?

[Julian]

Awesome, do you think this will be robust enough for what you have in mind for whatever else we'll need?

[gazpachoking]

Yeah, this solution seems good to me. The only thing I was thinking about, is that if someone turns off cache_remote, the remote schemas will be fetched again for any json pointer references within them.

[gazpachoking]

I have an idea for that though. Gimme a few.

[gazpachoking]

@Julian That should take care of previously mentioned issue. Seem decent?

[Julian]

Looks great! Gonna play with it locally for a bit and then we can merge.

[gazpachoking]

Should we make the json tests run with a custom RefResolver, and just load the results of jsonschema_suite remotes into the store to get the new tests running?

[Julian]

Um. Well I think the idea for the remotes was to actually get all the way to the point where some HTTP request was about to be fired, not just sticking in the store and relying on the cache. So e.g. stubbing out requests / urllib to return those remotes.

[Julian]

After that if the tests pass feel free to merge.

[gazpachoking]

Ahh, yeah, I was mostly thinking about testing the resolution went correctly. But it makes sense to test more of the code and mock the actual request. I'm almost certainly going to come up with some suboptimal way to get this going, you can show me how it should be done after that. :P

[Julian]

Heh OK ;) check out mock.Mock.side_effect and probably just set it to the remotes dict's .get if that makes sense.

[gazpachoking]

I'm a bit confused about what you mean with side_effect, I do have something working now, I'm having an issue with running on python 3 though, as jsonschema_suite is not python 3 compatible.

[gazpachoking]

Should it just use the remotes folder from the suite rather than running jsonschema_suite remotes?

[gazpachoking]

This is what I'm doing right now btw https://gist.github.com/gazpachoking/02c741d436a0a1b4546c

[Julian]

OK merged. See 65a0e17 for what I meant, sorry I sucked at explaining it.

Feel free to improve upon anything you see fit.

[Julian]

Oh I guess I broke 2.6 by using check_output.

[Julian]

I think 2.6 is going to be broken anyhow because it doesn't support 2.6. So I'll skip these for now on 2.6.

[gazpachoking]

Yeah, I knew you were going to come up with something cleaner. :) Also, pretty sure I had it working with 2.6, I also started with check_output, but removed it for that.

[Julian]

I see what you had, but bin/jsonschema_suite doesn't support 2.6. Unless I install argparse. I guess I can do that and add it to tox.ini.

[gazpachoking]

Oh yeah. I did that too. :P

[gazpachoking]

I'm not too concerned whether remotes get tested on 2.6 though, just wanted to point out it was possible.

[Julian]

OK, neither am I. We'll leave it for now. py3 is also failing beacuse of stupid json.load not taking actual non-text file objects I'm sure. I'm not really in the mood to debug that atm.

[Julian]

Think I'm gonna do a release. Anything you think is outstanding to get done beforehand?

[gazpachoking]

The FormatChecker stuff was the only thing I was thinking.

[Julian]

Ah, okay. We can do that after the release then. Just checking over the
documentation to make sure it's all accurate still.
On Feb 24, 2013 1:46 PM, "Chase Sterling" notifications@github.com wrote:

The FormatChecker stuff was the only thing I was thinking.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/68#issuecomment-14013333.