Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

gevent patch_all call in grequests breaks pymysql, etc. #8

Open
vitaly-krugl opened this Issue · 23 comments
@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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.