Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Link header when fetching remote contexts #10

Conversation

cheerfulstoic
Copy link
Contributor

I referenced this bit of the documentation (thanks @gkellogg in this comment). The most relevant being part 5 (quoted below). I haven't implemented all of it, but started with the basic case that would fix the issue. I'll start playing around with more fully supporting the spec.

One question, though: that document is for JSON-LD 1.1, and as far as I can see, this library doesn't support beyond 1.0 yet. I'm not sure if the Link header is a 1.0 thing, but I also wasn't able to get any JSON-LD data from various webpages to work because of this issue, so 馃し

"If the retrieved resource's Content-Type is application/json or any media type with a +json suffix as defined in [RFC6839] except application/ld+json, and the response has an HTTP Link Header [RFC8288] using the http://www.w3.org/ns/json-ld#context link relation, set contextUrl to the associated href."

"If multiple HTTP Link Headers using the http://www.w3.org/ns/json-ld#context link relation are found, the promise is rejected with a JsonLdError whose code is set to multiple context link headers and processing is terminated."

"Processors MAY transform document to the internal representation."

"NOTE: The HTTP Link Header is ignored for documents served as application/ld+json, text/html, or application/xhtml+xml."

@cheerfulstoic cheerfulstoic force-pushed the support-link-header-for-remote-contexts branch 3 times, most recently from 6f5d47d to fe6c8e9 Compare January 1, 2023 20:15
@marcelotto
Copy link
Member

Sorry for the late reply.

One question, though: that document is for JSON-LD 1.1, and as far as I can see, this library doesn't support beyond 1.0 yet.

That's correct.

I'm not sure if the Link header is a 1.0 thing, but I also wasn't able to get any JSON-LD data from various webpages to work because of this issue, so 馃し

It seems to be supported in 1.0 already: https://www.w3.org/TR/2014/REC-json-ld-20140116/#interpreting-json-as-json-ld

Copy link
Member

@marcelotto marcelotto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI doesn't pass yet.

@cheerfulstoic
Copy link
Contributor Author

馃 Is it passing on master? When I try to run it locally in master it fails. It seems to be that it's trying to connect to the http://json-ld.org/* URLs and they're failing to load.

@marcelotto
Copy link
Member

馃 Is it passing on master? When I try to run it locally in master it fails. It seems to be that it's trying to connect to the http://json-ld.org/* URLs and they're failing to load.

You're right. http://json-ld.org/ is currently down in general. json-ld/json-ld.org#802

@cheerfulstoic cheerfulstoic force-pushed the support-link-header-for-remote-contexts branch from fe6c8e9 to a7d68c6 Compare January 12, 2023 06:42
@cheerfulstoic
Copy link
Contributor Author

Seems like the site's back up. I re-pushed to see if I could get the tests to run, but maybe they're run manually. Anyway, hopefully they're working now 馃

Might be worth trying out exvcr for these tests which connect to json-ld.org so that the tests can run even if the site is down (I think maybe you can re-generate the cassettes occasionally to make sure you're still matching to real data)

@marcelotto
Copy link
Member

marcelotto commented Jan 12, 2023

Seems like the site's back up. I re-pushed to see if I could get the tests to run, but maybe they're run manually. Anyway, hopefully they're working now 馃

The problem was this: "First-time contributors need a maintainer to approve running workflows." So, your next PR should automatically trigger CI ;-)

I approved and it's passing now. 馃殌

Might be worth trying out exvcr for these tests which connect to json-ld.org so that the tests can run even if the site is down (I think maybe you can re-generate the cassettes occasionally to make sure you're still matching to real data)

Yes, I agree. In the sparql_client I'm already using this. But the whole remote-context handling was not written by me and there are more things to improve on this front, eg. the hard dependency on Hackney for this. I would prefer Tesla to give users the option to choose the HTTP client they want. Unfortunately, I don't have time to look into all of this at the moment. PRs are welcome, however.

@marcelotto marcelotto merged commit 9ece07a into rdf-elixir:master Jan 12, 2023
@cheerfulstoic cheerfulstoic deleted the support-link-header-for-remote-contexts branch January 12, 2023 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants