[Python-Dev] PEP 3148 ready for pronouncement

[Brian Quinlan]

On May 23, 2010, at 2:44 PM, Glyph Lefkowitz wrote:

>
> On May 22, 2010, at 8:47 PM, Brian Quinlan wrote:
>
>> Jesse, the designated pronouncer for this PEP, has decided to keep  
>> discussion open for a few more days.
>>
>> So fire away!
>
> As you wish!

I retract my request ;-)

> 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".

Did you find the terminology confusing? If not then I propose not  
changing it.

But changing it in the user docs is probably a good idea. I like  
"callables" too.

>
> 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.
>
> 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, <http://code.google.com/p/pythonfutures/source/browse/branches/feedback/python3/futures/process.py#241 
> >, 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).

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?

> 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?

I'm a bit surprised that this doesn't allow for better  
interoperability with Deferreds given this discussion:

At 02:36 PM 3/16/2010 -0700, Brian Quinlan wrote:

"""
 From P.J Eby:
> On Mar 7, 2010, at 11:56 AM, P.J. Eby wrote:
>
>> At 10:59 AM 3/7/2010 -0800, Jeffrey Yasskin wrote:
>>> Given a way to register "on-done" callbacks with the future, it  
>>> would be straightforward to wait for a future without blocking, too.
>>
>> Yes, and with a few more additions besides that one, you might be  
>> on the way to an actual competitor for Deferreds.  For example:  
>> retry support, chaining, logging, API for transparent result  
>> processing, coroutine support, co-ordination tools like locks,  
>> sempaphores and queues, etc.
>
> OK, but lets just think about making the APIs compatible e.g. you  
> have some code that uses Futures and now you want to integrate it  
> with some code that uses Deferreds.
>
> I think Jeff's suggestion of having a completion callback on Futures  
> would make it possible to write a Future-to-Deferred adapter. Is  
> that correct?

As long as the callback signature included a way to pass in an error,  
then yes, that'd probably be sufficient.
"""

If add_done_callback doesn't help with twisted interoperability then  
I'd suggest removing it to allow for something that may be more useful  
to be added later.

>
> The fact that add_done_callback is implemented using a set is weird,  
> since it means you can't add the same callback more than once.  The  
> set implementation also means that the callbacks get called in a  
> semi-random order, potentially creating even _more_ hard-to-debug  
> order of execution issues than you'd normally have with futures.   
> And I think that this documentation will be unclear to a lot of  
> novice developers: many people have trouble with the idea that "a =  
> Foo(); b = Foo(); a.bar_method != b.bar_method", but "import  
> foo_module; foo_module.bar_function == foo_module.bar_function".
>
> It's also weird that you can remove callbacks - what's the use  
> case?  Deferreds have no callback-removal mechanism and nobody has  
> ever complained of the need for one, as far as I know.  (But lots of  
> people do add the same callback multiple times.)
>
> 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.

Sounds good to me!

> 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.

That was a bug that I've fixed. Thanks!

>
> 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.
>
> 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 :).

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?


> Issues like the ones I'm bringing up could be fixed pretty  
> straightforwardly if it were just a matter of filing a bug on a  
> small package, but fixing a stdlib module is a major undertaking.

True but I don't think that is a convincing argument. A subset of the  
functionality provided by this module is already available in Java and  
C++ and (at least in Java) it is used extensively and without too much  
trouble. If there are implementation bugs then we can fix them just  
like we would with any other module.

Cheers,
Brian


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/python-dev/attachments/20100523/0751b7f5/attachment-0001.html>