HTTP PATCH

[Barry Warsaw]

My understanding is that PATCH is a proposed HTTP verb for doing partial
resource updates:

http://tools.ietf.org/html/rfc5789

It doesn't seem possible to hack PATCH into restish mostly because webob
doesn't recognize it. It doesn't look like it would be hard to add it to
restish itself, e.g.

class PATCH(MethodDecorator):
""" http PATCH method """
method = 'PATCH'

but that doesn't help much. Looks like PUT and POST are hardcoded in various
places in webob.

Has anybody looked into supporting PATCH, or other methods of partial resource
updating, in restish?

Thanks,
-Barry

[Nathan Davis]

Well, I've never implemented PATCH, but I can vouch that you can indeed implement more than just PUT and POST, having implemented a PROPFIND method myself.

Have you tried implementing a PATCH method and, if so, what failed?

Nathan Davis

[Matt Goodall]

Hi,

Sorry for slow response, I've been away for a few days.

As far as I can tell, PATCH works ok, e.g.

class PATCH(resource.MethodDecorator):
method = 'PATCH'

class Root(resource.Resource):
@resource.GET()
def get(self, reqeust):
print 'GET'
return http.ok([], '')
@PATCH(content_type='application/json')
def patch_json(self, request):
print 'PATCH - JSON'
return http.ok([], '')
@PATCH(content_type='text/plain')
def patch_text(self, request):
print 'PATCH - TEXT'
return http.ok([], '')

I quickly tested that under Paste's HTTP server and a plain gunicorn
server, using curl as the client each time. Not exactly a conclusive
test I know ;-) but It seemed to work as expected.

Barry, let me know if you have unearthed a problem. I deliberately
designed restish to be open to future and non-standard HTTP verbs and
only really chose webob because it seemed the most common option. If
there is some incompatibility then it needs fixing.

Cheers, Matt

> --
> You received this message because you are subscribed to the Google Groups
> "ish.io" group.
> To post to this group, send an email to is...@googlegroups.com.
> To unsubscribe from this group, send email to
> ishio+un...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/ishio?hl=en-GB.
>

[Barry Warsaw]

On Aug 27, 2010, at 09:05 PM, Matt Goodall wrote:

>Sorry for slow response, I've been away for a few days.

No worries, I've been traveling a bit too. Thanks for the follow up.

>As far as I can tell, PATCH works ok, e.g.
>
>class PATCH(resource.MethodDecorator):
> method = 'PATCH'
>
>class Root(resource.Resource):
> @resource.GET()
> def get(self, reqeust):
> print 'GET'
> return http.ok([], '')
> @PATCH(content_type='application/json')
> def patch_json(self, request):
> print 'PATCH - JSON'
> return http.ok([], '')
> @PATCH(content_type='text/plain')
> def patch_text(self, request):
> print 'PATCH - TEXT'
> return http.ok([], '')

Yep, restish is okay in the sense that the method gets called. The reason I
think webob is the culprit here is that the form data doesn't get unpacked.
For example, in your patch_text() method, try printing request.POST.items().
Even when you PATCH with form data, this will be an empty list. request.POST
is a NoVars and even request.params gives an empty NestedMultiDict.

Looking at webob's request.py it seems like it's hardcoded to only dig out
form variables when POST or PUT is used. See also

http://pythonpaste.org/webob/reference.html#query-post-variables

So yep, I think it's functionality missing from webob, and because those HTTP
methods are hard-coded, I see no way around it except to change the webob
source code. :(

-Barry

[Matt Goodall]

Ah, I see. That's annoying :-/. Like you, I would expect WebOb to
decode the body (assuming it understands the content-type) regardless
of HTTP method.

Of course, request.GET and request.POST are actually incorrect names;
QUERY and BODY might be better. Unfortunately, I suspect there's a
little too much baggage to get those changed now though ;-).

- Matt

[Barry Warsaw]

On Aug 30, 2010, at 08:53 PM, Matt Goodall wrote:

>Ah, I see. That's annoying :-/. Like you, I would expect WebOb to
>decode the body (assuming it understands the content-type) regardless
>of HTTP method.

I've sent a message off to the paste-users list. Let's see what they say.
How tightly married to WebOb is restish, just in case it's not feasible to
extend WebOb to support this? I wonder if it would be possible to slot in
some alternative WSGI request/response layer, even if optionally?

>Of course, request.GET and request.POST are actually incorrect names;
>QUERY and BODY might be better. Unfortunately, I suspect there's a
>little too much baggage to get those changed now though ;-).

This is a great point, and one that's kind of bugged me about WebOb. Not
enough to complain about it, but if it were me, I'd probably deprecate those
APIs in favor of more generic terms, as you suggest. WebOb is at 0.9.8 after
all :).

Thank,
-Barry

[Matt Goodall]

