Re: PATCH Draft

Hi Lisa,

On 7/4/07, Lisa Dusseault <lisa@osafoundation.org> wrote:
> Hi Mark,
>
> I fail to understand the overall thrust of your proposed changes.
> Several are proposals for removing requirements   with little to gain
> from removing them.  I'd like to see a lot more benefit to removing a
> requirement before opening up what should be a simple feature to more
> variation and complexity.

I'm not quite sure what you mean by "removing a requirement".  Is
there some kind of requirements document that I'm not familiar with,
or perhaps prior discussion where requirements were agreed upon?

>
> Specific comments inline,
>
> On Jul 4, 2007, at 11:07 AM, Mark Baker wrote:
>
> >
> > "The server MUST NOT create a new resource with the contents of the
> > request body"
> >
> > Why not?  I don't see the problem.
>
> This is to require that the server not treat the PATCH like a PUT.
> If there's a way to *apply* the patch to an empty body that might be
> fine, but the server should not save the delta algorithm as the
> resource body. If that's what the client wanted, it would do a PUT.

Oh, I didn't realize it was meant to address that case.  I interpreted
"create a new resource with the contents of the request body" as
meaning, in effect, that a PATCH shall never result in a 201 response
(one of the unanswered questions in the draft).  I think the wording
could be improved if that's what was intended.

But that brings up a bigger issue.  Even if the wording was fixed to
describe exactly what you describe, it will still be in terms of
server implementation which, IMO, isn't a very good way to specify a
protocol.  Consider the (admittedly edge) case where it just so
happens that the state of the resulting resource was identical to the
patch that was applied, perhaps by trying to patch a patch document.
Do we want to exclude that?

I think an improvement would simply be to describe, as unambiguously
as possible, what PATCH means, and let that definition be what
distinguishes it from PUT.

> > "If the entire delta encoding cannot be successfully applied then the
> > server MUST fail the entire request, applying none of the changes"
> >
> > Why is this needed?  It would seem to depend upon the diff format.
> > Plus I could imagine scenarios where it's undesirable (e.g. a streamed
> > patch).  Suggest removal.
>
> No.   A streamed patch or a diff format where it's desirable could
> override this requirement.  The possibility of such a format or
> extension should not induce us to leave this variation open.

What do you mean by "override"?  It's a MUST.

> > "Therefore, the client MUST verify that it is applying the delta
> > encoding to a known entity by first acquiring the strong ETag [...]"
> >
> > I don't see why this is required.  Of course, if the client wants the
> > guarantees provided by strong etags, it can get them.  But why does
> > that matter to a protocol specification?  Suggest removal of that
> > paragraph.
>
> Why remove it?  What's the harm in making this requirement that
> clients behave properly?

How can a spec prescribe that clients behave properly?

> > I suggest the use of 422 instead of 400 in the "malformed delta
> > encoding" case of sec 2.2.2.
>
> What's 422?

The 422 response code as specified in RFC 2518/4918.

> > Sec 2.3 seems to over specify a few things.  For example, the Allow
> > response header isn't required to list "all the allowed methods" so
> > there's no need for "MUST" there.  I think that all you really need in
> > this section is the definition of the Accept-Patch header.
>
> The benefit of this requirement is so that clients can count on the
> Allow header for at least this case.  What's the harm of this
> requirement?

Because that interpretation of Allow isn't licensed by RFC 2616.

I could go along with something like "servers SHOULD use the Allow
response header to indicate their support of PATCH".

Mark.
-- 
Mark Baker.  Ottawa, Ontario, CANADA.         http://www.markbaker.ca
Coactus; Web-inspired integration strategies  http://www.coactus.com

Received on Thursday, 5 July 2007 00:23:49 UTC