ably / ably-php

PHP client library SDK for Ably realtime messaging service
https://ably.com/download
Apache License 2.0
50 stars 10 forks source link

Use "https_proxy" env variable if defined to set the CURLOPT_PROXY option. #54

Closed DamienHarper closed 5 years ago

funkyboy commented 6 years ago

Looks good. Thanks. I wonder if there’s a quick way to test when the proxy has been set @damienharper

DamienHarper commented 6 years ago

@funkyboy Not sure we can find a quick way to test. The company I work for rely on a hosting provider which blocks any trafic to the outside that do not go through its proxy. Both my previous PR and this one solve the problem in our use case, that's how I test it ;)

funkyboy commented 6 years ago

I think some unit test along the lines of these is enough https://github.com/ably/ably-php/blob/master/tests/TypesTest.php Like “set proxy and check that is set”, without actually performing a request.

DamienHarper commented 5 years ago

@funkyboy @jdavid any chance this one gets merged and released (it's almost the same as #51 which has already been merged and released today)?

mattheworiordan commented 5 years ago

@DamienHarper we had assumed no one needed this so parked it. I assume you still need to connect through a proxy? If so, I will see if we can get this bundled in later this week after our 1.1 release (hopefully tomorrow)

DamienHarper commented 5 years ago

@mattheworiordan thanks for the feedback. As you guess, we still are using the same hosting provider (we won't change anytime soon) so we still need to go through their proxy (for both HTTP and HTTPS protocols) Until this PR is merged and released, the only solution for us right now is to use our own fork including the code from this PR.

mattheworiordan commented 5 years ago

@jdavid are you happy for us to merge this?

mattheworiordan commented 5 years ago

@DamienHarper this is merged and now released as 1.1.1. Sorry this took so long!

DamienHarper commented 5 years ago

@mattheworiordan @funkyboy @jdavid thanks!