> 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. > > Kenneth Giusti wrote: > 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). >
Sounds reasonable then. Or at least as reasonable as we can be given the circumstances. - Rafael ----------------------------------------------------------- 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 > >