On 31 August 2010 15:09, Barry Warsaw <ba...@python.org> wrote:
> On Aug 30, 2010, at 08:53 PM, Matt Goodall wrote:
>
>>Ah, I see. That's annoying :-/. Like you, I would expect WebOb to
>>decode the body (assuming it understands the content-type) regardless
>>of HTTP method.
>
> I've sent a message off to the paste-users list.  Let's see what they say.
> How tightly married to WebOb is restish, just in case it's not feasible to
> extend WebOb to support this?  I wonder if it would be possible to slot in
> some alternative WSGI request/response layer, even if optionally?

restish currently expects something webob-like but probably doesn't
use a whole lot of the WebOb API itself. It's more the affect on
restish-based applications I worry about. However, if it was optional
then the application gets to choose (hopefully at the beginning ;-))
and everyone wins.

I don't think I'd want anything approaching a complete
request/response abstraction layer but if it's just a few functions
that need need re-implementing to use environ information instead of
relying on WebOb's API then that would be fine.

Of course, other request/response libraries may well have other problems ;-).

>
>>Of course, request.GET and request.POST are actually incorrect names;
>>QUERY and BODY might be better. Unfortunately, I suspect there's a
>>little too much baggage to get those changed now though ;-).
>
> This is a great point, and one that's kind of bugged me about WebOb.  Not
> enough to complain about it, but if it were me, I'd probably deprecate those
> APIs in favor of more generic terms, as you suggest.  WebOb is at 0.9.8 after
> all :).

I notice Werkzeug call these args() and form(). Those are better names.

- Matt

[Barry Warsaw]

On Aug 31, 2010, at 04:09 PM, Matt Goodall wrote:

>On 31 August 2010 15:09, Barry Warsaw <ba...@python.org> wrote:
>> On Aug 30, 2010, at 08:53 PM, Matt Goodall wrote:
>>
>>>Ah, I see. That's annoying :-/. Like you, I would expect WebOb to
>>>decode the body (assuming it understands the content-type) regardless
>>>of HTTP method.
>>
>> I've sent a message off to the paste-users list.  Let's see what
>> they say. How tightly married to WebOb is restish, just in case it's
>> not feasible to extend WebOb to support this?  I wonder if it would
>> be possible to slot in some alternative WSGI request/response layer,
>> even if optionally?

>I don't think I'd want anything approaching a complete

>request/response abstraction layer but if it's just a few functions
>that need need re-implementing to use environ information instead of
>relying on WebOb's API then that would be fine.

I took a different approach. I managed to get PATCH working through hackery,
and fortunately it's localized to the implementation of the PATCH() decorator
in the Mailman 3 code. I would be interested to hear your thoughts about
moving some or all of this up into restish.

Here's the central diff relevant to restish:

http://bazaar.launchpad.net/~mailman-coders/mailman/3.0/revision/6936/src/mailman/rest/helpers.py

and the entire revision that implements PATCH in the Mailman 3 REST server:

http://bazaar.launchpad.net/~mailman-coders/mailman/3.0/revision/6936

Note the use of the PATCHWrapper class. At first I tried to subclass
ResourceMethodWrapper and override its __call__() to hack in request.PATCH but
that didn't work because it never got called. I'm not very familiar with the
restish code, but I get some hints that restish unwraps decorated methods at
some point. I don't quite understand why that is. So the approach I took was
to ensure that you'd end up with basically two layers of wrapping. When the
outer layer gets unwrapped, you're left with the PATCHWrapper instance to do
the request.PATCH hack. See the comments in the code for why I went with
request.PATCH.

I hope the above makes sense. ;) If there's a better way, please do let me
know, but this does seem to work, so I can now move on.

Thanks,
-Barry

[Matt Goodall]

Yeah, you're right, the decorated *methods* do get unwrapped - at the time they're collected by the Resource's meta class. That was done to avoid duplicating work as ResourceMethodWrapper.__call__ and Resource.__call__ ended up doing something similar but incompatible.

Basically, the ResourceMethodWrapper only really exists to support decorated *functions*, e.g.

@resource.GET(accept='json')

def resource_func(request):

    return ...

Something needs to check that the resource function is only called to respond to a suitable request. To be honest, it was a bit of a hack and I've never really been happy with the decorator + wrapper setup.

 

So the approach I took was
to ensure that you'd end up with basically two layers of wrapping.  When the
outer layer gets unwrapped, you're left with the PATCHWrapper instance to do
the request.PATCH hack.  See the comments in the code for why I went with
request.PATCH.

I hope the above makes sense. ;)  If there's a better way, please do let me
know, but this does seem to work, so I can now move on.

It does make sense and I can't think of anything simpler right now. Perhaps reworking the decorator+wrapper stuff would help but I'm not sure. I guess the best thing would still be to get WebOb fixed or make it pluggable.

If I have any bright ideas I'll post back.

- Matt