Re: Review of Graph Store Protocol (action 564)

Thanks for the review. See my response inline below:

On Friday, December 2, 2011 at 7:09 AM, Andy Seaborne wrote: 
> == Comment 1
> 
> Sandro write:
> > title
> > 
> > I know we went thought a long WG decision process (twice) to
> > arrive at the current title, but in actually talking about this
> > document to a few people, I find the only way to have it make
> > any sense is to use the word "RESTful". So, I propose we
> > amend the title to:
> > 
> > SPARQL 1.1 Graph Store (RESTful) HTTP ProtocolI support this suggestion.
> 
> Or even
> 
> "SPARQL 1.1 RESTful Graph Store Protocol"
> 
> Reading the doc, it says "REST" in sentence 1 of the introduction.
> 
> Saying "REST" conveys HTTP to me so both aren't needed.

I'm in agreement with the suggestion and the general sentiment that the RESTful is the most concise way of describing the intent of the protocol. However, I recall that there was some resistance by members of the WG with the use of that word. Perhaps I'm mistaken in recalling how this conversation played out. So, I would prefer we ensure we have consensus within the WG before making yet another change to the title. I don't, however, think of this as a substantive change.
> == Comment 2
> == Introduction
> = Editorial/Not required for LC/required for CR
> 
> The list of other SPARQL documents is incomplete (no JSON results, no 
> entailment). I thought we were going to use standard boilerplate in all 
> docs to reference the set of documents.
Changed.
> == Comment 3
> 
> My preference would be to use
> 
> Accept: text/turtle; charset=utf-8
> 
> (this is already registered).
> 
> for
> 
> Accept: application/rdf+xml
I have made this change consistently as I notice the only place where the payload is listed, turtle is used anyways.
> == Comment 4
> == IRI vs URI
> 
> = Section 4.1
> 
> The picture does not mention IRI but it's (IRI, RDF) element of graph store.
This has been changed: s/URI/IRI. This has also been reflected across the entire document, where appropriate.
> = Section 4.2
> "where the IRI is the result of percent-decoding"
> but the percent-encoding is about the ":"
> 
> Suggestion: an example with an accented character.
I don't feel this really adds to the example. 
> "In the example above, the embedded graph IRI" => confusing
> 
> "embedded" => "encoded"
Changed. 
> "embedded URI"
> => "query string URI"
Changed.
> == Comment 5
> "If the Accept header is not provided... the server SHOULD return RDF XML"
> 
> I prefer should "return one of RDF XML, Turtle or N-Triples". Turtle is 
> proving more acceptable (pun) as a serialization of RDF.
Changed. 
> == Comment 6
> Order of operations.
> 
> Could we have HTTP GET first? It's an operation that applies even if 
> the others are not allowed.
I agree and looking at the sequence now, it seems to follow the order of (what I imagine would be) decreasing usage (roughly).
> == Comment 7
> = section 5.2
> Bold DROP ==> <tt>DROP</tt>
> 
> Bold is used for the HTTP verbs in this section.
Changed. 
> == Comment 8
> = section 5.2
> 
> "If the request body is empty"
> 
> If it's content-type: rdf+xml, then an empty request is illegal by 
> definition of the content type. We should not be rewriting HTTP conneg 
> rules.
I have changed the text to add text ensuring that the empty body is appropriate for the indicated content-type and what to do if that is not the case:

"Note, this option is only relevant for situations where an empty body is appropriate for the indicated content-type. Otherwise, as described in section 5.1, a 400 Bad Request should be returned."

> == Comment 9
> = section 5.3
> "In the event the operation is overridden, ..."
> I don't understand who is overriding the operation.
This caveat is inherited from RFC 2616 (Hypertext Transfer Protocol -- HTTP/1.1)



"This method MAY be overridden by human intervention (or other means) on the origin server. The client cannot be guaranteed that the operation has been carried out, even if the status code returned from the origin server indicates that the action has been completed successfully." -- http://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html#sec9.7
> == Comment 10
> == section 5.4
> = Multipart.
> 
> Make this 5.4.1 or place as subsection earlier because it applies to PUT 
> as well, presumably.
So I began to move this to a section on its own (5.2), but it doesn't seem immediately obvious that this applies to PUT. Indeed, RFC2388 is not specific to any transport method or HTTP method. However, it seems to focus only on using this with forms and the HTML form/@method attribute doesn't allow any other values besides GET and POST and the former doesn't seem appropriate for the usecase that motivated the addition of support for multipart data.  So, I have left this as is.
> == Comment 11
> == Discussion of service description
> 
> Accept: application/rdf+xml
> 
> is followed by Content-Type Turtle.
> 
> s!Accept: application/rdf+xml!Accept: text/turtle; charset=utf-8!
The use of turtle for the content-type in examples consistently across the document has addressed this
> == Comment 12
> == End section 5.4
> 
> Include query over POST in POST operations.
The text in this section has been changed to reflect this.
> == Comment 13
> 
> [SPARQL] reference is to SPARQL 1.0.
> 
> Some RFC references mention a URL but it's not a link.
This has been addressed for the lone RFC2616 reference. 
> Some RFC references don't spell out the URL.
Do you mean the URL directly to the relevant section? If not, I've gone through and ensured all mentions of RFCs point to the relevant reference at the end of the document.

-- 
Chime Ogbuji
Sent with Sparrow

Received on Tuesday, 6 December 2011 00:11:30 UTC