-
Notifications
You must be signed in to change notification settings - Fork 814
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proxy support for the agent #377
Conversation
proxies = urllib.getproxies() | ||
proxy = proxies.get('https', None) | ||
try: | ||
proxy = proxy.split('://')[1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use urlparse instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good idea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
urlparse is not great (at least on old python versions)
>>> urlparse('https://user:password@host:1234')
('https', 'user:password@host:1234', '', '', '', '')
>>> urlparse('user:password@host:1234')
('user', '', 'password@host:1234', '', '', '')
So I'm going to keep this parsing.
Looks great! My only question is: are there any licenses we need to include for the 3rd-party stuff we're packaging? |
I didn't see any: |
Ah cool, I didn't realize it was just a patch to stdlib python. Sweet! |
If the forwarder is installed, it uses tornado's CurlAsyncHttpClient
CurlAsyncHttpClient requires PyCurl which is not easily installable from source.
Therefore proxy support won't be working on source installation of the agent
If the forwarder is not used it uses urllib2 in the emitter.
Warning:
It has been tested on:
DogstatsD doens't implement proxy support as it's local traffic in most cases.