Fix #607: make Tornado coroutines serializable #673
Conversation
As a bonus, serializing coroutines is now much faster and produces a smaller payload.
|
The reason to avoid putting this into cloudpickle directly is that tornado is a bit of a special case? |
|
I don't know, it depends on @rgbkrk's preference. The code involves a fair amount of hacks :-) |
|
I think that cloudpickle is entirely hacks. Also, there isn't really a single core maintainer of cloudpickle. It's a bunch of people like us that have some small amount of need for the project. You are about as likely to maintain cloudpickle in the future as anyone else. |
|
Still, I don't have push access to cloudpickle, so the current maintainers would have to give their opinion whether they'd like to support this kind of code (possibly a bit more hackish, since we have to avoid importing tornado unconditionally in cloudpickle). |
| Return whether *func* is a Tornado coroutine function. | ||
| Running coroutines are not supported. | ||
| """ | ||
| return func.__code__ is _tornado_coroutine_sample.__code__ |
There was a problem hiding this comment.
How reliable is this check? What sort of change in @gen.coroutine would break this is check?
There was a problem hiding this comment.
It would need gen.coroutine to return something else than a wrapper function. Apparently returning a wrapper function has been the case since the beginning, see tornadoweb/tornado@58b0dab
|
I'd be AOK with supporting tornado coroutines in cloudpickle if I were more confident in the detection of the coroutine (i.e. if tornadoweb/tornado#1890 is merged). I'd love it even more if tornado itself handled the pickle/unpickle of coroutines. |
|
cc @ajdavis |
I can understand that. From distributed's point of view, if tornadoweb/tornado#1890 isn't merged before the Tornado 4.5 release, we'll need our own workaround. |
|
On the good news front, tornadoweb/tornado#1890 has now been merged. |
|
Great news! |
|
I've submitted cloudpipe/cloudpickle#69 |
|
This is superseded by the aforementioned cloudpickle PR. Closing. |
4 participants
As a bonus, serializing coroutines is now much faster and produces a smaller payload.