pushyrpc / pushy

Easy-as RPC. Zero-server RPC for Python and Java.
http://github.com/pushyrpc/pushy
45 stars 18 forks source link

Catch I/O Errors on ValueError exception #42

Closed alfredodeza closed 11 years ago

alfredodeza commented 11 years ago

We are seeing some errors from threads that are not able to write to the stream because the connection is closing the file descriptors before the threads are joined (as seen in protocol.ssh.NativePopen.close ).

The connection object is already catching IOError but ValueError can also be raised for some IO Errors as well.

I am making sure that we only catch that exact issue ('I/O operation on closed file') otherwise the exception is re-raised (if for whatever reason a different ValueError is raised).

axw commented 11 years ago

Looks good, thank you!

alfredodeza commented 11 years ago

@axw do you have a release in mind? We are obviously interested in using this fix :)

axw commented 11 years ago

I haven't had anything planned, as I've not been actively working on pushy for a while now. I can release a minor version (0.5.2), but I probably won't have time until this weekend at the earliest. Is your project bundling pushy?

May I ask, what are you using pushy for? I see you have a Ceph-related repo; are you working on ceph-deploy?

alfredodeza commented 11 years ago

@axw yep, I am working on ceph-deploy and one of the issues we have often is this ValueError coming up (very very difficult to replicate as it involves threads).

I might be able to just wait until you push a new release (minor version is just fine).

axw commented 11 years ago

@alfredodeza I have uploaded a new version (0.5.2) to PyPI, and launchpad.

alfredodeza commented 11 years ago

Thank you! w00t!

axw commented 11 years ago

@alfredodeza If you're going to be bundling pushy with ceph-deploy, please be aware Issue #43, raised today/last night. This affects only Windows by default. If you pass "use_native=False", it'll also exercise the problem on Unix.

axw commented 11 years ago

FYI, I have uploaded a new release, 0.5.3, which fixes #43.

alfredodeza commented 11 years ago

Thank you for the update