If gevent is activated (using monkey_patch) we have to use grequests instead of requests. #22
Conversation
|
As simple as that 💃 |
|
I'm not comfortable with this happening automagically. How would you feel about exposing an explicit function that the app can call to enable this, e.g. |
|
(Although this is by far the most considerate automagical gevent detection I've ever seen, thank you!) |
|
I will try to pass the right session to the apiclient, how would this function work? |
We could copy what you've got at the top-level there into a function, and have it reach into each PyFxA sub-module and replace |
Natim
changed the title
Natim
force-pushed
the
use_gevent_if_present
branch
from
60998cc
to
a3862da
Compare
|
ping @rfk |
|
|
||
|
|
||
| from requests.packages import urllib3 | ||
| urllib3.disable_warnings() |
There was a problem hiding this comment.
I worry about accidentally disabling these in production by importing fxa.tests without realizing it. Can we put this into a module-level setup/teardown function so it only applies for the duration of the tests?
There was a problem hiding this comment.
Even if it was disabled in production it should not be a problem since it is only a warning related to the Python version.
Since it is used in every tests, I wouldn't add this in every testcase setUp. Should I create a BaseTestCase and inherit every FxA TestCase with it?
There was a problem hiding this comment.
Concretely, my concern is if we accidentally disable a warning about insecure SSL certificate checking or some other security-sensitive matter. If that's not a possibility then I'm happy to leave it as global disable. Maybe we can only disable this specific warning rather than all warnings?
Otherwise BaseTestCase seems like a good approach, but I don't want to create too much busywork for future test writers :-(
There was a problem hiding this comment.
According to the documentation you are right: https://urllib3.readthedocs.org/en/latest/security.html#insecureplatformwarning
Maybe we can setup PyOpenSSL usage for PyFxA as described here: https://urllib3.readthedocs.org/en/latest/security.html#pyopenssl
|
Otherwise LGTM! |
|
Updated. |
If gevent is activated (using monkey_patch) we have to use grequests instead of requests.
This fix works, but I am not a fan yet. I will try to figure out how we can check if gevent is activated instead.