Added Requests_Transport_cURL support for proxy options #37

Closed
wants to merge 1 commit into
from

Projects

None yet

2 participants

We can actually use the hooks to do this, however, $options is much simpler and easier to use IMHO.

Owner
rmccue commented Jan 15, 2013

This looks good, but I have a few comments:

  1. Can you make this work for fsockopen too please? The transport used has to be transparent to the end user, so it has to work identically on both.
  2. Can you write tests for this please? While the changes appear good, I have no idea if it actually works without those. :)

Thanks for contributing!

Hi Ryan,

Thanks.

I am in a hurry at the moment and won't be able to support fsockopen.
fsockopen proxy support does not seem to be as straightforward as curl
proxy.

As for the test, I'm not sure how I would write one for this feature,
since the test itself must work on all environments.
I can only say that I am using it in my project and its working as expected.

Best regards,
Glenn

On 1/15/2013 8:22 AM, Ryan McCue wrote:

This looks good, but I have a few comments:

  1. Can you make this work for fsockopen too please? The transport
    used has to be transparent to the end user, so it has to work
    identically on both.
  2. Can you write tests for this please? While the changes appear
    good, I have no idea if it actually works without those. :)

Thanks for contributing!


Reply to this email directly or view it on GitHub
#37 (comment).

@ozh ozh referenced this pull request Sep 7, 2013
Closed

Full proxy support #68

Owner
rmccue commented Sep 24, 2013

Fixed as part of #70; thanks for the patch anyway!

@rmccue rmccue closed this Sep 24, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment