Fix for broken PUT methods #414

[snail-coupe]

Pass _method directly to urllib_request to enable PUT requests and also ensure consistency between signature and actual request,.

Pass id parameter into auth.encode_params() for PUT requests, as required by two existing API methods using PUT.

[coveralls]

Coverage decreased (-1.9%) to 28.473% when pulling 51fd173 on snail-coupe:keep-id-with-PUT into 689a001 on sixohsix:master.

[boogheta]

This looks OK to me.

If I'm correct, the PUT routes require to send data as json, explaining why the id argument needs to be passed a second time, is that right?

Although Travis failed on this PR, but it seems it is for different reasons related to pycov, @hugovk would you mind taking a look?

[hugovk]

For one, it's a bit odd the Travis result isn't shown here in the PR. Perhaps the integration needs checking in the repo settings? Sometimes removing it and re-adding helps.

Anyway, the CI did run, here's the link: https://travis-ci.org/github/sixohsix/twitter/builds/703358264

It's also failing on master. Will check.

[hugovk]

PR to fix the CI: #416

[snail-coupe]

This looks OK to me.

If I'm correct, the PUT routes require to send data as json, explaining why the id argument needs to be passed a second time, is that right?

As I follow it, there are only two API calls that use PUT (updating DM welcome message, and hide replies currently in labs) - they both take most of the data within a JSON body, but the both also have an id parameter they are referencing in the url path.

If I've followed the existing code assumed if there was a json body then there was no need to pass any parameters in the path (other than OAuth stuff) so ignored them, but that leads to an error about missing id...

[boogheta]

@hugovk Yes indeed this is weird that Travis does not show up in the PRs anymore, but I don't have the rights to touch the repo settings. @sixohsix could you try and reattach Travis?

@snail-coupe yeah I got that, this is why I was asking whether both PUT routes require json data. So It sounds like a yes.
Can I ask that you rebase your branch against master and repush --force on the PR branch so that Travis will try again the tests? Thanks!

[snail-coupe]

@snail-coupe yeah I got that, this is why I was asking whether both PUT routes require json data. So It sounds like a yes.
Can I ask that you rebase your branch against master and repush --force on the PR branch so that Travis will try again the tests? Thanks!

Done (I think) - I should have included references for both PUTs - sorry - first timer

https://developer.twitter.com/en/docs/direct-messages/welcome-messages/api-reference/update-welcome-message
https://developer.twitter.com/en/docs/labs/hide-replies/api-reference/put-hidden

[boogheta]

Thanks, yes that did the trick
It looks though that this breaks compatibility with python 2.7 which still worked otherwise, because method is not a valid kwarg for urllib2.Request. I know python 2.7 is deprecated, but I think it's a bit sad to drop it just for this, I'd like if we could find out how to add the argument differently to handle this.
With python3.4 it seems to break as well, although this looks more like a timeout error which might work after retry

edit: a potential solution seems attainable following this trick: https://stackoverflow.com/questions/111945/is-there-any-way-to-do-http-put-in-python/111988#111988 (or here https://stackoverflow.com/questions/111945/is-there-any-way-to-do-http-put-in-python/44781372#44781372 )

[snail-coupe]

I didn't even think about other python versions, sorry!

My code runs on a raspberry pi which I think is 3.7 but does have 2.7 as well.

I'll make sure it works with 2.7, and push again once it does so; those stack overflow links looked useful.

[snail-coupe]

Sorry for not getting back to this - I've gone with a slightly hacky solution based upon one of those stack overflow answers. Given "our" code always determines the method it wants, just over-ride the get_method function of whichever version of urllib and Request to just use the method we want.

I recently needed to use an API call that uses DELETE and this appears to work fine for that as well.

[RouxRC]

Hello,
There was still an issue with python2 as shown by Travis here: https://travis-ci.org/github/sixohsix/twitter/jobs/741753711
I allowed myself to fork your PR and push an extra commit on a branch here: https://github.com/sixohsix/twitter/commits/snail-coupe-keep-id-with-PUT
It works as such with all versions: https://travis-ci.org/github/sixohsix/twitter/builds/743230828
So I will merge the branch and close this PR although it is actually merged, thanks again!

[snail-coupe]

Thank you!

And sorry for missing the other python 2.7 (I did intent to check the Travis output but when I pushed that update it was taking a while to go though and hasn't got back to it)

[RouxRC]

Yep, no problem, I understand, Travis is a mess these days and we are on the verge of migrating to github actions. Thanks again for your contribution!