Re: PATCH proposal

Lisa Dusseault wrote:

>> here are my updated comments. I think they should be addressed before  
>> this can go to a last-call:
> 
> 
> I have made a few changes based on this latest round of comments, but I  
> feel they are minor enough to be submitted in the Author's 48 hours --  
> e.g. replacing the word "standard" with "specification" or "draft"  
> where standard is inaccurate, removing the unused reference.  Some of  
> these comments are the same comments as before, and I had already  
> attempted to address most of these.   Details follow inline.

Lisa,

I think this draft should go through a mailing list last-call; and it 
should also not be submitted until open issues are resolved. Doing it 
after submission simply creates a lot of additional work in the 
publication process.

>> C-03-02, Section 2
>>
>> "This specification only defines usage of the platform-portable gdiff
>> [9] format identified as 'application/gdiff'."
>>
>> As far as I can tell, there is no registration for  
>> "application/gdiff". Options:
>>
>> 1) do the registration in this RFC (appendix)
>> 2) have a separate RFC for the gdiff format, including a registration
>> 3) workaround the issue by saying that in the absence of a MIME type,  
>> the gdiff
>> format is assumed
>> 4) ...?
>>
>> (Personal preference: #1)
>>
>> -04 update: I hear that Lisa is trying to get the MIME type  
>> registered. I
>> don't think this will work as long as the documentation only resides  
>> in a
>> W3C note. If PATCH is really to become an IETF standards-track document
>> at some point of time, normative parts of it (like the GDIFF  
>> documentation)
>> need to live in an acceptable place; I don't think a vendor note to  
>> the W3C
>> is suitable; for instance, what do we know about IP issues with it?
> 
> 
> If the IANA says that a W3C Note is not acceptable, then I'd certainly  
> act on that.  But we don't need to resolve all IP issues precisely  
> because GDIFF is not proposed as an IETF standard or specified in a  
> standards-track IETF document.

Well, if PATCH requires support for GDIFF, it's part of the PATCH spec 
(by normative reference).

>> C-03-04, Section 3.1
>>
>> "In the model defined in RFC3230 [5], the patch document is modelled
>> as an instance being sent to the server...."
>>
>> It's unclear whether we are reusing RFC3250 or not. As far as I can  
>> tell,
>> this spec is independant of RCF3250, so I'm not sure what this  paragraph
>> is trying to define.
> 
> 
> As I've said, this paragraph defines how a client can be compliant with  
> both PATCH and RFC3230.  If it's not specified whether the PATCH  
> document is itself an instance or an instance manipulation, clients and  
> servers could have difficulty interoperating if they implemented both  
> PATCH and RFC3230.

I appreciate if this is clarified; maybe the paragraph needs to be 
expanded and/or moved into a separate section (interdependencies with 
other specs)?

>> C-03-07, Sectionb 3.2.1
>>
>> "The server SHOULD provide a MD5 hash of the resource entity after the
>> delta was applied.  This allows the client to verify the success of
>> the operation.  The PATCH method MUST cause the ETag to change if the
>> resulting entity is not identical to the original.  If the server
>> supports strong ETags, the server MUST return a strong ETag for use
>> in future client operations.  The server SHOULD return the
>> Last-Modified header in any case, but the server MUST return the
>> Last-Modified header if ETags aren't supported."
>>
>> I don't like the interdependencies here. Why not simply say, in  addition
>> to the response headers that would be generated for a PUT request on  
>> that
>> resource, the server SHOULD also provide Content-MD5.
>>
>> Speaking of which; it would be nice if this spec could define a  standard
>> WebDAV property holding the content MD5, sich as DAV:getcontentmd5.
> 
> 
> Section 3.2.1 seems clear to me, perhaps you could be more clear what  
> you mean about "don't like the interdependencies here"?  The text about  
> returning a strong ETag is important and not redundant or  
> interdependent with the text about the MD5 hash.

I feel the current wording is problematic in that it is a cascade of 
conditional statements. Whatever makes this simpler and basically 
provides the same would be preferrable; thus my proposal.

> Defining new WebDAV properties is out of scope; PATCH does not depend  
> on WebDAV.  I would welcome specification of such a property elsewhere.
 >
>> C-03-08, Section 3.2.2
>>
>> 2) Make this optional for servers that do not care about WebDAV.
> 
> 
> This section has nothing to do with support for WebDAV on either side.   
> HTTP clients supporting PATCH can use error messages in the body just  
> as well as WebDAV clients do.  I do not agree with making it optional  
> at any rate; it's easy for the server, optional for the client, and  
> promotes interoperability.

