Singlethreading patch series

[Johannes Stezenbach]

On Wed, Jan 12, 2011 at 10:39:33AM +0100, Sebastian Spaeth wrote:
> On Mon, 10 Jan 2011 12:08:34 +0100, Johannes Stezenbach <js at sig21.net> wrote:
> > IMHO your patches 1...3 do not fix the no-traceback issue
> > (leaving the catch-all except: + self.ui.warn in place),
> > and litter the code with KeyboardInterrupt special handling.
> > I would argue that catch-all except: is almost always a bug,
> > and if you fix that we'd end up with a much better patch.
> 
> I agree that they do not fix the no-traceback issue completely yet, but
> I was told (by you :-)) to not overwhelm the maintainers with too many
> patches and changes. So let's do baby steps. This patch series allows us
> to run all the code in a single thread and it allows us to abort the run
> with ctrl-c.
> 
> This is an improvment.
> 
> I do not like the catch-it-all exceptions either but they were already
> there in the code, and I agree with Nicolas that we should probably deal
> with those in a separate patch series after this one has gone into the
> code.

Um, well, my thought was to _replace_ patches 1..3 with one simpler patch.
However, it means you would need to rework your patch series, not add another
patch on top of it.  But I currently do not have time to work on
offlineimap myself, I can only offer my opinion.  It's up to you
and Nicolas what you make of it.

Basically commit 30344587 fixed one problem and introduced
another one.  And if that regression is fixed your whole
KeyboardInterrupt and SystemExit issue does not exist anymore.

> IMHO, we should only catch exceptions where they are either
> a) expected and harmless or
> b) raising an exception would lead to a weird traceback rather than a
> nice error message (your example of the interrupted network connetion
> was pretty good).
> 
> In any case, we should never catch all possible exceptions but only
> those that we expect to be raised at a certain point.
> 
> Does this sound like a good plan?

In summay, one could say exception handling is for
_handling_ exceptions, not for ignoring them.
Unless you want to ignore them on purpose, of course ;-)


Thanks,
Johannes