Refactor to use requests library

[rufuspollock]

http://www.python-requests.org/

[davidread]

+1. pycurl dependency is problematic - requires libcurl development files installed.

[rufuspollock]

@Hoedic would you be up for taking a look at this?

[Hoedic]

So I have a working version of CKANClient using python-requests. I just have an little issue where I would like to have some feedback.

In the original version of the code (and my version using libcurl), there was a way to specify the content-type of the file (below is an example of the multipart content):

POST /storage/upload_handle HTTP/1.0
Host: 10.0.1.12
X-Real-IP: 10.0.1.193
X-Forwarded-For: 10.0.1.193
Connection: close
User-Agent: PycURL/7.24.0
Accept: */*
Authorization: 6ecb781e-4ac1-492e-a2b5-b1b369b75a70
X-CKAN-API-Key: 6ecb781e-4ac1-492e-a2b5-b1b369b75a70
Accept-Encoding: identity
Content-Length: 2173
Content-Type: multipart/form-data; boundary=----------------------------028158a3c368

------------------------------028158a3c368
Content-Disposition: form-data; name="key"

2013-12-27T112048/input.json
------------------------------028158a3c368
Content-Disposition: form-data; name="file"; filename="input.json"
Content-Type: application/json

....CONTENT OF THE FILE....
------------------------------028158a3c368--

See the Content-Type: application/json just before the file content.

With python requests, there is no way to set the content-type. Based on what I can read, the module is supposed to use system-wide libraries to define the content-type. But in my case, the content-type is just absent (See below).

POST /storage/upload_handle HTTP/1.0
Host: 10.0.1.12
X-Real-IP: 10.0.1.193
X-Forwarded-For: 10.0.1.193
Connection: close
Content-Length: 2117
X-CKAN-API-Key: 6ecb781e-4ac1-492e-a2b5-b1b369b75a70
Accept: */*
User-Agent: python-requests/2.1.0 CPython/2.7.2 Darwin/12.5.0
Content-Type: multipart/form-data; boundary=d2904fca5b0f43d3879c8395d9224911
Authorization: 6ecb781e-4ac1-492e-a2b5-b1b369b75a70
Accept-Encoding: identity

--d2904fca5b0f43d3879c8395d9224911
Content-Disposition: form-data; name="key"

2013-12-27T112048/input.json
--d2904fca5b0f43d3879c8395d9224911
Content-Disposition: form-data; name="file"; filename="input.json"

... FILE CONTENT...

--d2904fca5b0f43d3879c8395d9224911-- 

It seems that CKAN is not unhappy with this missing data (the file is successfully uploaded with no error in the log) and I can't find any place in the CKAN server where the file-level content-type is used.

So either someone has some idea on how to add content-type at the file level or confirm that CKAN does not need the content type (in which case, I just have to do some additional tests and ship the code)

[davidread]

@Hoedic great stuff getting this going in requests!

I don't have any insight on whether the content-type is needed by CKAN. BUT I have seen that requests added this feature in 2.0.1 according to the changelog. It's not in the documentation, but it's pretty clear how to do it in the patch: https://github.com/kennethreitz/requests/pull/1640

The only remaining issue is that one or two CKAN extensions are dependent on ckanclient and some depend on a fixed version of requests. Since only this one ckanclient command needs requests, I suggest we don't make a hard requirement of this newer version of requests on ckanclient (setup.py). I moved the import of pycurl into the function that used it and I think the requests import should live there too, with a comment saying that v2.0.1 or later is required. Are you happy with that?

Once again, thanks for this most useful improvement - let's get it merged in :-)

[Hoedic]

Thanks @davidread, I'll try to integrate the feature added in 2.0.1 tonight and move the include only in the _post_multipart function.

Would you be able to do some tests on the merged version after? I won't be able to advance that for at least one full week after tonight and I probably won't have time to do significant testing before I have to stop.

[Hoedic]

Ok! @rgrp, @davidread , I sent a pull request on this. I've done some basic tests on the new code and it seems ok (however it is still not able to support a filename with non-ascii chars...). However additional testing is still welcome...

One question: I see there is no requirements.txt to execute to specify (and install...) the dependencies in terms of modules/libraries. Is there a reason for this? Maybe we should add one and document that in the install so that people won't be surprised when users will get an import error on requests... no?

[davidread]

This has been merged in as #33 although an issue remains #34