As developer of WebDAV servers, I don't have any problem with that. 
However, I feel that there are people out there who'd prefer to have 
zero dependencies on WebDAV or related specs; and IMHO we should respect 
that.

>> 3) Put the condition names into a different namespace, unless the  
>> WebDAV WG
>> agrees to adopt these names (for instance use, urn:ietf:rfc:xxxx).
> 
> 
> The WebDAV WG has many chances to review this draft and object to the  
> use of the DAV: namespace.  I interpret silence as consent to adopt  
> these names.

With all due respect: I'm a member of the WebDAV WG and I have objected 
to this. This is *not* 'silence'! If you want to use the DAV: namespace, 
send an *explicit* request to the mailing list and try to get consensur 
in favor of it.

>> 3) Stick to RFC3253's terminology (the names identify conditions, not  
>> errors, thus
>>
>> s/DAV:delta-format-unsupported/p:delta-format-supported/
>> s/DAV:delta-format-forbidden-on-resource/p:delta-format-allowed-on- 
>> resource/
>> s/DAV:delta-format-badly-formatted/p:delta-documented-well-formatted/
>> s/DAV:delta-empty-resource/p:delta-format-applies-to-empty/
>> S/DAV:patch-result-invalid/p:patch-result-valid/
> 
> 
> As you know from this and other specs, I am sticking to the HTTP  
> approach, describing the error rather than the condition that would be  
> a lack of an error.  Describing the error is far more natural (in many  
> protocols, not just HTTP) and promotes readability of the spec and of  
> the error responses when they're encountered by programmers or  analysts.

Once you pick an existing protocol element - stick with it's semantics. 
I'm not saying that identifying errors instead of conditions by itself 
is bad; but it's inconsistent with what RFC3253 (from which you borrow) 
defines. If you don't like the semantics of a protocol element, by all 
means do NOT reuse it with silently changed semantics.

>> C-03-09, Section 3.3
>>
>> "OPTIONS * is not used to advertise support for PATCH because the
>> delta formats supported are likely to change from one resource to
>> another.  A server MAY include the Accept-Patch header in response to
>> OPTIONS *, and its value MAY be the union of known supported delta
>> formats."
>>
>> I think that if this paragraph is removed, the spec still says the same
>> thing. It doesn't add anything that doesn't already follow from  RFC2616.
>>
> 
> What "follows from RFC2616" differs for many people.  I added this  

(shouldn't)

> paragraph because there was lots of confusion and discussion about the  
> appropriateness of OPTIONS *.  A reader of this spec shouldn't need to  
> read the entire mailing lists to understand how OPTIONS * might work in  
> this situation.  Besides, this helps servers that do support OPTIONS *  
> do so in a consistent manner -- showing the union of supported delta  
> formats, rather than some other value in the Accept-Patch header.

I still think this is a mistake. Following your line of argument, any 
HTTP-related spec would need to explain how it doesn't affect the 
semantics of "OPTIONS *".

>> E-03-03, References
>>
>> URL for [9] is missing.
> 
> 
> URLs aren't strictly required, so until I can figure out how to include  
> it (help appreciated here) it's going to continue to be missing.  I  
> think this can be added in Auth48 unless there is an earlier  opportunity.

381c380
<       <reference anchor="refs.W3C-GDIFF">
---
 >       <reference anchor="refs.W3C-GDIFF" 
target="http://www.w3.org/TR/NOTE-gdiff-19970901">

>> E-04-01, Abstract
>>
>> I think the RFC Editor prefers abstracts that do not contain links into
>> the references section (in particular if they're numbered and do not
>> have symbolic names).
> 
> 
> I'd be happy to change that in Auth 48; I wasn't aware of this  
> preference.  Presumably the RFC Editor will also make me aware of this.

Well, now you are. Abstracts are copied as is into other documents; and 
references (in particular numbered) then do not work anymore.

>> E-04-02, References
>>
>> The references need to be split into normative and informative. As 
>> far  as
>> I can tell, only RFC2046 (MIME), RFC2616 (HTTP) and the GDIFF spec  
>> should
>> be normative.  Speaking of which, the reference to VCDIFF should be  
>> removed.
> 
> 
> I've removed VCDIFF, thanks.  I'm actually not sure with the XML format  
> how to split references into normative and informative, but I'd be  
> happy to do so.

See <http://xml.resource.org/>, "Helpful Hints".

Best regards, Julian

-- 
<green/>bytes GmbH -- http://www.greenbytes.de -- tel:+492512807760

Received on Monday, 23 August 2004 20:02:02 UTC