-
Notifications
You must be signed in to change notification settings - Fork 44
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
Specify PATCH #85
Comments
Reminder to discuss: What to do about the |
This comment has been minimized.
This comment has been minimized.
Just wanting to post this somewhere as a note to self, since we need to specify this behaviour, which is mostly correct:
|
@dmitrizagidulin IMO, we shouldn't bother to bring along a propriety header like @kjetilk Anything in particular that's incorrect or inadequate? I'm assuming the key things in that example is the method and the content type. |
IIRC, @timbl was OK with
Basically, mainly that this is a LDP-NR, and it can't be edited with SPARQL, so it shouldn't be there in this case even if it is there for LDP-RS. Also, with ref to #118, the But the observation that |
Isn't Wouldn't updating |
Actually... Yes! /me takes note to read RFC5789 in full... :-) Sorry.
Yes, it was the distinction between LDP-RS and LDP-NR that was the important part of my comment. |
A bit more on the HTML and XML family: https://tools.ietf.org/html/rfc7351 and https://tools.ietf.org/html/rfc5261 is one way to have Someone can correct me on this but AFAIK I don't think the spec needs to bring any attention to LDP-NR or LDP-RS. Simply, if server allows certain methods cross content type for a resource, it can be known through |
OK, so something around "If the server supports |
Like I said, already in LDP: https://www.w3.org/TR/ldp/#ldpr-patch-acceptpatch
I think that's just RFC 7231. |
Yeah, but... Perhaps I should defer commenting until I have caught up on RFC5789, but it seems terribly underspecified; It doesn't seem useful at all to know that a server supports a certain patch media type, for it to be useful, it needs to be known for each media type, or at the very least for each interaction model. |
Caught up on RFC5789, and I think LDP is awkward at that point. I think it makes zero sense to add
I think we need to tighten this up a bit, but also make clear that you don't need |
I don't quiet follow what you're saying or want. LDP inherits RFC 5789. Why say the opposite with not needing Tighten up how for instance? Specify or clarify something (non-normative)? Change the level of requirement somewhere? By the way, see also #43 .. basically no new semantics is needed for |
I'll try to clarify. The most important point being that the case of
So, when LDP says:
The implication of an unqualified "supported by the server" is that it applies to So, no we shouldn't break RFC5789 (that one is in line with my intuition), but break LDP because LDP is being silly. Moreover, I think we should respond with So, I'd rather prefer we adopt RFC5789's language, something like " Alternatively, we could do it just for read requests and Given LDPs silliness, we might relax the requirement somewhat from LDP, e.g. " This would allow implementors to stay conformant with LDP if they take it literally and has to, but allow Solid implementors to not spend any time on that. (I would have said "MAY NOT", but that's not in RFC2119). |
re 'unqualified "supported by the server"', I think I see what you mean but I'm not sure if there is any severe violation. I agree that one way of interpreting the LDP text is that it missed For the solid spec, I would suggest extending @TallTed 's proposed: https://github.com/solid/solid-spec/pull/103/files#diff-bc6e7d60c7ea3d7165eb78a87a94b626R368-R371 with the information he also gave in nodeSolidServer/node-solid-server#628 (comment) . That seems to cover what you're saying, right?
There are probably similar cases. So, raised #123 .
Which is a good enough reason to leave it optional.
I don't think that's needed. The context is |
Not sure it addresses all my concerns, we need to work out the exact wording. There are two important concerns that I'm not confident about: One is that we should make sure the client doesn't need more roundtrips than necessary, and so MUST OTOH, we certainly don't want to burden implementors with implementing useless stuff. So, for
they shouldn't need to respond:
because that's of very little use. In my reading, LDP's MUST requires them to do that, and we should make sure Solid implementors can safely ignore that and remain Solid compliant (but also, allow LDP implementors who do that be Solid compliant). I know that I would totally ignore that MUST if I wrote a Solid server. :-) |
First of all, I don't see LDP's Having said that, I haven't made use |
The MUST makes it mandatory, the SHOULD doesn't, which makes it a compliance issue. Simple, it might be, but it is still an issue. There is still actual time that has be allocated. I am not advocating a MUST NOT, I merely want language to make it so that Solid implementors wouldn't have to to do it to be compliant. The burden of proof should be on those advocating a MUST, so why do you think MUST is important? Say, a client wants to patch
find
realize it has to then figure out from heuristics which one applies to Or it would go I would really like to see a justification for MUST in this case, why is it worth the effort. I just can't see any use for it. MUST on |
I'll defer to LDP. I think the point is if Solid is generally inheriting LDP but with extensions and changes, we need to provide a reason for the changes which may be in conflict. What we could perhaps do re https://www.w3.org/TR/ldp/#ldpr-patch-acceptpatch is i) be certain about LDP's intention, or ii) clarify whether it is for all
Why? A conforming client would know that request ( https://tools.ietf.org/html/rfc7230#section-5.3.4 ) is for server's capabilities as opposed to |
I have a different philosophy: We are making something that should be without obvious overhead, and so, we should question everything that adds overhead without actual value.
If LDP is sufficiently precise, we wouldn't need to put down extra effort, and if LDP is imprecise, like in this case, I suggest that we instead look to use cases. So, I suppose LDP meant for specific targets, but that it had insufficient understanding of the
Yes, and in which case |
[
No objection from my end but I don't believe that was the approach we took or agreed on. If you want to revisit that, we can, perhaps in chat or at the next F2F?
So, why can't we just clarify meanwhile stay compatible with the specs? I don't think that we should introduce a new/different criteria around this. |
Yeah, lets discuss some differences in philosophy in the editorial call! |
@kjetilk wrote --
If the client knows it "wants to patch The (potential) inefficiency you're flagging would not be due to the requirements placed on the server by either existing HTTP RFC or LDP Spec The utility of an In the LDP WG, we tried very hard to not restate things that were already stated in existing RFC or other spec which LDP inherited. That effort may have meant that we left out details that should have been included. We clearly did not always succeed in clarifying existing ambiguities. All that said... A |
Yes, that would be strongly preferable. Do we have time?
But it is a superset of the functionality of |
I think this discussion is about a non-issue. The
The MUST constraint that LDP adds is only for LDP-Resources (it's in section 4.2). Meaning it also applies only to actual resource requests. On the other hand, the
Therefore, the must in LDP does not force servers to add |
@JordanShurmer Yeah, I agree with all this, my problem was the LDP in 4.2.7.1 doesn't mention resource. |
We're moving towards recording consensus on this, so just to summarize what is needed:
The rest seems to follow from LDP and other RFCs. |
@kjetilk all of the section 4 sub-sections apply only to resource requests. From the non-normative introduction, which is not normative but can help resolve ambiguities:
More importantly, 4.2.7.1 explicitly refers only to ldp resources:
|
Hmmm, right, that's a good point. |
Wrote an overview of what we need to consider for SPARQL Update in #125 , and it got pretty lengthy. |
Jotting this down for later: One criteria to emphasise in the Solid spec is PATCH's atomicity requirement. It is sufficiently clear in RFC 5789:
and what that entails. The Solid spec will have a global atomicity requirement ( #137 ) on creating nested containers when necessary and error if not possible. So, the requirements are well aligned. Potential errors should be reported from general (can create nested containers?) to specific (can modify resource?) |
I think that to advance this, we need to free ourselves from the requirements of SPARQL Update, as the number of issues to be settled there is pretty large, and it is best done in a panel. So, nevermind my list above, what do we really need to agree on to find rough consensus on this one? |
We agree on PATCH being required. It may require one or more specified ways to perform an update. If/when SPARQL Update and other approaches are cleared, the PATCH section can refer to them. There are other relatively minor things that's in agreement eg. PATCH appears in Allow, Allow-Patch in some places (I think for the sake of uniformity, it should also be in OPTIONS * but this is not a show stopper for rough consensus right now), atomicity expectations, .. There are no particular restrictions on the kind of resources that would allow PATCH eg. containers and regular resources can be PATCH'd. It is only the formats that's of a concern that can be addressed. |
Right, and since we have freed ourselves somewhat from the shacles of LDP since we had most of the discussion, I think we can go for something like
Then, we need to say something around atomicity. Is that it? Can we move to rough consensus? |
No description provided.
The text was updated successfully, but these errors were encountered: