Code review comment for lp:~julian-edwards/launchpad/poppy-warn-unsigned-bug-374019

(15:48:46) adeuring: bigjools: PoppyAnonymousShell.openForWriting() has a "catch-all" except. I think we do not want to catch KeyboardInterrupt and SystemExit. And I think we should log an error for other exceptions.
(15:49:00) adeuring: otherwise, the branch looks good, I think
(15:50:07) bigjools: adeuring: that was cargo-culted from the parent class... I wasn't sure what to do about it TBH
(15:51:40) adeuring: bigjools: admittedly, I am not familiar with twisted, so I am not sure about KBInterrupt and SystemExit. But nevertheless I think we should log other exceptions.
(15:52:54) adeuring: "eating" exceptions simply scares me ;)
(15:53:12) bigjools: adeuring: yeah
(15:54:27) bigjools: adeuring: so I'll re-raise KeyboardInterrupt and SystemExit
(15:54:44) bigjools: adeuring: oh wait
(15:55:23) bigjools: adeuring: yes twisted has confused us both momentarily - the defer.fail() is effectively the same as re-raising the exception, it gets dealt with later
(15:56:10) adeuring: bigjools: intersting. So, IOError and OSError are re-raised too?
(15:56:19) adeuring: ah, no, there no fail() call
(15:56:26) bigjools: adeuring: no, they are converted to ftp errors
(15:56:48) bigjools: ftp.errnoToFailure will return a defer.fail
(15:57:23) adeuring: bigjools: right, thanks for the explanation, so r=me. But perhaps a small comment that twisted properly deals with the exceptions? (just to avoid the same question I asked again)?
(15:57:36) bigjools: adeuring: yup, will do, thanks for the review