Add a Gzip middleware

[romanbsd]

No description provided.

[mislav]

Neat. However, is the middleware necessary? net/http, for instance, does this transparently. I would look into our other adapters and see if they do so as well. If most of them do it, I wouldn't maintain such middleware.

[romanbsd]

Well, it does handle it, in some ruby versions, sometimes, for some cases. For the best of my knowledge, it doesn't add the Accepts-Encoding header anyway. So yes, net/http, and em-http-request might uncompress the body, given the chance, but the behavior is far from being well defined. As a side note, we're using it with Faraday + net-http-persistent (which is backed by net/http).

[mislav]

net/http started transparently handling gzip in Ruby 1.9. It adds the accepts-encoding directive and handles gzipped responses. This only applies to GET though. We assume that the responses that are results of POST, PUT, DELETE, etc. are small enough to not benefit from compression.

Your middleware is good stuff, but it doesn't provide any benefits for people already using net/http or net-http-persistent. As for other HTTP clients, I don't know how they handle this. If most of them don't, then I would accept this middleware as a way to get compression no matter what adapter.

CI is failing because you need to add class-level documentation for Gzip, BTW.

[romanbsd]

Well, I'm 100% sure that Faraday.get(url) using the net/http doesn't use gzip.

[mislav]

What makes you think so? Given a debugging HTTP proxy on port 8888:

$ http_proxy=localhost:8888 ruby -rfaraday -e 'Faraday.get("http://api.github.com")'

This is the actual HTTP request that net/http makes:

GET http://api.github.com/ HTTP/1.1
User-Agent: Faraday v0.8.9
Accept-Encoding: gzip;q=1.0,deflate;q=0.6,identity;q=0.3
Accept: */*
Connection: close
Host: api.github.com

Notice the Accept-Encoding directive.

[romanbsd]

My mistake, I was referring to net-http-persistent:

Faraday.get 'http://m.yahoo.com/'
opening connection to m.yahoo.com...
opened
<- "GET / HTTP/1.1\r\nUser-Agent: Faraday v0.9.0\r\nAccept: */*\r\nConnection: keep-alive\r\nKeep-Alive: 30\r\nHost: m.yahoo.com\r\n\r\n"

[mislav]

Ah, alright. You're right that net-http-persistent doesn't do compression in Ruby 1.9. In Ruby 2.0+ it starts, though. Weird. I'm guessing this behavior is dependent on some internal net/http change in Ruby 2.0.

EM also supports transparent decompression. Excon and Patron don't seem to; at least not by default.

[romanbsd]

Yeah, I've noticed the inconsistency... :-/

[romanbsd]

I'm not sure why Travis is unhappy now. Something to do with 1.8 :(

[mislav]

Zlib::GzipReader.new(StringIO.new(env[:body]), :encoding => 'ASCII-8BIT')

Ruby 1.8 doesn't support encodings.

[stve]

I recently needed to do this as well, using the typhoeus adapter, and ended up writing something fairly similar to what @romanbsd has. This seems like it'd be a helpful addition, although I can see how it does pose an issue where an adapter already implements gzip encoding.

[romanbsd]

So, should I go ahead and fix the 1.8 compat issue?

[mislav]

Yep, go ahead and fix that

[mislav]

Seems like you fixed it. CI is failing now because the call method is too complex. Breaking it down into smaller methods will help.

Did you intentionally close this?

[romanbsd]

yes, I created a new pull request. Breaking it down doesn't make a lot of sense, if you check the code. Is there another way to work around this draconian code style validator?

[mislav]

I would prefer if you just force-pushed to the same branch after rebasing instead of opening a new PR. That way we would have kept the discussion and the code in the same place.

If you don't want to change the code, you could add an exception to Cane:

cane --abc-exclude Foo::Bar#method

However, I think that the method can definitely be simplified a bit. I can take care of that if you think you're done with the code.

[romanbsd]

ok, force pushed. Closing the other pull request.

[romanbsd]

And by the way, arsduo/koala#346

[romanbsd]

I can't believe it, the travis CI build has finally passed 👍

[mislav]

Thanks, merged and cleaned up a bit in d35f960