Issue/conneg by splattater · Pull Request #71 · AKSW/QuitStore

splattater · 2017-10-20T08:02:13Z

Fixes #68

coveralls · 2017-10-20T08:05:40Z

Coverage decreased (-0.5%) to 53.445% when pulling 2e374ba on issue/conneg into 36193f9 on feature/provenance.

coveralls · 2017-10-20T09:54:53Z

Coverage increased (+5.9%) to 59.91% when pulling 1519249 on issue/conneg into 36193f9 on feature/provenance.

white-gecko

Please squash.

white-gecko · 2017-10-23T12:17:09Z

quit/web/modules/endpoint.py

-queryTypeRegex += r"INSERT|DELETE|CREATE|CLEAR|DROP|LOAD|COPY|MOVE|ADD"
-queryTypeRegex += r"))"
+queryTypeRegex = r"""
+    (?P<queryType>(CONSTRUCT|SELECT|ASK|DESCRIBE|queryTypeRegexINSERT|DELETE|


there is a superfluous queryTypeRegex

white-gecko · 2017-10-23T12:32:40Z

quit/web/modules/endpoint.py

                    response.headers['Content-Type'] = 'text/html'
                    return response
-                elif mimetype in ['application/json', 'application/sparql-results+json']:
+                elif mimetype in ['application/sparql-results+json']:


https://www.w3.org/TR/sparql11-results-json/

Please support application/json and application/sparql-results+json

white-gecko · 2017-10-23T12:33:17Z

quit/web/modules/endpoint.py

-                elif mimetype in [
-                    'application/rdf+xml', 'application/xml', 'application/sparql-results+xml'
-                ]:
+                elif mimetype in ['application/sparql-results+xml']:


Please also accept application/xml here.

white-gecko · 2017-10-23T12:34:35Z

quit/web/modules/endpoint.py

+                    response = make_response(res.serialize(format='xml'), 200)
+                    response.headers['Content-Type'] = 'application/rdf+xml'
+                    return response
+                elif mimetype in ['text/turtle']:


please also support the old turtle mime type: application/x-turtle

white-gecko · 2017-10-23T12:35:25Z

quit/web/modules/endpoint.py

+                    response = make_response(res.serialize(format='nt'), 200)
+                    response.headers['Content-Type'] = 'application/n-triples'
+                    return response
+                else:


n-quads is missing.

white-gecko · 2017-10-23T12:37:47Z

quit/web/modules/endpoint.py

+                    response.headers['Content-Type'] = 'application/n-triples'
+                    return response
+                else:
+                    return make_response('', 406)


Maybe this could be replaced by some dictionary:
mimetype -> serializername

the response header content-type should correstpond to the best matching requested content type which was selected for serialization.

coveralls · 2017-10-24T11:14:22Z

Coverage increased (+5.01%) to 59.141% when pulling d53aa4c on issue/conneg into e4e8477 on feature/provenance.

coveralls · 2017-10-24T11:14:22Z

Coverage increased (+5.01%) to 59.141% when pulling d53aa4c on issue/conneg into e4e8477 on feature/provenance.

coveralls · 2017-10-24T11:14:23Z

Coverage increased (+5.01%) to 59.141% when pulling d53aa4c on issue/conneg into e4e8477 on feature/provenance.

white-gecko · 2017-10-24T13:31:42Z

quit/web/modules/endpoint.py

+            return render_template('provenance.html')
+
+
+def create_result_response(res, querytype, mimetype):


change signature to create_result_response(res, mimetype, mimetypes[mimetype])

white-gecko · 2017-10-24T13:32:14Z

quit/web/modules/endpoint.py

+
+            # Set default
+            if mimetype == '*/*':
+                mimetype = 'text/turtle'


white-gecko · 2017-10-24T15:28:21Z

quit/web/modules/endpoint.py

+queryTypeRegex = r"""
+    (?P<queryType>(CONSTRUCT|SELECT|ASK|DESCRIBE|INSERT|DELETE|
+    CREATE|CLEAR|DROP|LOAD|COPY|MOVE|ADD))
+"""


Are you sure the regex is still the same?

splattater · 2017-10-27T09:22:25Z

Will be rebased and squashed after final review

white-gecko

Some minor changes

white-gecko · 2017-10-30T10:51:39Z

quit/web/modules/endpoint.py

 def parse_query_type(query):
    try:
        query_type = queryTypePattern.search(query).group("queryType").upper()
+        return query_type


you can make this:
return queryTypePattern.search(query).group("queryType").upper()

white-gecko · 2017-10-30T10:52:49Z

quit/web/modules/endpoint.py

-                    return response
-            else:
+        if query_type in ['SELECT', 'ASK']:
+            querytype = 'query'


Having query_type and querytype as two different variables is misleading.

white-gecko · 2017-10-30T10:56:58Z

quit/web/modules/endpoint.py

+            dict_part = mimetypes[querytype][mimetype]
+            return create_result_response(res, dict_part)
+        except KeyError as e:
+            return make_response('Mimetype: ' + mimetype + ' not Acceptable', 406)


Maybe you change this to a format(mimetype).

coveralls · 2017-11-01T12:16:39Z

Coverage increased (+5.4%) to 58.999% when pulling 8088b93 on issue/conneg into 6ea54e0 on feature/provenance.

coveralls · 2017-11-01T12:28:31Z

Coverage increased (+5.4%) to 58.985% when pulling edec8ff on issue/conneg into 6ea54e0 on feature/provenance.

coveralls · 2017-11-01T12:28:31Z

Coverage increased (+5.4%) to 58.985% when pulling edec8ff on issue/conneg into 6ea54e0 on feature/provenance.

Add more supported mime types. Distinguish mime types between Select/Ask and Construct/Describe. Update default mime types. Add HTTP responds on error. Add conneq tests. Add a dictionary containing accept type, content type and rdflib serialization format for both, CONSTRUCT/DESCRIBE and SELECT/ASK queries. Add a method that creates responses in the requested serialization. This method deduplicates code of sparql and provenance method/path. Add raise of UnSupportedQueryType Update exception handling Update app versioning test Add endpoint test class Add test for parse_query_type method.

coveralls · 2017-11-01T12:47:48Z

Coverage increased (+5.4%) to 58.985% when pulling 696aa32 on issue/conneg into 6ea54e0 on feature/provenance.

splattater requested a review from white-gecko October 20, 2017 08:02

white-gecko requested changes Oct 23, 2017

View reviewed changes

splattater force-pushed the issue/conneg branch 2 times, most recently from 820038e to d53aa4c Compare October 24, 2017 11:07

white-gecko requested changes Oct 24, 2017

View reviewed changes

white-gecko reviewed Oct 24, 2017

View reviewed changes

white-gecko requested changes Oct 30, 2017

View reviewed changes

white-gecko added this to the feature/provenance milestone Oct 30, 2017

splattater force-pushed the issue/conneg branch from 2b2639a to 8088b93 Compare November 1, 2017 12:10

white-gecko approved these changes Nov 1, 2017

View reviewed changes

splattater force-pushed the issue/conneg branch from edec8ff to 1859cfc Compare November 1, 2017 12:31

splattater force-pushed the issue/conneg branch from 1859cfc to 696aa32 Compare November 1, 2017 12:40

white-gecko merged commit 0f760d9 into feature/provenance Nov 1, 2017

white-gecko deleted the issue/conneg branch November 1, 2017 12:57

		return render_template('provenance.html')


		def create_result_response(res, querytype, mimetype):

Conversation

splattater commented Oct 20, 2017

Uh oh!

coveralls commented Oct 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Oct 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

white-gecko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Oct 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Oct 24, 2017

Uh oh!

coveralls commented Oct 24, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

splattater commented Oct 27, 2017

Uh oh!

white-gecko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Nov 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Nov 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Nov 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Nov 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coveralls commented Oct 20, 2017 •

edited

Loading

coveralls commented Oct 20, 2017 •

edited

Loading

coveralls commented Oct 24, 2017 •

edited

Loading

coveralls commented Nov 1, 2017 •

edited

Loading

coveralls commented Nov 1, 2017 •

edited

Loading

coveralls commented Nov 1, 2017 •

edited

Loading

coveralls commented Nov 1, 2017 •

edited

Loading