Twython unnecessarily disables compression. #310
No reviewers
Labels
No labels
Bug
Enhancement
Feature Suggestion
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: code/twython#310
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "master"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
While doing performance profiling as mentioned in lukasa/hyper#19, I stumbled upon the fact that Twython appears to send the header
Accept-Encoding: identity(or leaves the header out altogether, it's not entirely clear, but they're semantically the same thing). This means that downloading the last 800 @-mentions to myself involves downloading 2.2MB of uncompressed JSON.It seems to be trivial to turn this on by passing the appropriate header (
Accept-Encoding: gzip, deflate) inclient_args. More importantly though, this should be on by default when using requests. I've looked through the code and it doesn't seem to be deliberately turned off, so it looks like it's probably an accident.From some code reading, the problem looks like it's around here. In this block of code you end up ensuring that
Twython.client_argsalways contains aheaderskey. Subsequently, in this block, you ensure thatSession.headersis overwritten with your newheadersdictionary. This overwriting gets rid of the default headers (set here), which include theAccept-Encodingline.I have two suggested fixes:
Accept-Encodingto your default headers dict.Sample code to reproduce the problem (this is set up for using
mitmproxyto examine the HTTP headers: if you don't want to, remove theclient_args):Oh wow, this is a pretty nice find - I would not have caught this. @michaelhelmick is more active with the merging at the moment, so I'm gonna leave this for him - just couldn't help but comment. :)
Also wanted to pop in and comment - very nice find and fixes Cory
On 2/23/14 8:18 AM, Cory Benfield wrote:
Thanks guys!
I've turned this into a pull request implementing the first of my possible fixes. Whether you end up wanting this solution is up to you guys.
I also want to note that the test is a little bit brittle: it's highly likely that a future version of requests will change the Accept-Encoding header it sends (see kennethreitz/requests#1916), at which point the test will break.
Very nice catch @Lukasa :D
I'll hopefully push a new Twython release within the next week! Thanks again!