jplana
Add proxy support
Hepp!
I added this as I needed to use etcd through a proxy. It works well and have been for weeks. Please pull this :-)
Kind regards, bahner
Coverage decreased (-0.1%) to 88.305% when pulling f31e768365c7d62ba2f2814c36017a4eee4f2321 on bahner:master into 0d0145f5e835aa032c97a0a5e09c4c68b7a03f66 on jplana:master.
Coverage decreased (-0.1%) to 88.305% when pulling f31e768365c7d62ba2f2814c36017a4eee4f2321 on bahner:master into 0d0145f5e835aa032c97a0a5e09c4c68b7a03f66 on jplana:master.
Coverage decreased (-0.9%) to 87.582% when pulling 757b27cefa716150ce10440732a49e878f2d45f3 on bahner:master into 0d0145f5e835aa032c97a0a5e09c4c68b7a03f66 on jplana:master.
@ashcrow _log statement added. The travis error seems to be unrelated to the actual code submitted.
Coverage decreased (-0.6%) to 87.843% when pulling 49810aa0221aa5b0085325c19fe81f2bf84f145e on bahner:master into 0d0145f5e835aa032c97a0a5e09c4c68b7a03f66 on jplana:master.
Coverage decreased (-1.2%) to 87.275% when pulling 72d836c1d3a266e12d0a8cd3f605d82c66be63d4 on bahner:master into 0d0145f5e835aa032c97a0a5e09c4c68b7a03f66 on jplana:master.
Please add tests for this (both unit tests and possibly integration?). I am not ok with decreasing our code coverage by 1.2% for a function that is surely ok to have but not really a must-have.
I will add the tests. But don't have time this weekend. :-) The reason for this, as where I work, is that the only allowed access to etcd is through a webproxy. Getting an etcd-proxy between zones is not an alternative - even if this would be cleaner.
The smaller change, with only the proxies, works very well for us, but when I tried to run an etcd locally, not so much.
I will write the tests, but I definetly think this should be in here. Proxies are generaly handled very badly in many applications, and I agree the reading an parsing of os vars is less than perfect. But that's basically the way it is.
Another option would be to write a small module, that does the parsing and hides the complexity somewhat. But that's another discussion.
2017-02-12 13:15 GMT+01:00 Giuseppe Lavagetto [email protected]:
@lavagetto requested changes on this pull request.
I think this functionality should live outside of the client (detecting OS variables and so on), and that the client should only get a proxy address if the proxy should be called, and maybe a list of no-proxy machines. But I am honestly doubtful this is useful at all, given you can use the etcd proxy that we support already and is thought as an alternative to a generic web proxy.
Also, code needs to be separated from the init function and be sent. And please add tests!
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jplana/python-etcd/pull/212#pullrequestreview-21405945, or mute the thread https://github.com/notifications/unsubscribe-auth/AJq5L02F6VDrhHAR2EeRBfxW9JVvlNvfks5rbvf-gaJpZM4K9Bds .
-- Mvh, Lars Bahner