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
_______________________________________________
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
