Don't busywait forever if Twitter hangs up on us. #194
Conversation
Twitter is moving all of its APIs towards HTTP1.1. https://dev.twitter.com/blog/deprecating-http-1.0-streaming-api Userstream passed already mid-november and has been borken for this lib since On Jan 6, Twitter will apply it as weel for all the other stream methods, meaning they probably would be all broken. The problem was in two pieces. First with this commit, we stop asking for gzip encoded data for streaming calls: for the HTPP1.0 calls, this was not taken into account anyway, but with HTTP1.1 the answer is now chunked and should therefore not caught zipped. (First part of fix for userstream and soon all streams cf #190 #116)
Instead of trashing it as in previous commit, return the sizes of the messages sent by the API as regular messages if requested through the normal API argument 'delimited=length' https://dev.twitter.com/docs/streaming-apis/parameters#delimited (Last part of userstream and other streams fix)
The existing error was never reached, luckily since it was using undefined arguments. moved it in its right place and added handling ssl errors as TwitterHTTPError. (Fix #136)
Added all missing methods from https://dev.twitter.com/docs/api/1.1 Also included some of the streaming methods which work with both GET and POST but accept arguments like "track" which can quickly require POST. (Closes #187 #145 #188)
Oups, the default for all other methods should be True, not False
So the calls to send media (update_with_media, update_profile_image, update_profile_background_image and update_profile_banner) require to send queries differently, as multipart form, and oauth needs to be signed without taking any parameter into account. This does the trick, following the rules here and there: https://dev.twitter.com/docs/uploading-media https://dev.twitter.com/docs/api/1.1/post/statuses/update_with_media https://dev.twitter.com/discussions/1059 Closes #127
convert non string before replacing in it... (follows da33e70 )
Even when there is a low traffic, we know we didn't lose the connection yet when ready_to_read indicates a signal from Twitter, so we don't yield any timeout message and continue, but we should at least yield None like in the non-blocking mode to allow iterator's use to get the hand back and exit cleanly if required. Completes 3df3413 Ping @dkanygin #178
TwitterHTTPError should not be tried to be made from SSLError, the original behavior of just raising this is best, my bad... (Follows 040f13e )
Conflicts: twitter/stream.py
|
I believe the question addressed here is handled by the "hangup" part that @adonoho added to his stream fix, should be checked. |
|
I'll check soon and get back. FYI, the hangups can be reproduced by attaching to a Twitter stream, and then separately attaching to another stream using the same credentials. Twitter notices and kicks you off. |
|
indeed good call :) |
|
Okay, I checked what it does now. If you get disconnected, the iterator will yield {'hangup': True} repeatedly (forever). This is certainly better than entering a busy-hang, but maybe not ideal; users have to notice the hangup message and break out themselves, or their own code will probably loop forever. It might be better to exit the iterator explicitly by adding a "return" after (or insted of) https://github.com/sixohsix/twitter/blob/master/twitter/stream.py#L87 . Thanks! |
|
Yeah, it's not great that it yields forever. @adonoho can you explain some things here? I'm a bit out of the loop on the specifics. It looks like we now have three "unusual" yields: Originally
raise StopIteration('hangup')Alternately, yield the dict, then return right after to stop iteration. Does that make sense? |
|
Oh, check out #198 . I can barely keep up with y'all. 😄 |
|
The Anon, |
|
Cool. I merged #198 and released it as 1.12.1. It's out there now. I'll look at the unusual yields later. @alin-luminoso any further work, or are we cool to close this? |
|
The |
|
Hey guys, great work again! Regarding the timeouts, we need to ensure it also works properly on low traffic queries. When I did my first tryouts, I tried both high traffic (track="bieber gaga") and low traffic (tracking whatever unusual word you like which you can then tweet after a few minutes without any tweet to test you still get tweets). I can't recall exactly the behavior, but I remember the existing code was actually either breaking into an infinite loop sometimes, either sending plenty of timeouts even though the connection was still up, depending on using the timeout and non blocking methods (or reciprocally). I'll try and take a bit of time to run my battery of combined different testcases on the new version this week-end, but it seems to me like this question should be checked before taking the risk to always throw exception and break connections when there might actually just not be any coming tweet in the pipe since the past "timeout" time. If eveyrthing is handled properly this shouldn't happen, since twitter still signals the connection is up to the sock in the timeout tester part. Again this might already be the case, just felt like sharing my 2 cents warnings since I've been dealing a bit with this over last christmas :) |
Twitter will sometimes disconnect you while you're listening to a stream. (In our experience this happens because the credentials you used are being used somewhere else.) Currently that causes the stream iterator to spin forever reading zero bytes from the socket. This patch causes it to return (ending the iterator) instead.
I am not sure what should happen in the case of timeout or non-blocking mode and did not attempt to address those.