<br><br><div class="gmail_quote">On Tue, Jan 29, 2013 at 6:44 AM, 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">
Sure, my last patch<br>
  <a href="http://codelabs.ru/patches/offlineimap/2013-IMAP-dont-suggest-multithreading-in-singlethreaded-mode.diff" target="_blank">http://codelabs.ru/patches/offlineimap/2013-IMAP-dont-suggest-multithreading-in-singlethreaded-mode.diff</a><br>

currently makes the second hint to be consistent with the<br>
config.singlethreading.  But since some other, future folder variant<br>
can (errorneously) suggest mutithreaded processing every time, the<br>
shorter 'if' variant will be broken again.<br></blockquote><div>Ok, point taken. </div><div>But at the same time, the number of people changing code will be small, so it's likely we'll spot a change in the suggestthreads of any folder variant.</div>
<div>I'm inclined to prefer the simple "if" variant anyway, since spreading config testing everwhere is, IMHO a bad design anyway and hard to maintain. So the improved safety here might not worth it.  </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
We could try to solve this problem by forcing suggestthreads() to return<br>
zero when config.singlethreading is True, but for me it is a slight<br>
deviation from the semantics of suggestthreads().<br></blockquote><div><br></div><div>Not sure about this. If you're asking the software to use a single thread mode, then the class should never suggest threads. </div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
I had been thinking of the shorter 'if' variant when I was extending<br>
your patch, but came to the conclusion that the explicit check for being<br>
single-threaded will be better: it has the proper semantics, it is more<br>
explicit and it coincides with the way the account.py was modified, so<br>
this provides some uniformity of code.<br>
<br>
That was my reasoning to be explicit.<br>
<div class="im"><br></div></blockquote><div>Again, I agree with your point. </div><div>However, I still don't like config testing everywhere in the code as it's a pain to maintain and document.</div><div>Let's see below how to solve this.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
<br></div>No, there is only one instance of any singleton -- that's the "meat"<br>
of this design pattern.  At any time, depot.Options() ==<br>
depot.Options, and in our pythonic case the magic is in the __new__<br>
method that just saves the first instance and returns it every time<br>
when a new instantiation is tried to be made.<br></blockquote><div><br></div><div>Well, I disagree. You are making a second instance which refers to the same object.</div><div>The implementation details are details, which will never be read by a casual python developer (who's is going to check a file called __init.py to figure out the "trick" of the singleton instance?).</div>
<div><br></div><div>It's currently confusing when you read the code that's using the depot.Option() instance.</div><div>Confusing about the case, and confusing about the logic (if I don't know how the Option() is implemented, which is not usual) of the object usage.</div>
<div><br></div><div>  </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">I am not trying to fight the changes of self.__options in the same class:</div>
such code is local and, probably, programmer will know what is going on.</blockquote><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

I am trying to protect the stored options from unintentional changes<br>
from the outside of the depot to avoid people to modify options from<br>
the code that isn't in offlineimap/init.py by accident or without<br>
careful evaluation of consequences.  May be this is an overkill.<br>
<div class="im"><br></div></blockquote><div>Ok, what do you think of the later:</div><div>1) Split config.get/set interface, in more rigid, global, config.general, config.repositories, config.accounts objects</div><div>   Pro: Documenting the config option is easier by simply grepping the code, instead of having to run through a debugger to spot each get() usage.</div>
<div>   Con: Rewrite of the config usage in the code</div><div>2) Identify which part should be const, and which should not (I think everything is supposed to be const, but I'm probably missing something)</div><div>3) Constify the config part that are supposed to be const (again, I don't get the need of a singleton here, compared to a global variable)</div>
<div>   Pro: Will raise const error if any code is doing config mixup, so we can check any "unusual" pattern / bug</div><div>   Con: I don't see any. A developer does not need to know the implementation details of a const object (it appears as a usual object), and an exception is easy to track and debug.</div>
<div>4) Add a config.runtime const object that's made for runtime passed arguments (like singlethread, singleaccount, etc..) and let the code refer to this.</div><div>   Pro: It's cleaner than the config "general" section hack I've made for singlethread mode, and the code will be easier to maintain.</div>
<div>   Con: Need some code refactoring, but not much</div><div><br></div><div>Cheers,</div><div>Cyril</div></div>