jdiegodcp / ramlfications

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

loader does not support reading file from url #104

Open derrickburns opened 8 years ago

derrickburns commented 8 years ago

We should be able to load a raml file from a URL, and not just a file. Note that this changes the way in which includes are processed.

econchick commented 8 years ago

@derrickburns just clarifying, are you wanting to load a main RAML file from URL (e.g. ramlfications.parse("https://api.example.com") or ramlfications.load("https://api.example.com"))? or are you wanting to load a local RAML file, that has an !include pointing to a URL?

The latter, I agree, should be supported (and I think is for remote JSON files IIRC).

derrickburns commented 8 years ago

The former. In that case, the loader must treat includes as references to URLs

Derrick

On Jun 7, 2016, 9:41 AM -0700, Lynn Rootnotifications@github.com, wrote:

@derrickburns(https://github.com/derrickburns)just clarifying, are you wanting to load a main RAML file from URL (e.g.ramlfications.parse("https://api.example.com")orramlfications.load("https://api.example.com"))? or are you wanting to load a local RAML file, that has an!includepointing to a URL?

The latter, I agree, should be supported (and I think is for remote JSON files IIRC).

— You are receiving this because you were mentioned. Reply to this email directly,view it on GitHub(https://github.com/spotify/ramlfications/issues/104#issuecomment-224340156), ormute the thread(https://github.com/notifications/unsubscribe/AAjo-hEXjVca8Coek3lY-yrp5uhBQZ-yks5qJZ8_gaJpZM4IwDma).

econchick commented 8 years ago

@derrickburns hmm so, this isn't actually a requirement in the spec - I don't see it in 0.8 or 1.0 (please correct me if I'm wrong). I'm also hesitant to actually add this feature, as it seems a bit insecure.

Looking at the json library in the stdlib in Python, loading remote JSON files do not seem to be supported. And it seems that the pyyaml package doesn't either (not without implementing your own loader, and hopefully a SafeLoader). I understand how it would be a nice feature to have, as it could get quite annoying to download files that you need.

I will keep this open, but this feature will be low priority, behind implementing RAML1.0 and any other missing functionality that is required of the spec.

derrickburns commented 8 years ago

Here is the key paragraph from the 0.8 spec:

"If a relative path is used for the included file, the path is interpreted relative to the location of the original (including) file. If the original file is fetched as an HTTP resource, the included file SHOULD be fetched over HTTP."

https://github.com/raml-org/raml- spec/blob/master/versions/raml-08/raml-08.md#includes

Sent from Nylas N1, the extensible, open source mail client.

On Jun 7 2016, at 12:31 pm, Lynn Root <notifications@github.com> wrote:

@derrickburns hmm so, this isn't actually a requirement in the spec - I don't see it in 0.8 or 1.0 (please correct me if I'm wrong). I'm also hesitant to actually add this feature, as it seems a bit insecure.

Looking at the json library in the stdlib in Python, loading remote JSON files do not seem to be supported. And it seems that the pyyaml package doesn't either (not without implementing your own loader, and hopefully a SafeLoader). I understand how it would be a nice feature to have, as it could get quite annoying to download files that you need.

I will keep this open, but this feature will be low priority, behind implementing RAML1.0 and any other missing functionality that is required of the spec.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

derrickburns commented 8 years ago

I understand and agree with the priorities. However, it means that one cannot parse a RAML file that is served from a web server. Copying the file is insufficient, because the include links would all be broken. Thus, it is not possible to write a service that checks for compliance with a remotely served raml file without also downloading all the files separately.

derrickburns commented 8 years ago

A simple option is to provide a hook for processing "!include" directives....

econchick commented 8 years ago

Ah okay good catch, thanks for correcting me. I'll re-prioritize this for the 0.2.0 release.

derrickburns commented 8 years ago

Would you like a PR from me for such a "hook"?

econchick commented 8 years ago

I would gladly accept any PRs! Be sure to branch off of the v0.2.0-dev branch.

derrickburns commented 8 years ago

Thx. I'll have a look to see what I can do.