Skip to content
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

Make keep_alive_timeout configurable #405

Closed

Conversation

nestegg
Copy link
Contributor

@nestegg nestegg commented Apr 12, 2017

Allow keep_alive_timeout to be set in persistent:

HTTP.persistent("http...", timeout: 120)

@nestegg
Copy link
Contributor Author

nestegg commented Apr 12, 2017

This CI failure is unrelated to the changes.

@@ -138,8 +140,11 @@ def timeout(klass, options = {}) # rubocop:disable Style/OptionHash
#
# @yieldparam [HTTP::Client] client Persistent client
# @return [Object] result of last expression in the block
def persistent(host)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We officially support ruby 2.0+, so it's better to use kwargs in here:

def persistent(host, timeout: nil)
  # ...

context "with timeout specified" do
subject { HTTP.persistent host, :timeout => 100 }
it "sets keep_alive_timeout" do
options = subject.default_options
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use named subject please:

subject(:client) { HTTP.persistent ... }
# ...
options = client.default_options

@@ -1,3 +1,4 @@
# coding: utf-8
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this duplicates line 3

Allow keep_alive_timeout to be set in persistent:

HTTP.persistent("http...", timeout: 120)
@nestegg nestegg force-pushed the configurable_keepalive_timeout branch from 2056ab7 to 8bb2a6c Compare April 13, 2017 04:57
@nestegg
Copy link
Contributor Author

nestegg commented Apr 13, 2017

This has been updated.

@ixti
Copy link
Member

ixti commented Apr 13, 2017

Merged as 4738d22

@ixti ixti closed this Apr 13, 2017
jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Jun 5, 2017
## 2.2.2 (2017-04-27)

* [#404](httprb/http#404),
  [#405](httprb/http#405)
  Make keepalive timeout configurable.
  ([@nestegg])
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants