-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Changes made in configuration.py to accept environmental variables #2390
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
base: master
Are you sure you want to change the base?
Changes from all commits
afd1d44
8f356ef
6b85a93
638abe2
b778952
b295c2d
9c35894
1c69346
e33362f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ | |
import multiprocessing | ||
import sys | ||
import urllib3 | ||
import os | ||
|
||
import six | ||
from six.moves import http_client as httplib | ||
|
@@ -158,9 +159,15 @@ def __init__(self, host="http://localhost", | |
""" | ||
|
||
self.proxy = None | ||
if(os.getenv("HTTPS_PROXY")):self.proxy=os.getenv("HTTPS_PROXY") | ||
if(os.getenv("https_proxy")):self.proxy=os.getenv("https_proxy") | ||
if(os.getenv("HTTP_PROXY")):self.proxy=os.getenv("HTTP_PROXY") | ||
if(os.getenv("http_proxy")):self.proxy=os.getenv("http_proxy") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If both HTTPS_PROXY and HTTP_PROXY are set, does this mean HTTP_PROXY will override HTTPS_PROXY? Is this behavior matching the websocket client behavior? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If both HTTPS_PROXY and HTTP_PROXY are set, HTTP_PROXY will override HTTPS_PROXY. But Web client has only one self.proxy variable. So, I think it will work fine unless user provide both HTTPS_PROXY and HTTP_PROXY. If user needed both variables we need to check that condition. Even then we can just give HTTPS_PROXY priority... |
||
"""Proxy URL | ||
""" | ||
self.no_proxy = None | ||
if(os.getenv("NO_PROXY")):self.no_proxy=os.getenv("NO_PROXY") | ||
if(os.getenv("no_proxy")):self.no_proxy=os.getenv("no_proxy") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar question here. Does no_proxy override NO_PROXY if both of them are set? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes no_proxy override NO_PROXY. But this is for case sensitive environments like macOS . Only if user declared both no_proxy and NO_PROXY. In windows it will treat both as same. So, I added both no_proxy and NO_PROXY. But they are same... |
||
"""bypass proxy for host in the no_proxy list. | ||
""" | ||
self.proxy_headers = None | ||
|
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.
This file is generated by the upstream openapi generator. Could you make the change upstream?
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.
I have checked https://github.com/OpenAPITools/openapi-generator. We need to change in https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/python/configuration.mustache (as it is python code change.). But while I am at it. I observed that there is no no_proxy variable there.

I am not sure whether I can add no_proxy variable to it or not. It was removed for some reasons according to this issue OpenAPITools/openapi-generator#20226.
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.
If it is only for self.proxy=None. I can add it to https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/python/configuration.mustache.
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.
This client is based on an old version of the upstream generator. The upstream generator probably had some change to the no_proxy variable since then.
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.
Instead of upstreaming the changes can we downstream the changes to files that using configuration.py variables. i.e. python_kubernetes\kubernetes\base\stream\ws_client.py and python_kubernetes\kubernetes\client\rest.py. We can add this change in above files. But I have a doubt will it impact this changes if we use Open-api generator...

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.
Even I cann't delete it. It is not my commit.

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.
@roycaihw Can you check and approve my PR
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.
@roycaihw Can you check and approve my PR
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.
@roycaihw Can you check and approve my PR
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.
Even though this client is using a old version of the generator, can we first get this change accepted by the upstream generator, and then we can patch this client?