classification
Title: asynchat push_callable() patch
Type: enhancement Stage: patch review
Components: Library (Lib) Versions: Python 3.2
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: giampaolo.rodola Nosy List: BreamoreBoy, giampaolo.rodola, josiah.carlson, josiahcarlson, nirs, pitrou, r.david.murray
Priority: normal Keywords: patch

Created on 2010-08-26 16:19 by giampaolo.rodola, last changed 2014-06-16 10:09 by giampaolo.rodola. This issue is now closed.

Files
File name Uploaded Description Edit
asynchat-push-callable.patch giampaolo.rodola, 2010-08-26 16:19 review
asynchat-push-callable.patch giampaolo.rodola, 2010-08-30 19:08 review
Messages (7)
msg115001 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-08-26 16:19
This can be useful in those circumstances where the user wants to execute some action right after some data has been sent (push()) or a producer has been consumed (push_with_producer()).
Example:


def log(msg):
    logging.debug(msg)

class Sender(asynchat.async_chat):

    def __init__(self, conn):
        asynchat.async_chat.__init__(self, conn)
        self.set_terminator("\r\n")
        self.push("220 hello\r\n")
        self.push_callable(log, "hello has been sent")
msg115239 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-08-30 19:08
Updated patch which fixes a little test error.
msg115241 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-08-30 20:13
Some comments:

- the signature in the doc is not the same as in the code: (fun, args, kwds) instead of (fun, *args, **kwds)
- I don't understand what _Callable is used for; why not just a tuple?
(or a function if you prefer)
- if you use _Callable, then why do you write "hasattr(first, '__call__')" rather than simply "isinstance(first, _Callable)"?
- why override __bool__??

The API also looks a bit weird to me - the Twisted model of Deferreds is so much better - but why not.
msg115273 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-08-31 19:18
Thanks for the comments.

> I don't understand what _Callable is used for; why not just a tuple?

_Callable is just a container to store function, args and kwargs objects.
I thought it made more sense than using a tuple because it's intrinsically callable.

> - if you use _Callable, then why do you write "hasattr(first, 
> '__call__')" rather than simply "isinstance(first, _Callable)"?

For flexibility, in the remote case the user wants to override _Callable class.  It's a corner case though, not really important.

> - why override __bool__??

I did that for performance.
Considering that initiate_send() method is called many times, and that it is almost always used to send data rather than firing functions I wanted to avoid to call isinstance(first, _Callable) (or hasattr(first,  '__call__')) on every loop.
By making __bool__ return False we achieve this, see:

            first = self.producer_fifo[0]
            # handle empty string/buffer or None entry
            if not first:
                ...
            # code to handle data to be sent follows...
            ...
       

> The API also looks a bit weird to me - the Twisted model of Deferreds 
> is so much better - but why not.

The API follows the exact same approach as push(), push_with_producer() and close_when_done() methods.
It adds "something" to a fifo and that "something" will be processed (called) only when the previous producers are exhausted, respecting the order of what has been "pushed" first and later.
This is different than a deferred.

A deferred is a callback that will be fired at a later time and that is related to the main loop (the reactor).
push_callable() is a callback that will be fired when all previous producers will be exhausted firsts and is related to the connection itself, not the main loop (if the connection gets closed the callback will be discarded).

Something similar to Twisted deferreds is this:
http://bugs.python.org/issue1641
...which, despite apparently similar, is different than this.
msg115280 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-08-31 22:23
> > - why override __bool__??
> 
> I did that for performance.
> Considering that initiate_send() method is called many times, and that
> it is almost always used to send data rather than firing functions I
> wanted to avoid to call isinstance(first, _Callable) (or
> hasattr(first,  '__call__')) on every loop.

This looks hackish... You should at least make the purpose very clear in
comments to the code.
Also, really, isinstance(..., _Callable) is better than the hasattr()
call, IMO.

> The API follows the exact same approach as push(),
> push_with_producer() and close_when_done() methods.
> It adds "something" to a fifo and that "something" will be processed
> (called) only when the previous producers are exhausted, respecting
> the order of what has been "pushed" first and later.
> This is different than a deferred.

Well, a deferred would be fired when a given piece of data has been sent
(or failed to send, in which case the errback would be run rather than
the callback). This is in the end equivalent, since I suppose the send
queue is sequential and never gets reordered.

In any case, I guess it's too late to change the overall design. I just
wanted to point out that the Deferred concept unified a lot of use cases
under a simple convenient abstraction. It avoids having to define ad hoc
methods such as push_callable().

> Something similar to Twisted deferreds is this:
> http://bugs.python.org/issue1641

This has nothing to do with Deferreds actually :)
msg220657 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-06-15 18:08
Is this ever likely to get applied given the arrival of asyncio?
msg220709 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2014-06-16 10:09
Yeah, I don't think this is worth it anymore. Closing as won't fix.
History
Date User Action Args
2014-06-16 10:09:16giampaolo.rodolasetstatus: open -> closed
assignee: giampaolo.rodola
resolution: wont fix
messages: + msg220709
2014-06-15 18:08:19BreamoreBoysetnosy: + BreamoreBoy
messages: + msg220657
2010-08-31 22:23:02pitrousetmessages: + msg115280
2010-08-31 19:18:55giampaolo.rodolasetkeywords: patch, patch

messages: + msg115273
2010-08-30 20:13:20pitrousetkeywords: patch, patch

messages: + msg115241
2010-08-30 19:08:13giampaolo.rodolasetkeywords: patch, patch
files: + asynchat-push-callable.patch
messages: + msg115239
2010-08-26 17:33:47giampaolo.rodolasetkeywords: patch, patch
nosy: + pitrou
2010-08-26 16:19:51giampaolo.rodolacreate