Can't Use 'method' As Query Param

[rynbrd]

I ran into this when trying to access the Flickr API via restkit. One of the two required query parameters to Flickr's API is 'method'. Of course, the Resource.request() method's first parameter is named 'method' so that's a no-go. Python spits out a big error:

Traceback (most recent call last):
File "./images.py", line 32, in
res = f.search('obama', 1)
File "./images.py", line 22, in search
return self.get(*_params)
File "/home/sadpengu/sandbox/pylons/lib/python2.6/site-packages/restkit-2.2.2-py2.6.egg/restkit/resource.py", line 135, in get
return self.request("GET", path=path, headers=headers, *_params)
File "./images.py", line 25, in request
res = Resource.request(self, _method, payload, headers, **params).body_string()
TypeError: request() got multiple values for keyword argument 'method'

The easy fix is to rename 'method' to '_method' on lines 181 and 202 of resource.py. I suppose you'd want to go ahead and rename 'path', 'payload', and 'headers' while you're at it. I've done so in my local copy just to get it to work.

However, this is more of a hack than a long-term fix, as this wouldn't help if you happened to run across a site that requires '_method' as one of the query parameters. What would really be needed in this case is a way to pass a plain-old dict of query parameters instead of passing them as arguments, as convenient as that is.

[rynbrd]

Proposal...rewrite Resource.request() as:
def request_dict(self, method, path=None, payload=None, headers=None, params=None):

And have it called from:
def request(self, method, path=None, payload=None, headers=None, **params):

Easy enough fix to implement while preserving existing compatibility.

[benoitc]

i'm not sure to follow, method isn't a named argument, so it should works, anyway rather than implementing another function maybe just doing def request(self, method, path=None, payload=None, headers=None, **params) where paramas is a dict in your code is enough ?

[rynbrd]

'method' is a named argument to Resource.request(). See line 181 of restkit/resource.py:

def request(self, method, path=None, payload=None, headers=None,
    **params):
    """ HTTP request

    This method may be the only one you want to override when
    subclassing `restkit.rest.Resource`.

    :param payload: string or File object passed to the body of the request
    :param path: string  additionnal path to the uri
    :param headers: dict, optionnal headers that will
        be added to HTTP request.
    :param params: Optionnal parameterss added to the request
    """

If you're confused because you see '_method' in the error above it's because I attempted to rename it in my overloaded method, but that doesn't matter because my overloaded method still has to call Resource.request() with the same params dict which contains a 'method' key and so it errors out.

I am passing the params argument as a dict when that error is generated, it makes no difference how you pass the arguments it still errors out.

[benoitc]

well method is an argument not a key argument that what I meant. I reproduce the issue _method isn't really friendly though, so looking for another way.

[rynbrd]

I agree, it's not a clean fix. My second option isn't a very clean fix, either, though I think it works better. Ideally there would be a third option that won't break existing applications built on top of restkit but I don't see one.

[rynbrd]

I'll be honest, my main concern is making any changes that would break couchdbkit. I just realized you're also the developer behind that project as well, so I suppose that is not really an issue as you can make changes to couchdbkit to support any changes made in restkit.

[benoitc]

Well, the main issue here is to keep the compatibility with existing code. There is a new resource thing coming with 2.3 this weekend that should allows that.

[drewp]

There's no such thing as 'key arguments', if I understand what you mean. Python will attempt to use any keyword arg on whatever matching param it can find (and then the leftovers go to **kw, if the function is taking kwargs).

I agree this is an important problem, but I think we can make a solution that doesn't require an alternate 'request_dict' method:

def request(self, method, path=None, payload=None, headers=None, 
            params={}, **kwParams):
    kwParams.update(params)
    ...

Now kwParams is catching all the old-style params (except I have reserved the name 'params', so that's a very minor incompatibility), but we can pass params as a dict in a very natural way. The caller can even combine both approaches:

s.request("GET", "/foo", params={'headers':'hello'}, message='world')
-> GET /foo?headers=hello&message=world

[benoitc]

key argumenst are are kwargs in my mind :) Like your idea anyway, that could indeed works.

[benoitc]

and fixed. thanks for the trick :)

[drewp]

https://github.com/benoitc/restkit/blob/master/restkit/resource.py#L185 still has a problem: it expects to be able to pass the user's arbitrary params_dict as keyword args to make_uri, which breaks with params_dict={"base":"hi"}, for example. make_uri ought to take a dict instead of kwargs.