Review of "SPARQL 1.1 RDF Dataset HTTP Protocol"

v1.69

[***] Critical: Important to fix
[**] Major
[*] Minor or editorial
[] Aside or comment

== Summary:

The document is in good shape.  I found 2 critical points, neither of 
which are large:

  1/ fixing the <h1>
  2/ removing the red box (all issues must be addressed for LC)

Other than that it is publishable as-is for LC - everything is covered 
so that it meets the LC criteria of "no outstanding issues".

Ideally, at least the major points should be addressed. Of those, the 
most useful is a suggestion to add some examples of the protocol in 
action into section 3 (Protocol model), which also helps address the KK 
point about needs of implementers.

Also, it would be nice to have an example of the protocol in each 
operation description, before the equivalent SPARQL Update.

== All docs:

[**] List of documents
We need some std text for all docs.
Or the overview doc needs provide it so other docs don't need to.

As query is large and may be accessed directly, I'm inclined to put some 
doc suite text even if its in the overview.

----------------------

== General Points:
[**] service or a graph store

In
http://example/x?graph=...
is <http://example/x> a service or a graph store?

In the final section, it is very definitely a service.  But (e.g.) 
section 3 does not mention service and says "route native HTTP 
operations to a Graph Store" which, in the absensce of anything else, 
implies to me it's naming the graph store.

[**]
Is a Compliance section required by W3C process?  especially as we will 
have tests. I think we need one by CR, and ideally for LC but a 
placeholder in LC will do.

== Title

[***] The <h1> is wrong (the <title>is right).

== Section: Abstract.

[*] Don't define the doc in terms of other documents.  Simply state what 
it is.

Drop the first 2 sentences.

"The protocol described here is meant to provide a minimal set of 
uniform, ..."
=>
"This document describes the use of HTTP verbs applied to managing a 
collection of graphs in the REST architectural style."

== Section 1

[] When seeing a list like this I expect, somewhere, to see either text 
later tying back to it (e.g. conclusion) or expansion of the points to 
say how the doc meets the claims.

[] "It emphasises the distinction between" => "It emphasises the 
following distinct concepts"

It reads oddly to say "distinction" then see some things that are 
obviously different.

[*] "*MUST* ....and the words appear as emphasized text, "

The document does not use them in this way. They are not in bold later, 
and bold is reserved for HTTP Verbs.
SmallCaps might be useful to emphasize.

[*] It's "SPARQL 1.1 Query", not "SPARQL 1.1/Query" and similarly for 
other docs.
This occurs in other places as well (e.g. sec 8)

== Section 2:
[*] "Resolvable URI": potentially has a representation.
Might only be able to PUT/POST to it only so no representation.

[] Serialize (verb)
Does not work for me.

A document is a serialization of a graph (noun)

An alternative use of the verb Serialize is the action of doing it, and 
that's done by the application code/CPU.

"A document serializes graph." does not seem right to me if read as the 
document is carrying out the action.  In fact, the document avoids such 
a phrase and says:

"document ... which serializes ... graph."
"document ... that serializes ... graph."
which isn't a simple subject-verb-object sentence.
and "graph serialized by document" occurs as well.

[**] Graph Store
It's not necessarily a single service.
e.g. SPARQL Update and SPARQL HTTP Dataset Management services on one 
graph store.

[**] RDF Graph Content: "named graph" -> what about default graphs?

[] "Semantic" - does not add anything - omit.

== Section 3 Protocol Model

[**] Some text and some examples here of the protocol in action would 
address the KK comment about speaking to implementers

[**] "compliant"
There is no compliance section in the document.

Section 4:
[] "(intuitively)"
Remove - does not add anything, but breaks the flow.

[*] "In this case " => "In this case, " (comma)
to match style elsewhere in doc.

[*] "An HTTP request can route"

Isn't it the server that does the routing of a request?
At least "can be routed"

[*] "attempt to emphasise" => "The diagram below shows this ..." or even 
straight to
"The diagram illustrates ..."

