add ability to use http proxy when making web requests

[rloomba]

Summary

• add ability to use a proxy server when making http requests
• modifyHTTPProjectConfigManager and EventDispatcher to take in proxy config options

I am unable to use this library as all my web requests must be routed through a web proxy.

HTTParty proxy config documentation

Issues

• Fixes [feature request] Add ability for HTTP Config Manager to take proxy options #241

[thomaszurkan-optimizely]

Hi, this looks good. Can I ask for a little refactor (or I can do it if you want)? Could you create a proxy config object that holds these 4 fields instead of having them at the config manager level? Meaning the config manager holds a proxy config object rather than 4 separate fields that are very related. Thanks!

[rloomba]

@thomaszurkan-optimizely sounds good! Happy to do the refactor.

[jasonkarns]

Does it make sense to further entrench the httparty gem via this configuration? As I opened in #211 , I think it makes sense to eliminate the gem dependency entirely and just depend on Ruby stdlib alone. (ie, net/http or open-uri) Both of the stdlib tools respect HTTP Proxy env vars out of the box and would require no configuration at all.

[rloomba]

I'm 100% onboard with #211.

I don't think we'll entrench this gem that much further If we extract the proxy config settings to a separate config object that @thomaszurkan-optimizely mentioned above, couldn't that be used in whatever HTTP library we end up moving to?

I think respecting the HTTP Proxy env var is a good approach, but I also like having the option to explicitly config the proxy in application level code. What if we have multiple proxies that we'd like switch between?

@jasonkarns implementing #211 seems like a bigger chunk of work that I'm not sure I can lead, but I'm happy to pitch in if there are some smaller pieces to be worked on or at least offer some code review.

[jasonkarns]

I think respecting the HTTP Proxy env var is a good approach, but I also like having the option to explicitly config the proxy in application level code. What if we have multiple proxies that we'd like switch between?

Good call. The trick will be ensuring that we distinguish the options being unset (to allow env vars to affect), from them being intentionally set to nil (to ignore env vars and disable the proxy).

[thomaszurkan-optimizely]

Hi @rloomba,
We removed httparty and created a HttpUtils class to wrap our http calls. Can you refactor your proxy settings here and we can merge it? Otherwise, we will do it sometime in the next few sprints. Thanks!

[rloomba]

@thomasgodwell-optimizely ah sorry for letting this drag on. Yes, happy to rebase master and make those changes sometime this week.

[rloomba]

@thomasgodwell-optimizely took a pass at this refactor, let me know if you have any feedback.

[oakbani]

@msohailhussain and @thomaszurkan-optimizely Tested FSC Suite here #261

[coveralls]

Coverage increased (+0.003%) to 99.568% when pulling 0873952 on rloomba:rloomba/add-proxy-config into 6ea2681 on optimizely:master.

[thomaszurkan-optimizely]

Looks good. But, can we update the travis mdspell url?

[thomaszurkan-optimizely]

can we merge and fix this?

[thomaszurkan-optimizely]

This was move to PR #263 so that we could run FSC tests. @rloomba thank you! I made sure to include your credit in the PR and will also make another note to include you when I merge it. Thanks again!