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

An implementation of the SPARQL 1.1 Graph Store Protocol #5

Merged
merged 9 commits into from
Aug 31, 2013
Merged

An implementation of the SPARQL 1.1 Graph Store Protocol #5

merged 9 commits into from
Aug 31, 2013

Conversation

uholzer
Copy link
Contributor

@uholzer uholzer commented Jun 12, 2013

This pull request is intended to get feedback. There are some issues to be addressed before merging:

Once this has been merged, I will work on the SPARQL 1.1 Protocol implementation, i.e. query and update.

This pull request is a part of #1.

The specification can be found here: http://www.w3.org/TR/sparql11-http-rdf-update/

Relative imports make it impossible to do

    python -n rdfextras_web.endpoint

because this runs rdfextras_web/endpoint.py as __main__.
@uholzer
Copy link
Contributor Author

uholzer commented Jul 22, 2013

I did some work today: The endpoint now makes use of the API changes to Dataset implemented in the branch graphaware from rdflib's repository.

I also added my testcase. Maybe it shouldn't be based on unittest, because it isn't really a unittest. Also, it does not give enough debugging output in case of test failure. The tests are written in a specially formatted HTML file. This should make it easy to add additional tests.

I also updated the description of this pull request to reflect ongoing development, especially in rdflib.

@gromgull
Copy link
Member

Just to let you know I've not forgotten this! I'll return to finalise the dataset changes in rdflib core shortly - and then we can get on to this.

@gromgull
Copy link
Member

I've probably just made merging this a bit trickier, by both renaming the package and by making the applications into blueprints. I am happy to do it once the time comes!

@uholzer
Copy link
Contributor Author

uholzer commented Jul 29, 2013

I seperated the the endpoint into two parts. One is generic_endpoint which does not use anything from flask and should be suitable to be integrated into any framework. The other part glues flask and the generic part together.

It comes closer to an end now, there remain only two failing tests.

About the tests: They are written as HTTP requests and responses in HTML and parsed by python for execution. I wonder whether it would be better to avoid HTML and instead write the tests in some trivial line based text format. I have some inner urge to go with a line based approach.

@gklyne
Copy link

gklyne commented Jul 31, 2013

FWIW, I tend to test server functions using direct calls to the HTTP handling code, or make using python code in my unit tests that makes calls to httplib. I think your GenericEndpoint could easily be tested using direct calls.

I recently discovered httpretty which intercepts HTTP calls at the Python sockets level for doing mock responses - there's an example of use at https://github.com/wf4ever/ro-manager/blob/master/src/MiscUtils/MockHttpResources.py (static responses only). It's worked very well for me.

uholzer added 4 commits July 31, 2013 13:45
This is a copy of
http://www.w3.org/2009/sparql/docs/tests/data-sparql11/http-rdf-update/

The idea is to parse this HTML page and run the tests it contains.
Fixed missing Accept headers

Uncommented a wrong test:
This request should fail according to the testcase. To me and to my
implementation, this request looks perfectly valid.

Fixed relative URLs as path argument

Fixed response codes:
Changed several response codes. Mostly from 200 to 204 because there
is no response body.

Fixed bad turtle syntax

Added comments indicating the changes (look for 'rdflib').
This parses all HTML files in test/endpoint_protocol and runs the
tests they contain.
@uholzer
Copy link
Contributor Author

uholzer commented Jul 31, 2013

I rebased to celan up the commit history. I guess you will have to use --force to make git fetch the branch.

Basically, I am now happy with the code. Any feedback?

@uholzer
Copy link
Contributor Author

uholzer commented Aug 3, 2013

Upcoming: Support for rdflib.Graph and rdflib.ConjunctiveGraph.

Since my PC just broke down, this will have to wait a little. Maybe I can continue work in one week.

This should make it much easier to write tests and it comes with
github flavour!
The endpoint now works with Graph, ConjunctiveGraph and Dataset or any
class implementing a compatible API.
@uholzer
Copy link
Contributor Author

uholzer commented Aug 11, 2013

Completed:

  • Support for Graph and ConjunctiveGraph
  • Switch to a simpler syntax for tests
  • Additional tests
  • All graph store tests pass

@gromgull If nobody complains, I would be happy if you could merge.

@gromgull
Copy link
Member

I've merged your changes and my blueprint changes and pushed as a new branch https://github.com/RDFLib/rdflib-web/tree/uholzer-sparql11-graphstore

