<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On May 24, 2010, at 5:16 AM, Glyph Lefkowitz wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On May 23, 2010, at 2:37 AM, Brian Quinlan wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div>On May 23, 2010, at 2:44 PM, Glyph Lefkowitz wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On May 22, 2010, at 8:47 PM, Brian Quinlan wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>Jesse, the designated pronouncer for this PEP, has decided to keep discussion open for a few more days.<br><br>So fire away!<font class="Apple-style-span"><font class="Apple-style-span" color="#144FAE"><br></font></font></div></blockquote><br></div><div>As you wish!</div></div></blockquote><div><br></div>I retract my request ;-)<font class="Apple-style-span" color="#000000"><font class="Apple-style-span" color="#144FAE"><br></font></font></div></div></blockquote><div><br></div>May you get what you wish for, may you find what you are seeking :).<br><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>The PEP should be consistent in its usage of terminology about callables. It alternately calls them "callables", "functions", and "functions or methods". It would be nice to clean this up and be consistent about what can be called where. I personally like "callables".</div></div></blockquote><div><br></div><div>Did you find the terminology confusing? If not then I propose not changing it.</div></div></div></blockquote><div><br></div><div>Yes, actually. Whenever I see references to the multiprocessing module, I picture a giant "HERE BE (serialization) DRAGONS" sign. When I saw that some things were documented as being "functions", I thought that maybe there was intended to be a restriction like the "these can only be top-level functions so they're easy for different executors to locate and serialize". I didn't realize that the intent was "arbitrary callables" until I carefully re-read the document and noticed that the terminology was inconsistent.</div></div></div></blockquote><div><br></div>ProcessPoolExecutor has the same serialization perils that multiprocessing does. My original plan was to link to the multiprocessing docs to explain them but I couldn't find them listed.</div><div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div>But changing it in the user docs is probably a good idea. I like "callables" too.</div></div></div></blockquote><div><br></div><div>Great. Still, users will inevitably find the PEP and use it as documentation too.</div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>The execution context of callable code is not made clear. Implicitly, submit() or map() would run the code in threads or processes as defined by the executor, but that's not spelled out clearly.</div></div></blockquote></div></div></blockquote><div><br></div><div>Any response to this bit? Did I miss something in the PEP?</div></div></div></blockquote><div><br></div><div>Yes, the execution context is Executor-dependent. The section under ProcessPoolExecutor and ThreadPoolExecutor spells this out, I think.</div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>More relevant to my own interests, the execution context of the callables passed to add_done_callback and remove_done_callback is left almost completely to the imagination. If I'm reading the sample implementation correctly, <<a href="http://code.google.com/p/pythonfutures/source/browse/branches/feedback/python3/futures/process.py#241">http://code.google.com/p/pythonfutures/source/browse/branches/feedback/python3/futures/process.py#241</a>>, it looks like in the multiprocessing implementation, the done callbacks are invoked in a random local thread. The fact that they are passed the future itself *sort* of implies that this is the case, but the multiprocessing module plays fast and loose with object identity all over the place, so it would be good to be explicit and say that it's *not* a pickled copy of the future sitting in some arbitrary process (or even on some arbitrary machine).</div></div></blockquote><div><br></div>The callbacks will always be called in a thread other than the main thread in the process that created the executor. Is that a strong enough contract?<br></div></div></blockquote><div><br></div><div>Sure. Really, almost any contract would work, it just needs to be spelled out. It might be nice to know whether the thread invoking the callbacks is a daemon thread or not, but I suppose it's not strictly necessary.</div></div></div></blockquote><div><br></div><div>Your concerns is that the thread will be killed when the interpreter exits? It won't be.</div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>This is really minor, I know, but why does it say "NOTE: This method can be used to create adapters from Futures to Twisted Deferreds"? First of all, what's the deal with "NOTE"; it's the only "NOTE" in the whole PEP, and it doesn't seem to add anything. This sentence would read exactly the same if that word were deleted. Without more clarity on the required execution context of the callbacks, this claim might not actually be true anyway; Deferred callbacks can only be invoked in the main reactor thread in Twisted. But even if it is perfectly possible, why leave so much of the adapter implementation up to the imagination? If it's important enough to mention, why not have a reference to such an adapter in the reference Futures implementation, since it *should* be fairly trivial to write?</div></div></blockquote><div><br></div><div>I'm a bit surprised that this doesn't allow for better interoperability with Deferreds given this discussion:</div></div></div></blockquote><div><br></div><div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div><discussion snipped></div></div></div></blockquote><div><br></div></div><div>I did not communicate that well. As implemented, it's quite possible to implement a translation layer which turns a Future into a Deferred. What I meant by that comment was, the specification in the PEP was to loose to be sure that such a layer would work with arbitrary executors.</div><div><br></div><div>For what it's worth, the Deferred translator would look like this, if you want to include it in the PEP (untested though, you may want to run it first):</div><div><br></div><div> from twisted.internet.defer import Deferred</div><div> from twisted.internet.reactor import callFromThread</div><div><br></div><div><div> def future2deferred(future):</div><div> d = Deferred()</div><div> def invoke_deferred():</div><div> try:</div><div> result = future.result()</div><div> except:</div><div> d.errback()</div><div> else:</div><div> d.callback(result)</div><div> def done_callback(same_future):</div><div> callFromThread(invoke_deferred)</div><div> future.add_done_callback(done_callback)</div><div> return d</div><div><br></div><div>This does beg the question of what the traceback will look like in that except: block though. I guess the multi-threaded executor will use python3 exception chaining so Deferred should be able to show a sane traceback in case of an error, but what about exceptions in other processes?</div></div><div><br></div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>I suggest having have add_done_callback, implementing it with a list so that callbacks are always invoked in the order that they're added, and getting rid of remove_done_callback.</div></div></blockquote><div><br></div>Sounds good to me!<br></div></div></blockquote><div><br></div>Great! :-)</div><div><br></div><div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>futures._base.Executor isn't exposed publicly, but it needs to be. The PEP kinda makes it sound like it is ("Executor is an abstract class..."). Plus, A third party library wanting to implement an executor of its own shouldn't have to copy and paste the implementation of Executor.map.</div></div></blockquote><div><br></div><div>That was a bug that I've fixed. Thanks!</div></div></div></blockquote><div><br></div><div>Double-great!</div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>One minor suggestion on the "internal future methods" bit - something I wish we'd done with Deferreds was to put 'callback()' and 'addCallbacks()' on separate objects, so that it was very explicit whether you were on the emitting side of a Deferred or the consuming side. That seems to be the case with these internal methods - they are not so much "internal" as they are for the producer of the Future (whether a unit test or executor) so you might want to put them on a different object that it's easy for the thing creating a Future() to get at but hard for any subsequent application code to fiddle with by accident. Off the top of my head, I suggest naming it "Invoker()". A good way to do this would be to have an Invoker class which can't be instantiated (raises an exception from __init__ or somesuch), then a Future.create() method which returns an Invoker, which itself has a '.future' attribute.</div></div></blockquote></div></div></blockquote><div><br></div><div>No reaction on this part? I think you'll wish you did this in a couple of years when you start bumping into application code that calls "set_result" :).</div></div></div></blockquote><div><br></div>My reactions are mixed ;-)</div><div><br></div><div>Your proposal is to add a level of indirection to make it harder for people to call implementation methods. The downside is that it makes it a bit harder to write tests and Executors. I also can't see a big problem in letting people call set_result in client code though it is documented as being only for Executor implementations and tests. </div><div><br></div><div>On the implementation side, I don't see why an Invoker needs a reference to the future. Each Invoker could own one Future. A reference to the Invoker is kept by the Executor and its future is returned to the client i.e.</div><div><br></div><div>class Invoker(object):</div><div> def __init__(self):</div><div> """Should only be called by Executor implementations."""</div><div> self.future = Future()</div><div><br></div><div><div> def set_running_or_notify_cancel(self):</div><div> # Messes with self.future's internals</div><div><br></div><div><div> def set_result(self):</div><div><div> # Messes with self.future's internals</div><div><br></div></div><div><div> def set_exception(self):</div><div><div> # Messes with self.future's internals</div><div><br></div></div></div></div><div><br></div></div><div>Cheers,</div><div>Brian </div><div><br><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>Finally, why isn't this just a module on PyPI? It doesn't seem like there's any particular benefit to making this a stdlib module and going through the whole PEP process - except maybe to prompt feedback like this :).</div></div></blockquote><div><br></div><div>We've already had this discussion before. Could you explain why this module should *not* be in the stdlib e.g. does it have significantly less utility than other modules in stdlib? Is it significantly higher risk? etc?</div></div></div></blockquote><div><br></div><div>You've convinced me, mainly because I noticed later on in the discussion that it *has* been released to pypi for several months, and does have a bunch of downloads. It doesn't have quite the popularity I'd personally like to see for stdlib modules, but it's not like you didn't try, and you do (sort of) have a point about small modules being hard to get adoption. I'm sorry that this, my least interesting point in my opinion, is what has seen the most discussion so far.</div><div><br></div><div>I'd appreciate it if you could do a release to pypi with the bugfixes you mentioned here, to make sure that the released version is consistent with what eventually gets into Python.</div><div><br></div><div>Oh, and one final nitpick: <<a href="http://www.rfc-editor.org/rfc/rfc2606.txt">http://www.rfc-editor.org/rfc/rfc2606.txt</a>> says you really should not put real domain names into your "web crawl example", especially not "<a href="http://some-made-up-domain.com">some-made-up-domain.com</a>".</div><div><br></div></div></div>_______________________________________________<br>Python-Dev mailing list<br><a href="mailto:Python-Dev@python.org">Python-Dev@python.org</a><br>http://mail.python.org/mailman/listinfo/python-dev<br>Unsubscribe: http://mail.python.org/mailman/options/python-dev/brian%40sweetapp.com<br></blockquote></div><br></body></html>