Tidying up a bit for requests 1.0 #4
Conversation
|
You left some print statements in there: |
|
I'm just letting you know that I saw this. I don't have time to get to it this evening, but didn't want you to think it had fallen off my radar. =) |
|
No worries, just threw this together quickly in cause many were affected. Would have updated this earlier but since I don't have much time for open source atm I tunnelvision on oauthlib and am completely oblivious to where it is actually used :) |
| r.url, r.headers, r.data = self.client.sign( | ||
| unicode(r.full_url), unicode(r.method), r.data, r.headers) | ||
| unicode(r.url), unicode(r.method), r.body or '', r.headers) |
There was a problem hiding this comment.
r.body or '' should probably read r.body or None.
There was a problem hiding this comment.
This is an artifact from when I refactored the code, if its r.body, '' or None does not matter at all in this case. Unless I did think of something clever which I've now forgotten...
|
Makes sense to me. @kennethreitz, when/if you get time, can you take a look at this too? |
|
Seems fine to me. |
|
It still requires the swapping in requests tho, don't see a good way around that unless more data is sent into the auth or if the content-type header is populated earlier. |
|
Good point. Want to create a PR on Requests for that change? |
|
I'll be happy to make that change. I didn't put much thought into the order. |
|
Send a PR :) |
|
Hehe, sure. |
|
You can probably go ahead and merge this even if the PR is not in requests yet since the current code does not work at all =) |
Tidying up a bit for requests 1.0
|
@ib-lundgren was there ever a PR that got merged into requests? I had the chance to update requests_oauthlib to 0.2.0 and requests to 1.0.4 and still get the same error I did in #1 |
|
Yepp, it is merged and in at least 1.0.4. However the changes you need are not included in requests_oauthlib 0.2.0. I never tried running the code mentioned in #1 but my flask test example ran fine |
|
Oh, alright. Yeah, cause I was going to say that it didn't fix my problem. :( haha Mike Helmick On Tuesday, January 8, 2013 at 2:47 PM, Ib Lundgren wrote:
|
I've added some initial tests and cleaned up the extension code a bit.
Unfortunately this code depends heavily on an update to requests.models.py in which lines 200 & 201 are swapped, i.e.
becomes
Why? Because we have no idea in auth whether body will soon be filled with files data or not and consequently it would be foolish to assume form encoded on empty body.
I've not had time to look into what implications this might have for requests. Will send a PR when I have. @kennethreitz