[PATCH 6/6] Re: Handle abort messages from GMail

[Nicolas Sebrecht]

On Wed, May 18, 2011 at 08:48:57AM -0400, Ethan Glasser-Camp wrote:

> diff --git a/offlineimap/imapserver.py b/offlineimap/imapserver.py
> index 5f697b7..b24f355 100644
> --- a/offlineimap/imapserver.py
> +++ b/offlineimap/imapserver.py
> @@ -406,10 +406,17 @@ class IdleThread(object):
>              if self.event.isSet():
>                  return
>              self.needsync = False
> +            self.imapaborted = False
>              def callback(args):
> -                if not self.event.isSet():
> -                    self.needsync = True
> -                    self.event.set()
> +                if args[2] is None:

Nitpick: this args[2] is not very obvious. What is args[2]?

A little bit comments or a temporary variable with a good name could
help, I think...

> +                    if not self.event.isSet():
> +                        self.needsync = True
> +                        self.event.set()
> +                else:
> +                    # We got an "abort" signal.

... Oh, this looks like the comment I was looking for. :-)

Could it be reviewed and set at a bette place?

> +                    self.imapaborted = True
> +                    self.stop()
> +
>              imapobj = self.parent.acquireconnection()
>              imapobj.select(self.folder)
>              if "IDLE" in imapobj.capabilities:
> @@ -418,7 +425,11 @@ class IdleThread(object):
>                  imapobj.noop()
>              self.event.wait()
>              if self.event.isSet():
> -                imapobj.noop()
> +                if not self.imapaborted:
> +                    # Can't NOOP on a bad connection.
> +                    imapobj.noop()

I'm puzzled. :-/

We say we can't noop() but we do?

> +                    # We don't do event.clear() so that we'll fall out
> +                    # of the loop next time around.
>              self.parent.releaseconnection(imapobj)
>              if self.needsync:
>                  self.event.clear()
-- 
Nicolas Sebrecht