search

gaffneyc / usps

USPS Webtools API for Ruby
MIT License
63 stars 54 forks source link

Drop typhoeus dependency #21

Closed laurynas closed 7 years ago

laurynas commented 8 years ago

Typhoeus is big dependency for a very small task. It also had some thread safety concerns. I suggest using native Net::HTTP.

schleg commented 8 years ago

@laurynas why not https://github.com/lostisland/faraday if we make any change? we could default to typhoeus as the default back-end, then give the option to choose a different back-end in the usps options.

gaffneyc commented 8 years ago

I agree with removing Typhoeus as a dependency. This was originally written against Ruby 1.8 that had pretty bad net/http performance. It's improved a ton since then so think this is a good move.

As for Faraday, which I'm also a big fan of, I think it's great for writing your integrations but gems should try to stick to net/http if they can. Just looking at one project we have httparty, faraday, and excon in the Gemfile.lock. The biggest draw of any of these now is that they have APIs that are easier to work with than net/http.

I will recommend taking a look at what I did in the Snitcher gem where we use net/http. This makes sure all of the timeouts are configured and supports HTTPS and custom headers.

laurynas commented 8 years ago

Yes, faraday is nice, but I suggest to have less dependencies if possible. Similar to @gaffneyc example, we already have multiple http clients in one of our project's Gemfile.lock and tyhphoeus was only added as a dependency for usps :) Thanks for listing other options - always good to know them when building some bigger API client. We also use simple RestClient sometimes.

laurynas commented 8 years ago

Added open and ssl timeouts.