flowcommerce / ruby-sdk

MIT License
1 stars 4 forks source link

Expose HTTP Handler to allow config / swappability #17

Closed mbryzek closed 7 years ago

mbryzek commented 7 years ago

provides implementation for https://github.com/flowcommerce/ruby-sdk/pull/16

paololim commented 7 years ago

Will any change to this be overwritten the next time we generate a new apidoc client?

mbryzek commented 7 years ago

not until we merge https://github.com/mbryzek/apidoc-generator/pull/174

paololim commented 7 years ago

lgtm

Tonkpils commented 7 years ago

This looks good to me as it gives us complete control over the http library to use as we're not tied down to use the default http handler.

jimm commented 7 years ago

LGTM

Tonkpils commented 7 years ago

Does the usage of the Flow client always return the same instance (e.g singleton)? If that's the case, this may not be threadsafe when changing options on the exposed client on DefaultHttpHandler.

mbryzek commented 7 years ago

threadsafe: You're right - The current implementation creates a new instance - will go back to that

mbryzek commented 7 years ago

fyi: replaced puts with logger

mbryzek commented 7 years ago

Refactored a bit to separate instantiation of the client itself from the logic. Removes the need to remember past settings - and isolates custom handlers from any internal config (like service base url)

paololim commented 7 years ago

lgtm

mbryzek commented 7 years ago

@jimm @Tonkpils we've updated based on your feedback - we have published new gem version: 0.2.0 - if you get a change, would love to know if this works for you guys (and if not we can iterate from here).

thanks for the suggestion and feedback!

jimm commented 7 years ago

@mbryzek thank you, we'll check as soon as we're able