-
-
Notifications
You must be signed in to change notification settings - Fork 460
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
Fixes SSL Certificate Verification problems when running behind proxy through a environ var #252
Conversation
Hello @miguelius! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2021-10-19 16:26:17 UTC |
@miguelius cool! It's look same as I see it: #253 check it out! @SergeyPirogov here are two PR's. Choose the better one ;-) |
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.
Looks cool, but we can make it cooler!
response = requests.get(url, stream=True) | ||
session = requests.Session() | ||
session.verify = handle_driver_download_ssl_verify() | ||
response = session.get(url, stream=True) |
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.
No session necessary, you can still use
requests.get(url, stream=True, verify=handle_driver_download_ssl_verify()
@@ -88,9 +88,15 @@ def write_file(content, path): | |||
return path | |||
|
|||
|
|||
def handle_driver_download_ssl_verify(): |
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.
about naming, may be one of this would be more clear:
def wdm_ssl_verify_property():
def wdm_ssl_verify_value():
def wdm_ssl_verify_env_value():
or something like that
@@ -88,9 +88,15 @@ def write_file(content, path): | |||
return path | |||
|
|||
|
|||
def handle_driver_download_ssl_verify(): | |||
verify = os.getenv('WDM_DRIVER_DOWNLOAD_SSL_VERIFY') |
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.
About naming WDM_DRIVER_DOWNLOAD_SSL_VERIFY
can be named smth shorter like WDM_SSL_VERIFY
- because it's not only about downloading, but also getting latest versions.
): | ||
session = requests.Session() | ||
session.verify = handle_driver_download_ssl_verify() | ||
self._session = session |
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.
no need to use session here, because requests.get()
has verify
param, so you can use requests.get(url, verify=value)
Solved by #253 |
Refactor to use environment variable instead of session in a function and variable.
Just create an environment variable like this with value False. On bash:
´´´
export WDM_DRIVER_DOWNLOAD_SSL_VERIFY=False
python my_cool_test.py
´´´