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

Add the content type 'application/sparql-update' when preparing a SPARQL update request #1022

Merged

Conversation

NoutieH
Copy link

@NoutieH NoutieH commented Apr 24, 2020

PR for issue #1021 @nicholascar

When sending a POST request to the SPARQL update endpoint, the endpoint wants to be told which type of content it'll receive in request.
Since SPARQLConnector encodes the data with UTF-8 the request library can not deduce the correct Content-Type itself. By adding it explicitly to the headers parameter, the requests library will send to request the SPARQL update endpoint understands. At least when the sparql endpoint is a GraphDB instance...

Note: Didn't find a SPARQLConnector unit test where the addition of the content type header could be validated. Skipped making one... (?)

Closes #1021

…header before sending the update query to a the SPARQL update endpoint
@coveralls
Copy link

coveralls commented Apr 24, 2020

Coverage Status

Coverage decreased (-0.3%) to 75.601% when pulling 0986737 on NoutieH:feature/add_content_type_to_request_header into 3f0401d on RDFLib:master.

@effigies
Copy link
Contributor

Going by https://w3c.github.io/rdf-tests/sparql11/data-sparql11/protocol/index.html, might be worth updating query() to use application/sparql-query as well?

@nicholascar
Copy link
Member

@effigies I've done as you suggest adding to query().

@white-gecko: since I've added to this, please could you review?

@NoutieH
Copy link
Author

NoutieH commented Apr 28, 2020

@nicholascar Tnx for your updates! One thing: Given the w3C query spec the Content-Type header only needs to be set when request method is POST...

@nicholascar
Copy link
Member

@white-gecko can you please review and likely merge? Just want to make sure I, as a contributor to the PR, don't merge

@white-gecko
Copy link
Member

Could we add a test for this?

@white-gecko white-gecko self-assigned this May 1, 2020
@white-gecko
Copy link
Member

Now I have contributed as well. But I think the test is the code-ification of my review result so I did approve it and will merge it.

@white-gecko white-gecko merged commit 827eabd into RDFLib:master May 1, 2020
@white-gecko white-gecko added this to the rdflib 6.0.0 milestone May 1, 2020
white-gecko added a commit to white-gecko/rdflib that referenced this pull request May 3, 2020
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.

Missing Content-Type header for SPARQLConnector update method (?)
6 participants