search

mwunsch / weary

A framework and DSL for building RESTful web service clients
MIT License
480 stars 23 forks source link

Excon and Typhoeus Adapters #2

Closed hypomodern closed 12 years ago

hypomodern commented 12 years ago

If you're interested, I was intrigued by how you were organizing things and ended up implementing Excon and Typhoeus adapters. To do this I've added a simple configuration option Weary.adapter--which defaults to Net/HTTP and is therefore optional--and moved the loading of the adapter classes from require to another autoload statement.

The Typhoeus adapter is a little naive in that it uses the hydra singleton rather than attempting to expose parallelization, but still, either of these libraries has more horsepower than good old Net/HTTP.

I gather that you're in release candidate prep and this isn't intended to interrupt that, unless you like it enough to include now.

mwunsch commented 12 years ago

Thanks for this -- this is great! But, I'm on the fence if the best place for new adapters is in Weary itself, or built as standalone gems to work with Weary (or Rack, since they're implemented as Rack applications).

I don't have an answer for that yet, and am not sure if I need to arrive at one for the 1.0 release. So I'm going to punt.

Let's keep this pull request open and come back to it again following the official 1.0 release, and try to generate discussion around the value of Weary-targeted middlewares.

czarneckid commented 12 years ago

I would only say that something like this does belong in Weary itself because you've got the following directory structure, lib/weary/adapters. Plural. But you'd only have one adapter for Net::HTTP? I contrast this with lib/weary/middleware. Singular? But there are multiple middlewares. I'd love to see, at the very least, the Typhoeus adapter as an included alternative to Net::HTTP since Typhoeus is proveably faster than Net::HTTP. Typhoeus is definitely my go-to HTTP library nowadays.

aaronchi commented 12 years ago

+1

I think it makes sense for the adapters to live in weary. Just add your desired dependency in Gemfile and set the adapter. That is the way I'm used to doing it.

I suppose the only issue is whether changes to the adapters will create problems for managing weary releases. While I think the multiple gem structure makes sense for projects like omniauth that have a lot of external dependencies, I can only imagine there being a handful of http adapters and assume that they won't change that often.

In any case, looking forward to using this. Thanks!

mwunsch commented 12 years ago

Alright, I'll work on merging this weekend.

mwunsch commented 12 years ago

I HAVEN'T FORGOTTEN THIS!

Going to rework the configuration parts a bit...