> On May 30, 2014, 1:07 p.m., Rafael Schloming wrote: > > /trunk/qpid/python/qpid/compat.py, line 50 > > <https://reviews.apache.org/r/22024/diff/1/?file=598726#file598726line50> > > > > Isn't this check dependent on the order of importing with respect to > > monkey patching? What happens if the monkey patching happens after > > compat.py gets imported? > > > > Given that in general it can be difficult to control the order in which > > imports happen, I suspect this could appear to mysteriously break if > > compat.py happens to get imported sooner rather than later. I would either > > do this check lazily, which unfortunately adds check overhead everytime > > select is invoked, or alternatively add a mechanism for explicitly > > controlling which implementation is chosen.
You are 100% correct - however I suspect the effort/complexity not worth it in light of the eventlet documentation for monkey-patching: (see http://eventlet.net/doc/patching.html#monkey-patch) Specifically from that page: "It is important to call monkey_patch() as early in the lifetime of the application as possible. Try to do it as one of the first lines in the main module. The reason for this is that sometimes there is a class that inherits from a class that needs to be greened – e.g. a class that inherits from socket.socket – and inheritance is done at import time, so therefore the monkeypatching should happen before the derived class is defined. It’s safe to call monkey_patch multiple times." So I believe that the "correct" use case will be to monkey patch -before- importing qpid. In general, this whole 'monkey-patching' thing gives me the willies. I don't want to imply anything other than the recommended use case is going to work (eg. late monkey-patching won't be supported). - Kenneth ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22024/#review44359 ----------------------------------------------------------- On May 30, 2014, 12:47 p.m., Kenneth Giusti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22024/ > ----------------------------------------------------------- > > (Updated May 30, 2014, 12:47 p.m.) > > > Review request for qpid, Gordon Sim, Rafael Schloming, and Ted Ross. > > > Bugs: qpid-5790 > https://issues.apache.org/jira/browse/qpid-5790 > > > Repository: qpid > > > Description > ------- > > Modify the python client to use select.select() instead of select.poll() if > the application has been monkey-patched by Eventlet/Greenthreads. > > Eventlet only properly patches select.select(), not select.poll(). If > select.poll() is used, the application may hang. This patch detects if the > application is using a patched select, and uses that instead of poll. > > > Diffs > ----- > > /trunk/qpid/python/qpid/compat.py 1598260 > > Diff: https://reviews.apache.org/r/22024/diff/ > > > Testing > ------- > > Tested the python client: > > 1) running it in an environment that did not have eventlet installed - poll > used > 2) eventlet was installed, but select was not patched - poll used > 3) select was patched - patched select was used. > > > Thanks, > > Kenneth Giusti > >
