drbrain / net-http-persistent

Thread-safe persistent connections with Net::HTTP
http://seattlerb.rubyforge.org/net-http-persistent
339 stars 117 forks source link

Ability to set options in initializer #38

Closed ajsharp closed 11 years ago

ajsharp commented 11 years ago

My desire to do this comes when using net-http-persistent with faraday. Faraday uses a middleware-esque initialization process, so, it'd be nice to pass a hash of options in order to set properties on an instance of Net::HTTP::Persistent. Would you be open to a patch that adds support for this? Thanks.

drbrain commented 11 years ago

Why can't the Faraday adapter for net-http-persistent accept an options hash and set the options on the Net::HTTP::Persistent instance? I'm guessing that you would provide the hash to faraday which would then provide it to net-http-persistent.

I am not in favor as it makes my API less clean and the options hash is difficult to document which lead to increased maintenance burden for me.

When only ruby 2 or newer are supported I would support keyword arguments which alleviates these concerns of mine.

ajsharp commented 11 years ago

Why can't the Faraday adapter for net-http-persistent accept an options hash and set the options on the Net::HTTP::Persistent instance? I'm guessing that you would provide the hash to faraday which would then provide it to net-http-persistent.

Right. So, something like this:

Faraday.new('https://example.com') do |conn|
  conn.adapter :net_http_persistent, :idle_timeout => 30
end

So it sounds like you're in favor of the faraday adapter code doing something like the following on their side?

def adapter(name, options = {})
  @net_http = Net::HTTP::Persistent.new('Faraday')
  options.each { |k, v| @net_http.send("#{k}=", v) }
end
drbrain commented 11 years ago

Yes, because I'm a selfish person and don't want to have mildly distasteful code like the second block in my library. I'd also like to have good default values so you only need to set other options in rare cases.

drbrain commented 11 years ago

Since I do not wish to expand my API I will close this.