Re: PATCH proposal

>
> 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.

>
> 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.

>
>
> 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.

>
>
> 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.

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.

>
> 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.

>
> 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.

>
>
> 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  
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.

>
> E-03-02
>
> "This standard defines the new method PATCH to alter a single existing
> resource, in place, by applying a delta or diff file."
>
> 1) It's not a standard. It may become one later.

OK; I've fixed this in my local copy and will submit in Auth 48 unless  
there is an earlier opportunity.

>
> 2) The text uses the terms "delta", "diff file" and later "patch  
> document".
> I think it would be wise to formally define *one* term for it once, and
> use that one consistently throughout.

OK; I've become more consistent  in my local copy and will submit in  
Auth 48 unless there is an earlier opportunity.


>
>
> 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.

>
>
> 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.

>
>
> 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.


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

Received on Monday, 23 August 2004 19:21:53 UTC