[PATCH 4/4] Re: Implement single threading mode

Nicolas Sebrecht nicolas.s-dev at laposte.net
Wed Dec 22 23:02:28 GMT 2010


On Wed, Dec 22, 2010 at 03:27:20PM +0100, Sebastian Spaeth wrote:
> 
> Don't sync accounts in subthreads when:
>  1) option singlethreading has been chosen

Ok.

>  2) There is only 1 account to sync

Is this what we want to? I still have no idea of how subthreads is
implemented and used. Why couldn't we use subthreads for 1 account sync?

>  3) maxacountsync setting is 1
> 
> Signed-off-by: Sebastian Spaeth <Sebastian at SSpaeth.de>
> ---
>  offlineimap/init.py       |   61 +++++++++++++++++++++++++++++---------------
>  offlineimap/syncmaster.py |    4 +--
>  2 files changed, 41 insertions(+), 24 deletions(-)
> 
> diff --git a/offlineimap/init.py b/offlineimap/init.py
> index 828d696..db27bd4 100644
> --- a/offlineimap/init.py
> +++ b/offlineimap/init.py
> @@ -17,10 +17,6 @@
>  #    Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
>  
>  import imaplib
> -from offlineimap import imapserver, threadutil, version, syncmaster, accounts
> -from offlineimap.localeval import LocalEval
> -from offlineimap.threadutil import InstanceLimitedThread, ExitNotifyThread
> -from offlineimap.CustomConfig import CustomConfigParser
>  from optparse import OptionParser
>  import re, os, sys
>  from threading import *
> @@ -28,6 +24,10 @@ import threading, socket
>  import signal
>  import logging
>  import offlineimap
> +import offlineimap.accounts
> +from offlineimap.threadutil import InstanceLimitedThread, ExitNotifyThread
> +from offlineimap.localeval import LocalEval
> +from offlineimap.CustomConfig import CustomConfigParser

You're doing odd things here regarding to your commit message. To be
perfect, it should be a dedicated patch. That said, I'm not as strict as
what you may hit in other projects. You've already might have notice. :)

<...>

> @@ -321,14 +318,24 @@ class OfflineImap:
>              signal.signal(signal.SIGUSR1,sig_handler)
>              signal.signal(signal.SIGUSR2,sig_handler)
>      
> -            threadutil.initexitnotify()
> -            t = ExitNotifyThread(target=syncmaster.syncitall,
> +            #various initializations that need to be performed:
> +            offlineimap.threadutil.initexitnotify()       #TODO: Why?

So, here is my question: why?

<...>

> diff --git a/offlineimap/syncmaster.py b/offlineimap/syncmaster.py
> index 2da8e3e..5a0a2fb 100644
> --- a/offlineimap/syncmaster.py
> +++ b/offlineimap/syncmaster.py

<...>

> @@ -33,11 +32,10 @@ def syncaccount(threads, config, accountname, siglisteners):
>      thread.setDaemon(1)
>      thread.start()
>      threads.add(thread)
> -    
> +

Thanks! I like this kind of whitespaces fixes.

Otherwise, the patch looks good to me.

-- 
Nicolas Sebrecht




More information about the OfflineIMAP-project mailing list