CaliDog / certstream-python

Python library for connecting to CertStream
MIT License
425 stars 72 forks source link

Add http proxy support #8

Closed Goblenus closed 6 years ago

Goblenus commented 6 years ago

Now you can use http proxy :)

Fitblip commented 6 years ago

Ooh nice! Thanks for the PR, and great idea :)

I'm going to propose a counter-pr to yours that actually structures this a bit differently to essentially expose any of the WebsocketApp.run_forever method's arguments (by passing around kwargs instead of explicit arguments). The documentation/consumption should be the same and just allows one to override some other features (like the origin, or ssl configuration, etc).

Goblenus commented 6 years ago

Not sure, that this a best way to add note about kwargs.

Fitblip commented 6 years ago

Nice, maybe we should link to the function itself so they can see what options are available easily?

https://github.com/websocket-client/websocket-client/blob/87861f951d1a65ed5d9080f7aaaf44310f376c56/websocket/_app.py#L169-L192

Cheers :)

Goblenus commented 6 years ago

I thought about that, and this is not good idea, because the function location may be changed and our url will be useless. But this is the best way to tell why and what for we use kwargs.

Fitblip commented 6 years ago

That URL is actually a permalink to that function args/docs at that specific commit, which of course means it won't be updated automatically, but it will be correct for some definition of correct which I think will suffice for our needs.

The github permalink thing is also just another one of those neat github things that's not obvious, so in case you were wondering where I got it: image

Goblenus commented 6 years ago

Wow...I didn't know about that, thanks. Can you add url and merge by yourself?

Fitblip commented 6 years ago

Sure! Just wanted to make sure you were cool with it :)

Thanks again for the PR!