Am I crazy, or does normal querying no longer work? Here https://github.com/RDFLib/rdflib-web/blob/uholzer-sparql11-graphstore/rdflib_web/endpoint.py#L89 we do g.graph.query, but g.graph is no longer set!

Did I mess up the merge somehow? It looks the same in your branch :)

@uholzer
Copy link
Contributor Author

uholzer commented Aug 13, 2013

Damn! Sorry, I messed up normal querying, forgot about it and did not run its tests. I indeed moved the the g.graph to g.ds and g.generic. I will look at it at the weekend.

Quick untested fix: Modify the following function of endpoint.py accordingly

@endpoint.before_app_request
def _set_generic():
    """ This sets the g.generic if we are using a static graph
    set in the configuration"""
    g.generic = current_app.config["generic"]
    g.graph = current_app.config["ds"]

@gklyne
Copy link

gklyne commented Aug 14, 2013

FWIW, I was looking at the N3 parser yesterday, and I noticed they seem to have a way of (nearly) wrapping a Graph in a ConjunctiveGraph:

def parse(self, source, graph, encoding="utf-8"):
     # we're currently being handed a Graph, not a ConjunctiveGraph
    assert graph.store.context_aware  # is this implied by formula_aware
    assert graph.store.formula_aware

    conj_graph = ConjunctiveGraph(store=graph.store)
    conj_graph.default_context = graph  # TODO: CG __init__ should have a
                                        # default_context arg
     # TODO: update N3Processor so that it can use conj_graph as the sink
    conj_graph.namespace_manager = graph.namespace_manager

    TurtleParser.parse(self, source, conj_graph, encoding, turtle=False)

-- https://github.com/gklyne/rdflib/blob/master/rdflib/plugins/parsers/notation3.py#L1749

#g

@gromgull
Copy link
Member

I guess I can also change g.graph to g.generic.ds - a quick test shows it works! :)

@gromgull
Copy link
Member

@gklyne : that snippet of code I have previously considered too ugly to live and I have a branch working towards streaming parsers where it is removed :)

@uholzer
Copy link
Contributor Author

uholzer commented Aug 18, 2013

@gromgull: Yes, your solution works.

Unfortunately, a test in test_endpoint_protocol.py (after fixing the includes) fails with

rdflib_web.endpoint: ERROR: Exception on /graph-store [POST]
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/flask/app.py", line 1687, in wsgi_app
    response = self.full_dispatch_request()
  File "/usr/lib/python2.7/dist-packages/flask/app.py", line 1360, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/usr/lib/python2.7/dist-packages/flask/app.py", line 1358, in full_dispatch_request
    rv = self.dispatch_request()
  File "/usr/lib/python2.7/dist-packages/flask/app.py", line 1344, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/home/urs/p/RDFLib/rdflib-web/rdflib_web/endpoint.py", line 129, in graph_store_indirect
    return graph_store_do(None)
  File "/home/urs/p/RDFLib/rdflib-web/rdflib_web/endpoint.py", line 117, in graph_store_do
    accept_header=request.headers.get("Accept")
  File "/home/urs/p/RDFLib/rdflib-web/rdflib_web/generic_endpoint.py", line 157, in graph_store
    url = self.coin_url()
  File "/home/urs/p/RDFLib/rdflib-web/rdflib_web/endpoint.py", line 148, in <lambda>
    coin_url=lambda: url_for("graph_store_direct", path=str(rdflib.BNode()), _external=True)
  File "/usr/lib/python2.7/dist-packages/flask/helpers.py", line 361, in url_for
    return appctx.app.handle_url_build_error(error, endpoint, values)
  File "/usr/lib/python2.7/dist-packages/flask/helpers.py", line 354, in url_for
    force_external=external)
  File "/usr/lib/python2.7/dist-packages/werkzeug/routing.py", line 1620, in build
    raise BuildError(endpoint, values, method)
BuildError: ('graph_store_direct', {'path': 'Nf9823305f509465fba670bf19f82e5e5'}, None)

Do you have an idea what the problem could be?

@uholzer uholzer merged commit ccbce7f into RDFLib:master Aug 31, 2013
@uholzer
Copy link
Contributor Author

uholzer commented Aug 31, 2013

I got the tests of the old funcitonality working again. My new tests for the graphstore protocol still don't succeed, but this can be solved at a later point.

Since the former tests work now and because this pull-request is now lingering around for too long, I take the freedom to merge this into master.

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.

3 participants