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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Register additional serializer plugins for SPARQL mime types. #987

Merged

Conversation

darrengarvey
Copy link
Contributor

This mirrors the similar parser registrations below in the same file.
With this commit, an Accept header can be used directly to serialize a
response, making putting up a SPARQL compatible endpoint trivial.

@coveralls
Copy link

coveralls commented Mar 26, 2020

Coverage Status

Coverage increased (+0.006%) to 75.894% when pulling 09d0652 on darrengarvey:register-additional-mimetype into 2aebbf0 on RDFLib:master.

@nicholascar nicholascar added this to the rdflib 5.0.0 milestone Mar 27, 2020
@ashleysommer
Copy link
Contributor

ashleysommer commented Mar 28, 2020

@darrengarvey
This looks great, thanks.

Just one thing we'd like to discuss before merging.

Do we really need two different entries for application/sparql-results+xml; charset=UTF-8 and application/sparql-results+xml? Seems this is an opportunity to make this lookup more concise.

I understand the content-type header can and sometimes does include the charset (or other additional parameters), do you expect the user will pass that directly into the serializer? Should there be a mimetype sanitizer that splits removes anything after a semicolon when matching the mimetype to serializer (or parser)?

This mirrors the similar parser registrations below in the same file.
With this commit, an Accept header can be used directly to serialize a
response, making putting up a SPARQL compatible endpoint trivial.
@darrengarvey
Copy link
Contributor Author

I was following the pattern from a few lines below on the parser side, which was added intentionally in #941.

Actually the charset isn't valid in the Accept header: there is Accept-Charset for that. The line you referenced was redundant, so I've removed it. Thanks @ashleysommer!

@ashleysommer ashleysommer merged commit 5e52fa8 into RDFLib:master Mar 28, 2020
@ashleysommer
Copy link
Contributor

Thanks. Merged, just in time for the 5.0.0 release.

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

4 participants