make http/https_proxy env var checking case insensitive (address issue #5137) #5347
Conversation
|
|
|
I have agreed to the CLA license about 5 hours ago, but it still asking me to sign |
amanda11
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I've put a suggestion in for a change, and could you also:
- sign the CLA with the email address you are using for github (we did change the CLA so might need to re-sign)
- add an update to the changelog file
st2client/st2client/base.py
Outdated
| print("---------------") | ||
| print("HTTP_PROXY: %s" % (os.environ.get("HTTP_PROXY", ""))) | ||
| print("HTTPS_PROXY: %s" % (os.environ.get("HTTPS_PROXY", ""))) | ||
| print("HTTP_PROXY: %s" % (os.environ.get("HTTP_PROXY", os.environ.get("http_proxy")))) |
There was a problem hiding this comment.
This means that if neither are specified that None is outputted instead of "", could you use , "" on the inner os.environ.get.
Also, what happens if both are set. I think http_proxy in lowercase generally takes prioiryt, so we should probably change the order so that we check for the lowercase value, and only look for uppercase if its not set.
There was a problem hiding this comment.
Hey, thanks for feedback, I will update it asap
There was a problem hiding this comment.
Hey , I used Star Lord by mistake(There is no such account), I have accepted CLA License by S-T-A-R-L-O-R-D
|
@S-T-A-R-L-O-R-D: |
|
If you need help rewriting history, let me know. I can help. |
|
My zscaler Id was my corporate Id and I no longer have to acces to that Id.
If possible I would like to rewrite my history.
Thanks and regards,
Devesh Solanki
…On Sat, 2 Apr, 2022, 2:15 am Jacob Floyd, ***@***.***> wrote:
If you need help rewriting history, let me know. I can help.
—
Reply to this email directly, view it on GitHub
<#5347 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKSPWQCWIFJ3CMRRYYGE5XLVC5N5XANCNFSM5C76QWTA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Rebased on master and corrected the author email. |
…None by '' in case no such variable found on system
|
Rebased and reformatted with black. |
| ) | ||
| print( | ||
| "HTTPS_PROXY: %s" | ||
| % (os.environ.get("http_proxy", os.environ.get("HTTPS_PROXY", ""))) |
There was a problem hiding this comment.
is this http_proxy or https_proxy for HTTPS_PROXY ENV?
There was a problem hiding this comment.
Good catch. If you submit a PR to add the missing s, then I can merge it fairly quickly. Thanks!
There was a problem hiding this comment.
@cognifloyd Done, please check: #5678.
feel free to let me know if any parts need to be modified.
checking both 'http_proxy' and 'HTTP_PROXY' to get proxy info when using st2 command with --debug option.
Fixes #5137