gevent patch_all call in grequests breaks pymysql, etc.

[vitaly-krugl]

I was looking forward to using this awesome package with my app and gevent until I noticed that it accomplishes gevent-compatibility via gevent.monkey.patch_all():

try:
    import gevent
    from gevent import monkey as curious_george
    from gevent.pool import Pool
except ImportError:
    raise RuntimeError('Gevent is required for grequests.')

# Monkey-patch.
curious_george.patch_all(thread=False, select=False)

Unfortunately, patch_all() impacts all other packages/modules used by the app, breaking some of them that were not designed for reentrancy. For example, gevent.monkey.patch_all() broke the combination of pymysql/pooled_db (with sporadic, hard-to-reproduce failures); my app has several greenlets that on occasion make pymysql calls; I isolated the problem to gevent's monkey-patching of the socket module in that case.

A local solution involving monkey-patching of just the requests package to use gevent's variants of blocking API's would greatly enhance compatibility of grequests with apps and eliminate unexpected/undesirable side-effects on other packages. For example, I recently monkey-patched HBase/Thrift via the following code snippet with the desired effect on Thrift and no harm to the rest of the app (pymysql, etc.):

# Monkey-patch Thrift's TSocket to use gevent.socket to make our HBase interface
# gevent-friendly
import gevent.socket
from thrift.transport import TSocket
TSocket.socket = gevent.socket

gevent-zeromq is another example of local monkey-patching, albeit a more complex one: https://github.com/traviscline/gevent-zeromq/blob/master/gevent_zeromq/__init__.py

Thank you,
Vitaly

[hjwp]

I think I'm experiencing a similar issue - just used grequests for a django management command, and started seeing gevent threading errors in other, unrelated parts of the app.

[vitaly-krugl]

This breakage of other modules is what's still preventing me from using grequests.

[i-trofimtschuk]

I am experiencing similar issues when i try to use grequests in a celery task.
The error i get is:
NotImplementedError: gevent is only usable from a single thread

What do you think about Vitalys suggestion?

[katrielalex]

The monkeypatching also breaks PyDev's debugger, which appears to use ordinary sockets. It would be much nicer if requests could be patched, instead of all of Python.

[kennethreitz]

I'm leaning more and more towards recommending people use celery

[hjwp]

really? wouldn't that involve installing a message broker? Seems a little heavyweight.... surely there's an option to use gevent without hitting the nuclear patch all option?

(i understand it's easy to criticise, if it's so easy why don't i just go ahead and write it, pull request gratefully accepted etc etc..)

[kennethreitz]

programming is hard?

[kennethreitz]

Sorry :) I agree that it's not ideal.

[hjwp]

ow. just took a look at the gevent api. agree it's far from straightforward!

[katrielalex]

From a cursory glance, it looks like grequests could patch the version of socket imported by requests.urllib3 (e.g. requests.packages.urllib3.connectionpool.SocketTimeout) -- I have no idea whether that would work, though. Will try to investigate if I can find time.

[hjwp]

Could you use some kind of selective monkeypatch, where you only patch out the socket from requests.models? I would use the mock library's patch, but that's just because it's what I know...

[vitaly-krugl]

I find it really problematic for stability when an imported module all of a sudden changes so much of the environment for everything else that your app does. It's akin to having the rug pulled from under your feet :)

[kennethreitz]

It's intended to be used standalone. Anything with Gevent is.

[vitaly-krugl]

My app uses pymysql, PooledDB, Thrift client, Haigha AMQP library, etc., and it also needs a gevent-compatible HTTP client lib that doesn't mess up the app's environment. Presently, I'm working around this by running HTTP client from a separate posix thread, but I hate the unnecessary level of complexity and the extra kernel-level context switches that this entails. The app is I/O-bound, so it should be able to easily handle everything inside a single posix thread.

@kennethreitz Regarding "It's intended to be used standalone. Anything with Gevent is.": the same functionality can be accomplished by monkey-patching the underlying requests module instead of monkey-patching the entire app's environment. Perhaps the requests module can formalize this by providing an API function that takes the necessary patch-points as args and applies them, and grequests would simply call that function during import. What do you think?

[piotr-dobrogost]

+1 for idea of local monkey-patching

I'm leaning more and more towards recommending people use celery

@kennethreitz
How does celery relate to the problem of application global monkey-patching to support gevent?

[kenshinx]

Same question as @piotr-dobrogost
Use gevent in one celery task, lead many other type task use multi-thread parallel become serial 。
I think all the issues incude by gevent global monkey-patching .

[nonagonal]

In case it's helpful to others, I was importing grequests in some code which got used by many different processes. grequests was only actually used in one of the processes, but the import itself broke things (mod_wsgi+flask+threading in my case) as described above.

My solution was to use a modified version of grequests.py that contains this at the top:

def monkey_patch():
    """Perform monkey-patching if it hasn't been done already."""
    if not hasattr(monkey_patch, 'patched'):
        from gevent import monkey as curious_george
        curious_george.patch_all(thread=False, select=False)
        monkey_patch.patched = True

and in AsyncRequest's init method I call:

    monkey_patch()   # make sure we're monkey-patched

This will defer the monkey-patching until it's actually needed, which was enough to solve my problem. In my case I'm only actually using grequests in a very simple process with one thread that does nothing else, but I can imagine that this solution won't work in other more complex cases.

[dmk23]

Why is this still not fixed? Seems like lots of people cannot use grequests because it breaks lots of things. Is there any real need to monkey-patch anything beside sockets in requests module?

[vitaly-krugl]

A friendlier solution is for a module to use the appropriate geven objects directly instead of patching anything at all. requests could easily provide the necessary hooks for something like grequests to create gevent-compatible sockets, etc. directly. This way, gevent modules can coexist with legacy modules in the same app.

[dmk23]

@vitaly-krugl That should be fine too, though seems like it would require more extensive changes to requests. Has anyone taken a stab at forking / coding anything for this issue?

P.S. Regarding @nonagonal 's solution, it did not work for me, since the application needs to run continuously and use sockets in multiple ways from multiple processes / tasks. Because of that the problem has been quite hard to detect in the first place...

[parautenbach]

Just to add on: I've been using paramiko in a project, and recently added grequests. These two cannot co-exist either. Python 2.7.

[jpotterm]

This library does what you're asking for: https://github.com/gwik/geventhttpclient. I thought I'd mention it as an alternative until this gets fixed. Also, if anyone's working on implementing this feature, you can take tips from that library.

[dmk23]

Check out my latest solution posted here #55 (comment), I wonder if it works for you guys or you could find any issues / point out any problems with what I've done there.

@nonagonal: I see your solution uses monkey_patch() without excepting thread and select option. That would be the real reason it was working - could you validate?

[vcarel]

I'm coming back on this issue, because gevent 1.1 introduced a warning message when monkey_patch is called multiple times:

grequests.py:21: RuntimeWarning: Patching more than once will result in the union of all True parameters being patched
curious_george.patch_all(thread=False, select=False)

I don't think libs should do monkey patch by themselves. If this is a requirement (and this is one obviously), I'd rather get a warning saying that "select", "socket", etc. should be monkey patched so that grequests can work.
Regarding the above warning, imagine that there is another lib doing the same monkey patch. I would get the same warning! It doesn't make sense...

Sure, it's only a warning and it doesn't hurt much. But nobody likes useless warnings, and I'm getting this one at every unit test, every run and in my production logs.

Please reconsider your decision to close this issue...