jdiegodcp / ramlfications

Python parser for RAML
https://ramlfications.readthedocs.org
Apache License 2.0
234 stars 50 forks source link

Load JSON $refs #40

Closed econchick closed 8 years ago

econchick commented 8 years ago

Addresses #4; includes commits from #19

ping @benhamill @nvillalva @jhl2343 if you'd like to review my approach.

benhamill commented 8 years ago

This looks at least mostly-good to me. I want (before or after merge) to sit down with the tests when I'm less sleepy and really grok them and compare the inputs and expectations. With them broken across files like that, it's a bit hard to get, but I totally understand why you did it that way.

econchick commented 8 years ago

A bit of a design question for anyone (@benhamill looking at you :smile: ) - the RAML spec says, in relation to a RAML file with !includes:

If the original file is fetched as an HTTP resource, the included file SHOULD be fetched over HTTP.

So if a RAML file was on a remote server requested over HTTP, then included files should also be fetched over HTTP (versus HTTPS, or another protocol).

The JSON Schema draft 3 and draft 4 explicitly says the specification is agnostic to protocol. So theoretically, a $ref could point to an FTP server, or something (it looks like this works with the jsonref library). I may have missed it, but I can't find anything within the drafts that say if I get a remote JSON file over HTTP, any remote $refs have to match protocols.

Q: If a RAML file is remote, should the $ref protocol match the protocol over which the RAML file was fetched? e.g. if I got a RAML file over HTTPS, and there's a JSON $ref pointing to HTTP, or FTP, or anything other than HTTPS, should ramlfications error out?

econchick commented 8 years ago

meh - I've concluded that it's not worth it (enforcing protocols for remote $refs)

benhamill commented 8 years ago

I read through some test cases and my head started to hurt. Realistically, I don't think I'm going to grok them the way I'd like. I say merge it.

econchick commented 8 years ago

Oooph @benhamill I don't like the sound of that! What's confusing? Is there something that could be clearer/better?

benhamill commented 8 years ago

Heh. There could be fewer permutations to test! ;) But seriously: I think it's just a complex problem space with a lot of corners and the data under test necessarily has to be spread across a number of files for each test and given the amount of hard thinking I'm doing in other aspects of my life, I can't marshal the mental resources to hold it all in my head at once. It's not the code; it's me.

econchick commented 8 years ago

closes #19 and #4

benhamill commented 8 years ago

Woo! :metal: