-
Notifications
You must be signed in to change notification settings - Fork 321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add HTTP::Response#connection #364
Conversation
I believe connection/client thingy might be changed in relatively near future. |
@ixti In which way might it be changed? I think it would still make sense to have it available through the response object, because at the moment I have to do response.body.instance_variable_get("@client").close to close the connection. |
Ping 😃. Since you said it might be changed soon, that's all the more reason to create a public method for accessing the connection. |
@@ -10,6 +10,8 @@ class Body | |||
include Enumerable | |||
def_delegator :to_s, :empty? | |||
|
|||
attr_reader :client |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, it appears as if the @client
instance variable is in fact an HTTP::Connection
. In that case, I suggest renaming it (and the initialize
param) to @connection
and exposing an attr_reader :connection
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
e2b811e
to
8c161e4
Compare
8c161e4
to
b735637
Compare
Looks good to me 👍 |
Thanks! |
@janko-m I'm very sorry. Got lots of things moving around this time of my life (which seems like happen every August to me). I'm totally fine with the PR (I should have noted that more explicitly). There were ideas of moving pretty much of sockets work things into separate gem. There definitely will be changes to API of HTTP methods once I'll find time to work on that (block form that will be closing underlying connection auto-matically). |
In any case. Thanks for your PR! 👍 |
Don't want to be nagging, but it would be awesome to get a new release with |
Released as 2.1.0. |
@tarcieri Thank you! ❤️ |
Upstream changes (from CHANGES.md): ## 2.2.0 (2017-02-03) * [#375](httprb/http#375) Add support for automatic Gzip/Inflate ([@Bonias]) * [#390](httprb/http#390) Add REPORT to the list of valid HTTP verbs ([@ixti]) ## 2.1.0 (2016-11-08) * [#370](httprb/http#370) Add Headers#include? ([@ixti]) * [#364](httprb/http#364) Add HTTP::Response#connection ([@janko-m]) * [#362](httprb/http#362) connect_ssl uses connect_timeout (Closes #359) ([@TiagoCardoso1983])
The Down gem provides the ability to wrap anything that streams chunks into an IO object. I want to add support for HTTP.rb to Down so that you can do the following:
For this I need to be able to access the connection through the response object, so that I can close the connection if either the downloading finishes or user calls
remote_file.close
.