-
Notifications
You must be signed in to change notification settings - Fork 564
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
SPARQLStore: Use SPARQLWrapper for updates #397
Conversation
New versions of SPARQLWrapper now support SPARQL updates too. Therefore the methods inherited from SPARQLWrapper should be used for updates instead of building the request manually. Drawback: The user can no longer set whether to send updates as application/sparql-update or application/x-www-form-urlencoded. This feature should be introduced in SPARQLWrapper and the old behavoiur in SPARQLStore restored.
Mainly, we don't need to handle our own connection anymore. The keep alive option of SPARQLWrapper is activated. Some other small changes to improve consistency.
Removed empty/unnecessary comments and improved existing one.
Cool, so we will be able to use However, I noticed a bug when making a CREATE query: graph.update("CREATE GRAPH <urn:graph>") This query is not treated like a SPARQL Update query but is sent to the query endpoint as follows:
Instead of (with the current implementation)
|
guys, feel free to push to SPARQLWrapper whatever missing feature required for a better integration |
Ok, I have submitted a pull request RDFLib/sparqlwrapper#29 to fix the regression I have mentioned. |
great, pull request RDFLib/sparqlwrapper#29 merged on master |
Thanks! |
As I see, RDFLib/sparqlwrapper#28 is the only remaining issue wee have to address? |
A priori, yes. But I cannot fully tested this branch because it does not include recent commits of the master branch (named graph support). |
Support for named graphs in the SPARQLUpdateStore is required for further development. Conflicts: rdflib/plugins/stores/sparqlstore.py
There you go. |
Thanks. Ok, no problem detected while running the unittests of OldMan with Fuseki (mem and TDB) on Python 2.7. Looks good. |
Thanks to the newly introduced method `setUpdateMethod` of the SPARQLWrapper, the user can again choose whether to send SPARQL Updates as `application/sparql-update` or as `application/x-www-form-urlencoded`.
Clearly, the travis build is failing because the spaqlstore tries to import newly introduced symbols from SPARQLWrapper which are not available in the latest release. The error message is a bit misleading as it claims that SPARQLWrapper is not installed at all. (Maybe I should make the import warning more clear or remove it entirely.) Question is, what to do now? Wait with merging until the next release of SPARQLWrapper is published? |
Sorry guys, as soon as we'll fix RDFLib/sparqlwrapper#17 release 1.6.1 will be out to not block this anymore. |
SPARQLWrapper 1.6.1 has been released, but you should update this PR to reflect the last-minute changes introduced by RDFLib/sparqlwrapper#28 Please, let us know any issue with this feature or with any other you might need. |
This should work with SPARQLWrapper 1.6.1.
Unfortunately, RDFLib/sparqlwrapper#35 causes trouble. |
@uholzer that should be the single change, right. I preferred to do a last-minute change than deprecate the api later. Sorry. |
SPARQLStore: Use SPARQLWrapper for updates SPARQLStore was using its own implementation to send SPARQL Update requests. Now, it uses SPARQLWrapper's update mechanism in the same way it already uses its query mechanism.
Finally, its done. Thanks to everyone involved! |
SPARQLStore was using its own implementation to send SPARQL Update requests.
I modified it to use SPARQLWrapper's update mechanism as it already does for queries.
Remaining problems to sort out before merge:
application/x-www-form-urlencoded
or asapplication/sparql-update
. Since this has been possible with SPARQLStore, this currently breaks backwards compatibility of the API. See also Choose Content-type for Update Requests sparqlwrapper#28NSSPARQLWrapper
andTraverseSPARQLResultDOM
, but SPARQLWrapper doesn't seem to have similar capabilies.fixes #392