[*] "most basic" - it's basic or it isn't.

The diagram looks general to me for direct references.
Direct references aren't "basic" compared to indirect.

== Section 4.2
[] First sentence is 4.2 lines long in my print out.  Reword.

[**] "HTTP listener"
Not sure about this. Is it an official term?  I could not find it with 
Google.

Suggest "service", "processor", even "graph store containing the graph"
"service" is used later in the section.

[*]
"(and default)" - remove () - "named or default graph"

[*] "is a a representation" => is a representation" (doubled "a")

== Section 5:

[] "Where an equiv SPARQL Update op is given"
It always is, except for HEAD.

[*] Suggestion: Add example the HTTP operation before the SPARQL Update 
equivalent.  Speaks to KK comment about implementers.

PUT /service?default  HTTP/1.1
Host: example
Accept: application/rdf+xml
=>
DROP SILENT DEFAULT;
INSERT DATA {  .. RDF payload .. }

[*] Exmaple of *SHOULD* (bold) / SHOULD (unbold) to avoid confusion with 
*PUT* bold.

[*]
"For requests that use HTTP verbs not listed here"
Just don't say anything and leave to HTTP.
Maybe some webdav verbs are meaningful. An extended implementation 
should still be compliant.
And it conflicts in spirit of "additional details of appropriate 
behavior beyond those specified here"

[] Make "Status Codes" discussion a section on it's own - it's worth a 
section header

== Section 5.1 PUT
[*] "be be" => "be"

[**] "Whether the DROP is necessary"
if empty graphs are recorded => content exists (the empty RDF graph).
Suggest "DROP is needed to remove any previous RDF content"

[**] Maybe a general discussion of empty graph in Sec 5 start.  Section 
on status codes, section on empty graphs.

== 5.5 POST
[**] "incorporate" reads to me as an open process "RDF merge" is the 
specific operation.

[*] 201 Created.
This is a different style from other status code which is (plain)"NNN 
(Common Text)" (I prefer the style here of <code>200 OK</code> FWIW)
And later in sec 7 it has "401 status code (Unauthorized)

[***] Red Box
Address, remove or anything to get rid of redness by LC.
Else it's not a LC.

[*] "alternative behavior" of POST
To me it's not "alternative", (what's it an alternative to?)
It's a specific behaviour for something that REST leaves open to definition.

[] Text after red box.
Make first part specific to POST

[] "that serializes" see above.

== Sec 5.4 HEAD

[**] HEAD is metadata about the response and representation, not the RDF 
content.

Terminology has "RDF graph content", and that's what the IRI names, not 
the representation.

[] "As described" - what? where?
? here or HTTP RFC2616?

== Sec 5.5 PATCH

[*] "doesn't modify a single graph"
has two meanings
1 - request mentions only one graph or 2 - makes changes to only one 
graph, but attempts to change many with no visible effect.

What about modifies no graph?

e.g.
DROP SILENT <doesnotexists1> ;

does not modify(=change) a single (i.e. any) graph.  Ditto INSERT DATA 
of already present triples.

== Section 6:
[*] I'm not sure what this section is adding. It seems to say "do the 
right HTTP thing" which would be better places at the front (sec 3 
maybe) as a principle.  Then the section can be dropped.

== Section 7:
[*] First sentence:
This should be about security issues, not that it should use security 
features. The security section is a requirement of REC docs, I think, 
and has some proforma requirements.

== Section 8:
Interesting (to me) but unnecessary in the spec and a hostage to 
fortune. The doc works without it.  If its necessary, then it should be 
at the start anyway.

[] OPTIONS only mentioned here

== Section 9;
[*]
  [SPARQL] is used in sec 4.1 but not mentioned as a reference.

[*] Surely [SPARQL-UPDATE] is normative because it provide the meaning 
of PUT etc. specifically on a graph store.

[] Can we have links from the [RFC3986] etc to the reference entry please?

Received on Friday, 4 February 2011 09:06:25 UTC