<span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)">Hi,</span><div style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)">
<br></div><div style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)"> Can you elaborate on the singleton approach ?</div><div style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)">
I thought the config was read-only already, and read before any thread is spawn, so I don't get why it would require a singleton class ?</div><div style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)">
That being said, I like the idea of a "depot" for the common, "behavior related" options.</div><div style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)">
Having to create a "key" for searching the "general" section of the config file is painful, and the number of such options is limited anyway, so it can be moved out the config object.</div><div style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)">
However, two point I don't like in your implementation:</div><div style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)">1) Not sure about the requirement of the Singleton class (and if required, let's put it in a Singleton.py file, not __init.py file)</div>
<div style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)">2) The Option object should be lowercase (all the other members are lowercase, so it's easy to use a "option" member instead of "Option"</div>
<div style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)"><br></div><div style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)">
Cheers,</div><div style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)">Cyril</div><br><div class="gmail_quote">On Sun, Jan 27, 2013 at 11:10 PM, Eygene Ryabinkin <span dir="ltr"><<a href="mailto:rea@codelabs.ru" target="_blank">rea@codelabs.ru</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>Sun, Jan 27, 2013 at 08:39:13PM +0100, Nicolas Sebrecht wrote:<br>
> Can't say whether the patch really fixes the issue<br>
<br>
</div>It should and in my testing -- is, but I am still waiting for Cyril's<br>
review.<br>
<div><br>
> but it's very pleasant to see work like that and activity on the<br>
> repository in general (like Dmitrijs did recently).<br>
<br>
</div>Thanks!<br>
<div><br>
> > One thing that worries me currently is the way that options.singlethreading<br>
> > is propagated from the main code: adding new pseudo-item to the 'general'<br>
> > configuration section isn't that good. Something like the singleton that<br>
> > will provide read-only access to the options is a better way, in my view.<br>
><br>
> Well, as long as contributors and users are aware of what's going on and<br>
> as long as whatever is released as stable is not experimental stuff,<br>
> everything should be ok.<br>
<br>
</div>Yes. I had implemented read-only depot for options, the modified patch<br>
is at<br>
<a href="http://codelabs.ru/patches/offlineimap/2013-fix-deadlock-singlethreaded-IMAP-sync.diff" target="_blank">http://codelabs.ru/patches/offlineimap/2013-fix-deadlock-singlethreaded-IMAP-sync.diff</a><br>
<br>
It additionally teaches folder/IMAP.py not to suggest multithreading while<br>
OfflineIMAP works in single-threaded mode: it is simple, because now we<br>
have global-like object that has all options.<br>
<div><br>
> So, about this subtle and critical topic I'd say it's fine to go step by<br>
> step. This would prevent from non-required headaches and could later<br>
> help to distinguish newly introduced issues coming from the "core" fix<br>
> and the "polishing" commit.<br>
<br>
</div>Well, I had missed this paragraph while I was polishing my commit. Will<br>
split it into two tomorrow.<br>
<span><font color="#888888">--<br>
rea<br>
</font></span><div><div><br>
_______________________________________________<br>
OfflineIMAP-project mailing list<br>
<a href="mailto:OfflineIMAP-project@lists.alioth.debian.org" target="_blank">OfflineIMAP-project@lists.alioth.debian.org</a><br>
<a href="http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/offlineimap-project" target="_blank">http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/offlineimap-project</a><br>
<br>
OfflineIMAP homepage: <a href="http://software.complete.org/offlineimap" target="_blank">http://software.complete.org/offlineimap</a><br>
</div></div></blockquote></div><br>