Add http_proxy configuration with unittest #15
Conversation
Codecov Report
@@ Coverage Diff @@
## master #15 +/- ##
=======================================
Coverage 93.64% 93.64%
=======================================
Files 11 11
Lines 803 803
=======================================
Hits 752 752
Misses 51 51Continue to review full report at Codecov.
|
|
@pokoli Sorry that I missed this. Please ping me if I missed a PR like this ever in future. It should not take me more than a couple of days to do a code review. More than that is not usual :) |
|
Please resolve the conflicts and ping me after. |
rest_test.py
Outdated
|
|
||
| from mock import patch | ||
|
|
||
| from .configuration import configuration |
There was a problem hiding this comment.
Get this from kubernetes package like if you installed the package. CI tests expect that.
pokoli
force-pushed
the
proxy
branch
2 times, most recently
from
3a573cb to
b2b9dfb
Compare
|
Running on @mbohlool Any idea? |
|
|
|
Uou, so much days without kubernetes-client lost my memory :( @mbohlool Thanks for the tips Now only functional tests are failing, but I dont' think this is related to my change. |
|
Closing and reopening to see if it helps with ci failure. |
rest_test.py
Outdated
|
|
||
| def test_proxy(self): | ||
| 'Test that proxy is created when the config especifies it' | ||
| configuration.http_proxy_url = 'http://proxy.example.com' |
There was a problem hiding this comment.
I think this is the problem, you are changing configuration singleton that has effect on other clients. You should create your own version of configuration:
from kubernetes.client import ConfigurationObject
config = ConfigurationObject()
...There was a problem hiding this comment.
That makes sense, but it does not fix the problem.
Indeed, running py27-functional on my laptop passes.
There was a problem hiding this comment.
The way e2e tests are setup here is not the best. They will pass if there is no cluster to run on. You need to make sure you have a minikube running on default port before run functional tests. We need to fix that but let see what is the problem of this PR.
There was a problem hiding this comment.
So the problem is still Configuration(). It should be ConfigurationObject(). Configuration() is there for backward compatibility with generated client and still returns the singleton. using ConfigurationObject() should fix things.
rest_test.py
Outdated
| def test_poolmanager(self): | ||
| 'Test that a poolmanager is created for rest client' | ||
| with patch.object(urllib3, 'PoolManager') as pool: | ||
| RESTClientObject(config=Configuration()) |
There was a problem hiding this comment.
do you mean to pass a new config or singleton config here? Configuration() is the singleton. ConfigurationObject() will be a new one.
rest_test.py
Outdated
|
|
||
| def test_proxy(self): | ||
| 'Test that proxy is created when the config especifies it' | ||
| config = Configuration() |
There was a problem hiding this comment.
config = ConfigurationObject()
|
I pushed a change to use ConfigurationObject and merged this because I want it to be in the release. |
|
@mbohlool thank you for your help and for finishing it! |
This PR extends #8 by adding some unit test for the PoolManager and the proxy manager.