AutomatedTester / browsermob-proxy-py

A python wrapper for Browsermob Proxy
http://oss.theautomatedtester.co.uk/browsermob-proxy-py
236 stars 104 forks source link

Support to use httpsProxy and httpProxy at proxy creation time #28

Closed i5513 closed 10 years ago

i5513 commented 10 years ago

Hello,

I'm doing a pull request as you said at #27

I'm trying to mantain your style. I would prefer to write the if with other format (one line less):

urlparams = ""
if params:
    urlparams = "?" + urlencode(params)

Thank you

i5513 commented 10 years ago

Hello,

urlencode is breaking ":", so a replace is necesary: urlparams = "?" + urlencode(params) urlparams = urlparams.replace("%3A",":",100)

I will try to update the commit or make a new commit

AutomatedTester commented 10 years ago

I'm not sure why there is a second parameter with empty string to requests.post function call. Maybe it > should be maintain in the patch ?

Well, I was ask you to add a second datapost parameter, to support "bindAddress" example from https://github.com/lightbody/browsermob-proxy , but I see Client is not enough implemented to support such feature out of the box.

Thank you

I can't remember why I put that there. If you remove it and the tests still pass then I am happy for it to be taken away

i5513 commented 10 years ago

Well,

Until it is prepared, I think it is not important to maintain that parameter, it could be there to remember that in the future you can add post data in that parameter

Thanks !

i5513 commented 10 years ago

Hello,

I have updated the patch to use unquote (I think it is better than use replace)

Regards,

AutomatedTester commented 10 years ago

committed in c68dfec446b72754fcbf654d6bc3b081aea23137