classification
Title: asyncore delayed calls feature
Type: enhancement Stage: patch review
Components: Library (Lib) Versions: Python 3.4
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: josiahcarlson Nosy List: akuchling, djarb, facundobatista, forest, giampaolo.rodola, gvanrossum, haypo, intgr, j1m, jafo, josiahcarlson, kevinwatters, mark.dickinson, markb, mcdonc, pitrou, python-dev, r.david.murray, stutzbach, terry.reedy, tseaver
Priority: normal Keywords: patch

Created on 2007-12-17 16:24 by giampaolo.rodola, last changed 2014-05-28 21:37 by haypo. This issue is now closed.

Files
File name Uploaded Description Edit
asyncore.patch giampaolo.rodola, 2008-09-14 19:36 New patch improving speed and adding new tests and documentation
asyncore.patch giampaolo.rodola, 2009-03-03 23:10 Adds "tasks" keyword arguments and tells close_all() to remove unfired delayed calls left behind
scheduler.patch josiahcarlson, 2009-03-31 21:08
Messages (57)
msg58695 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2007-12-17 16:24
Hi,
I post this message here in the hope someone using asyncore could review
this.
Since the thing I miss mostly in asyncore is a system for calling a
function after a certain amount of time, I spent the last 3 days trying
to implement this with the hopes that this could be included in asyncore
in the the future.
The logic consists in calling a certain function (the "scheduler") at
every loop to check if it is the proper time to call one or more
scheduled functions.
Such functions are scheduled by the new delayed_call class which is very
similar to the DelayedCall class defined in /twisted/internet/base.py I
drew on.
It provides a basic API which can be used for setting, resetting and
canceling the scheduled functions.
For better performance I used an heap queue structure. This way the
scheduler() only needs to check the scheduled functions due to expire
soonest.

The following code sample implements an idle-timeout capability using
the attached modified asyncore library.

--- code snippet ---
import asyncore, asynchat, socket

class foo(asynchat.async_chat):

   def __init__(self, conn=None):
       asynchat.async_chat.__init__(self, conn)
       self.set_terminator(None)
       self.create_socket(socket.AF_INET, socket.SOCK_STREAM)
       self.connect(('127.0.0.1', 21))
       self.scheduled_timeout = self.call_later(120, self.handle_timeout)

   def collect_incoming_data(self, data):
       self.scheduled_timeout.reset()
       # do something with the data...

   def handle_timeout(self):
       self.push("500 Connection timed out.\r\n")
       self.close_when_done()
       
   def close(self):
       if not self.scheduled_timeout.cancelled:
           self.scheduled_timeout.cancel()
       asyncore.dispatcher.close(self)

foo()
asyncore.loop()
--- /code snippet ---


Today I played a little more with it and I tried to add bandwidth
throttling capabilities to the base asynchat.py.
The code could be surely improved but it's just an example to show
another useful feature which wouldn't be possible to implement without
having a "call_later" function under the hood:


--- code snippet ---
class throttled_async_chat(asynchat.async_chat):
    # maximum number of bytes to transmit in a second (0 == no limit)
    read_limit = 100 * 1024
    write_limit = 100 * 1024

    # smaller the buffers, the less bursty and smoother the throughput
    ac_in_buffer_size = 2048
    ac_out_buffer_size  = 2048

    def __init__(self, conn=None):
        asynchat.async_chat.__init__(self, conn)
        self.read_this_second = 0
        self.written_this_second = 0
        self.r_timenext = 0
        self.w_timenext = 0
        self.r_sleep = False
        self.w_sleep = False
        self.delayed_r = None
        self.delayed_w = None

    def readable(self):
        return asynchat.async_chat.readable(self) and not self.r_sleep

    def writable(self):
        return asynchat.async_chat.writable(self) and not self.w_sleep

    def recv(self, buffer_size):
        chunk = asyncore.dispatcher.recv(self, buffer_size)
        if self.read_limit:
            self.read_this_second += len(chunk)
            self.throttle_read()
        return chunk

    def send(self, data):
        num_sent = asyncore.dispatcher.send(self, data)
        if self.write_limit:
            self.written_this_second += num_sent
            self.throttle_write()
        return num_sent

    def throttle_read(self):
        if self.read_this_second >= self.read_limit:
            self.read_this_second = 0
            now = time.time()
            sleepfor = self.r_timenext - now
            if sleepfor > 0:
                # we've passed bandwidth limits
                self.r_sleep = True
                def unthrottle():
                    self.r_sleep = False
                self.delayed_r = self.call_later((sleepfor * 2), unthrottle)
            self.r_timenext = now + 1

    def throttle_write(self):
        if self.written_this_second >= self.write_limit:
            self.written_this_second = 0
            now = time.time()
            sleepfor = self.w_timenext - now
            if sleepfor > 0:
                # we've passed bandwidth limits
                self.w_sleep = True
                def unthrottle():
                    self.w_sleep = False
                self.delayed_w = self.call_later((sleepfor * 2), unthrottle)
            self.w_timenext = now + 1

    def close(self):
        if self.delayed_r and not self.delayed_r.cancelled:
            self.delayed_r.cancel()
        if self.delayed_w and not self.delayed_w.cancelled:
            self.delayed_w.cancel()
        asyncore.dispatcher.close(self)
--- /code snippet ---


I don't know if there's a better way to implement this "call_later" feature.
Maybe someone experienced with Twisted could provide a better approach.
I would ask someone using asyncore to review this since, IMHO, it would
fill a very big gap.
msg58763 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-12-18 20:06
If you want attention, please post to python-dev if you didn't already.
 Or widen the audience to python-list if you want to.
msg62398 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2008-02-14 16:40
The issue #2006 (asyncore loop lacks timers and work tasks) was closed
as duplicate of this one... noting this just for reference.
msg64099 - (view) Author: Sean Reifschneider (jafo) * (Python committer) Date: 2008-03-19 21:05
Giampaolo: Can you pleaes bring this up on python-dev or the normal
python mailing list for further discussion on the issue?
msg64103 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2008-03-19 21:25
Sean, I already tried to raise two discussion attempts on both lists here:
http://groups.google.com/group/python-dev2/browse_thread/thread/ecbf4d38a868d4f/ec5c7dbd40664b7f?lnk=gst&q=asyncore+giampaolo
...and here:
http://groups.google.com/group/comp.lang.python/browse_thread/thread/603d6c05aa6965c0/af451aedadb75832?lnk=gst&q=delayed+call#af451aedadb75832
...but no one seems to be interested at this feature.

Moreover, before doing anything against asyncore and asynhat there are a
lot of long-time pending patches which should be committed first; see here:
http://groups.google.com/group/python-dev2/browse_thread/thread/eec1ddadefe09fd8/a38270231620870e?lnk=gst&q=asyncore
msg64115 - (view) Author: Daniel Arbuckle (djarb) * Date: 2008-03-19 22:36
Unfortunately, it appears that asyncore and asynchat are caught in a
deadlock, in which it is demanded that certain patches be applied before
any further work is done, but nobody (even among those making the
demands) is both willing and able to review and apply those patches.

We need this situation to be resolved, preferably by somebody with
commit access doing the necessary work, but failing that by allowing new
patches and requiring the old ones to be updated at whatever time
somebody decides to actually address them.
msg69206 - (view) Author: Josiah Carlson (josiahcarlson) * Date: 2008-07-03 16:44
Generally speaking, delayed calls, and/or a practical scheduling
algorithm are useful for async servers.  Since 2.6 and 3.0 are on
feature freeze right now, this is going to have to wait for 2.7 and 3.1
.  I'll make sure to get something like this into 2.7 / 3.1 .
msg73232 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2008-09-14 19:36
I try to revamp this issue by attaching a new patch which improves the
work I did against asyncore last time.
The approach proposed in this new patch is the same used in the upcoming
pyftpdlib 0.5.0 version which has been largely tested and benchmarked.
In my opinion, without the addition of an eventual paired heap module
into the stdlib there are no significant faster ways to do this than
using the common heapq module.
The patch in attachment includes:

- various changes which improve the speed execution when operating
against the heap.
- a larger test suite.
- documentation for the new class and its methods.

Josiah, do you have some time to review this?
msg73416 - (view) Author: Josiah Carlson (josiahcarlson) * Date: 2008-09-19 02:24
I have an updated sched.py module which significantly improves the 
performance of the cancel() operation on scheduled events (amortized 
O(log(n)), as opposed to O(n) as it is currently).  This is sufficient 
to make sched.py into the equivalent of a pair heap.

From there, it's all a matter of desired API and features.

My opinion on the matter: it would be very nice to have the asyncore 
loop handle all of the scheduled events internally.  However, being able 
to schedule and reschedule events is a generally useful feature, and 
inserting the complete functionality into asyncore would unnecessarily 
hide the feature and make it less likely to be used by the Python 
community.

In asyncore, I believe that it would be sufficient to offer the ability 
to call a function within asyncore.loop() before the asyncore.poll() 
call, whose result (if it is a number greater than zero, but less than 
the normal timeout) is the timeout passed to asyncore.poll().  Obviously  
the function scheduler would be written with this asyncore API in mind.
msg83045 - (view) Author: Forest (forest) Date: 2009-03-02 23:00
I'm looking forward to having this functionality in asyncore.  It would
help me remove some unwanted hackery from my own code.

Giampaolo, I'm concerned that your patch uses a global 'tasks' list
which cannot be overriden.  Shouldn't loop() accept an optional task
list argument, as it already does with the socket map?  That would keep
with the spirit of asyncore and make things easier for those of us who
use multiple event loops in multiple threads.

Josiah, is your updated sched module the one described in this blog
post?  Is there an issue in the bug tracker about it?
http://chouyu-31.livejournal.com/316112.html
msg83080 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2009-03-03 18:17
> Giampaolo, I'm concerned that your patch uses a global 'tasks' list
> which cannot be overriden.  Shouldn't loop() accept an optional task 
> list argument, as it already does with the socket map?  That would keep
> with the spirit of asyncore and make things easier for those of us who
> use multiple event loops in multiple threads.

Personally I can't think of any use case in which that would come
helpful, but perhaps it's because I've never mixed asyncore and threads.
Can't you do that by simply overriding the global list?
msg83081 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2009-03-03 18:44
The idea is to be able (whether you see a use case or not) to use
different tasks lists simultaneously.  Messing with globals is the worst
possible API for that.  All you need is to add a tasks=None argument to
the loop() signature, rename the global tasks list to (e.g.)
default_tasks, and add this to the top of loop:

if tasks is None:
    tasks = default_tasks

similar to what it does for map.  You'd also have to pass the tasks list
to the scheduler() call and the call_later() constructor.  Defaulting to
a global is fine.
msg83082 - (view) Author: Josiah Carlson (josiahcarlson) * Date: 2009-03-03 19:10
Forest:
To answer your question, yes, that blog post discusses a better variant 
of sched.py , but no, there isn't a bug.  I should probably post it some 
time soon for 2.7/3.1 inclusion.
msg83093 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2009-03-03 20:10
> You'd also have to pass the tasks list to the scheduler() call and the
> call_later() constructor.  Defaulting to a global is fine.

Unless I change the current API I can't add a new optional arguments to
call_later constructor because it already uses *args **kwargs:

def __init__(self, seconds, target, *args, **kwargs):
msg83094 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2009-03-03 20:12
You could solve this with a "reserved" keyword argument _tasks.

Or you could have two different factory methods, call_later_with_tasks()
and call_later().
msg83102 - (view) Author: Josiah Carlson (josiahcarlson) * Date: 2009-03-03 21:26
I've just attached a patch to sched.py and asyncore.py to offer a richer 
set of features for sched.py, with a call_later() function and minimal 
related classes for asyncore.py to handle most reasonable use-cases.

There is no documentation or tests, but I can add those based on 
Giampaolo's tests and docs if we like this approach better.
msg83103 - (view) Author: Josiah Carlson (josiahcarlson) * Date: 2009-03-03 21:29
Here's a better patch without tabs.
msg83109 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2009-03-03 23:10
A new patch is in attachment. 
Changes from the previous one (Sep 2008):


- renamed "deafult_tasks" global list to "scheduled_tasks"

- loop(), scheduler() and close_all() have a new "tasks" keyword
argument defaulting to None

- close_all() other than iterating over all existing dispatcher
instances and closing them, also iterate over any unfired scheduled call
found in "tasks" list, cancel() it and finally clears the list.

- call_later constructor accepts a reserved _tasks argument

- call_later overrides __lt__ instead of __le__


Tests and documentation are also included.
msg84905 - (view) Author: Josiah Carlson (josiahcarlson) * Date: 2009-03-31 21:08
I fixed some bugs with my patch, merged in Giampaolo's tests and 
documentation, and altered the API to match Giampaolo's API almost 
completely.

This new version differs from Giampaolo's patch only in underlying 
implementation; this uses a modified sched.py, and doesn't have a 
standard "execute outstanding methods" function built into asyncore 
(asynchat.scheduled_tasks.run(time.time()) is sufficient).

The major difference is that the modifications to sched.py offer a fast 
cancel/reschedule operation, which Giampaolo's lacks.
msg84972 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2009-04-01 03:36
At the language summit last Thursday there was widespread disappointment
with the changes to asyncore.py in 2.6, which broke almost all code that
actually uses it.  Unfortunately, the documented API is lame, so
everybody depended on undocumented internals, and those were changed
without respect for established use.  I'd like to prevent more problems
like that.
msg85052 - (view) Author: Josiah Carlson (josiahcarlson) * Date: 2009-04-01 17:12
IIRC, there was a threat to remove asyncore because there were no 
maintainers, no one was fixing bugs, no one was improving it, and no one 
was really using it (I believe the claim was that people were just using 
Twisted).  The patches that were ultimately committed to 2.6 and 3.0 
were committed 3 months prior to 2.6 release, after having languished 
for over a year because no one would review them.  If people care about 
where asyncore/asynchat are going, then it is *their* responsibility to 
at least make an effort in paying attention at least once every few 
months or so.

The delayed calls feature discussed in the current bug doesn't alter the 
behavior of previously existing code, except there are additional checks 
for pending tasks to be executed.  If people never use the call_later() 
API, it is unlikely they will experience any difference in behavior.

If you are concerned about the sched module, I'd be happy to do a search 
for it's use to verify that people aren't relying on it's internal 
implementation, only the features of it's external API.
msg85055 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2009-04-01 17:28
I guess the Zope developers aren't that tuned in to core Python
developement.  They were sorely bitten.  I don't think you can claim
that users should be tuned in to python-dev just to assure their
favorite module isn't removed or broken.  It behooves you to request
their feedback now that you know there still are asyncore users, not to
hijack the module for your own purposes.

IIRC one option discussed at the summit was to restore asyncore to its
pre-2.5 state and to slowly end-of-life it, giving Zope and other users
plenty of time to start maintaining their own copy (which they've
half-done already with all the monkey-patching that goes on :-), and
create a new module with a better specified API that won't require users
to use undocumented internals.  Part of this (even if we don't actually
roll it back to the 2.5 version, which is controversial) would be not
adding new features.
msg85066 - (view) Author: Josiah Carlson (josiahcarlson) * Date: 2009-04-01 18:28
I'm happy to let them know proposed changes now that I know issues 
exist, but you have to admit that they were pretty under-the-radar until 
4-5 months *after* 2.6 was released.  If there is a mailing address that 
I can send proposed changes to asyncore so that they can have a say, I'd 
be happy to talk to them.

Generally, what you are saying is that I'm damned if I do, damned if I 
don't.  By taking ownership and attempting to fix and improve the module 
for 2.6 (because there were a bunch of outstanding issues), people are 
pissed (generally those who use Zope and/or medusa).  Despite this, 
other people have continued to use it, and have been pushing for new 
features; event scheduling being one of the major parts.

Pulling asyncore out of Python is silly.  Not improving the module 
because of fear of breakage is silly.  I'm happy to hear suggestions 
from the Zope crew, but I'm only going to put as much effort in 
communicating with them as they do me.
msg85070 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2009-04-01 18:44
Josiah, you need an attitude adjustment. The breakage of asyncore in 2.6
was real and is now harming adoption of 2.6 by those folks (who are by
nature not early adopters -- their customers are typical enterprise users).

Talk to Tres Seaver and Jim Fulton. They occasionally post to python-dev
but that doesn't mean they read all of it.
msg85098 - (view) Author: Josiah Carlson (josiahcarlson) * Date: 2009-04-01 21:27
Here's a question: How do we fix 2.6?

From what I've read, the only answer I've heard is "revert to 2.5 in 
2.6.2", which has the same issues as adding True/False in 2.2 .

I agree that Zope not working in 2.6 is a problem, I agree that the 
documentation for asyncore is lacking, I agree that I probably wasn't as 
vocal as I could have been prior to the changes, I agree that 3rd 
parties relying on internal implementation details not covered in the 
limited documentation is a problem, I agree that we need to figure 
something out for asyncore 2.7 and beyond, I agree that we need to 
figure something out for asyncore 2.6 issues related to Zope and Medusa, 
...

I'm happy to take the blame for changing asyncore internals in Python 
2.6 .  And I've not stated otherwise in any forum.  At the time I 
thought it was the right thing to do.  If I could change the code 
retroactively, I would probably do so.

But it seems to me that "fork asyncore", "pull asyncore out of the 
stdlib", and "revert to 2.5" are all variants of the cliche "throwing 
the baby out with the bathwater".  There are good bug fixes in 2.6, and 
depending on how much of the internals that Zope and/or medusa rely on, 
we might even be able to write a short wrapper/adapter to throw in to 
Zope and/or asyncore.

I'll contact Tres and Jim, and hopefully be able to come to some 
reasonable solution.
msg85101 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2009-04-01 22:01
Well arguably asyncore is unsalvageable due to the undocumented
internals issue, and we sure know a bit more about how to design a
*good* asynchronous API than we did when asyncore was created. (One
hint: don't make subclassing part of your API.)

The Zope folks at the meeting in all seriousness proposed reverting to
the 2.5 version of asyncore since "it is broken in 2.6".  Since I don't
use it myself I really have no idea if anyone is using the 2.6 version.
msg85109 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2009-04-01 23:06
I don't know what are the problems experienced by the Zope folks (is
there a place where this is discussed?) but I can guess that they're
having problems with asynchat rather than with asyncore, since the
latter hasn't changed too much between 2.5 and 2.6 except for low level
connection related bug fixes.

The greatest difference in the new asynchat is that the producer_fifo
attribute is no longer an asyncore.fifo() instance but a deque().

Python 2.5:
> self.producer_fifo = fifo()

Python 2.6
> self.producer_fifo = deque()


Although they're quite similar the old code relying on the fifo() API
can't obviously work anymore.
This could have been a bad choice and there are probably other changes
that might have caused the problem (one other change that comes to my
mind is the different readable() writable() implementation).

An alternative to completely reverting asynchat.py to the 2.5 version,
which is somewhat too drastic IMO, could be identifying what are the
changes that caused the incompatibility, and reverting those parts only
for 2.6.2, in a way that no one (2.5 and 2.6 users) is affected.

If there's a place where this is discussed I could contribute in some
way since I've been working on asynchat/asyncore for a long time now.
msg85115 - (view) Author: Josiah Carlson (josiahcarlson) * Date: 2009-04-01 23:12
To be wholly clear about the issues, it's not with asyncore, the core asynchronous 
library, it's with asynchat and the internal changes to that.  Any changes to asyncore 
were to fix corner cases and exceptions.  No API, internal or external was changed.

People who subclassed from asyncore should have no problems.  People who subclassed 
from asynchat may have problems.

If we want to revert selected changes to asynchat, that's fine with me.  AFAICT, there 
is only 1 substantial bugfix in asynchat (if your text terminator isn't discovered in 
the first ac_in_buffer_size bytes read since the last terminator, your connection will 
hang), which is easily pulled out.  Offering a compatibility mode is also relatively 
easy.

Six months ago you were 'eh' with what was going on with the asyncore libraries (see 
messages from early October).  Over a year ago everyone on python-dev cared so little 
about the libraries that it was preferred to give me commit access than for someone to 
review the code.  Now everyone seems willing and happy to remove the library because it 
is "unsalvageable".

Ultimately the change that broke Zope/medusa was replacing the use of asynchat.fifo 
with a deque, and getting rid of ac_out_buffer.  Those are *tiny* changes that we can 
change back, temporarily pull into Zope, and tweak Medusa to fix (I'd be happy to offer 
a patch to AMK to produce Medusa 0.5.5).

As for your "subclassing is bad" comment, Twisted, wxPython, SocketServer 
(SimpleXMLRPCServer, TCPServer, ...), sgmllib.SGMLParser, etc., all use subclassing as 
part of their APIs.
msg85119 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2009-04-01 23:22
Josiah, there's no need to get all defensive and passive-aggressive
about it. I'm just reporting about strong feelings that were brought up
at the language summit -- to my surprise too! Admitting somebody made a
mistake would be step one (and I'll gladly admit I wasn't aware of the
Zope issues at the time or I would've warned you).

I've asked Tres Seaver and Jim Fulton to comment on this issue, I really
can't help you more with the details of which module actually broke and
what to do about it. I'm just recommending you use your commit
privileges wisely.
msg85144 - (view) Author: Chris McDonough (mcdonc) Date: 2009-04-02 02:46
I am the developer of Supervisor (http://supervisord.org) which depends
on (and extends) Medusa 0.5.4, which itself depends on some
implementation details of pre-2.6 versions of asynchat (e.g.
"ac_out_buffer").

I need to make sure Supervisor works with Python 2.3, 2.4, and 2.5, as
well as Python 2.6.  To do so, I intend to just ship the Python 2.5
version of asyncore/asynchat along with Medusa 0.5.4 and Supervisor in
the next Supervisor release; straddling the asynchat stuff would just be
too hard here.

I don't know of any other consumers of Medusa other than Zope and
Supervisor, so maybe Medusa should just ship with its own version of
asyncore and asynchat forever from now on;  I'm certainly not going to
take the time to "fix" Medusa to forward port it to the 2.6 version of
asynchat.

I might argue that in retrospect the the current implementation of
asynchat might have been better named "asynchat2", as the changes made
to it seem to have broken of its major consumers.  But we can work
around it by just forking, I think.
msg85154 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2009-04-02 03:49
Looking back, I think Zope and Medusa should have adopted and evolved
their own copy of asynchat a long time ago...
msg85206 - (view) Author: Jim Fulton (j1m) * (Python committer) Date: 2009-04-02 15:03
> Looking back, I think Zope and Medusa should have adopted and evolved
> their own copy of asynchat a long time ago...

This statement is puzzling.  No big deal, but I'm curious why you say
this.
msg85207 - (view) Author: Jim Fulton (j1m) * (Python committer) Date: 2009-04-02 15:03
For the record, afaict, Zope wasn't broken by this. Supervisor isn't
part of Zope.
msg85216 - (view) Author: Tres Seaver (tseaver) * Date: 2009-04-02 16:05
Sidnei da Silva had to put some "straddling" code in the Zope2 trunk to
workaround the 2.6 changes to asyncore / asynchat:

- http://svn.zope.org/Zope/?rev=91981&view=rev

- http://svn.zope.org/Zope/?rev=92023&view=rev
msg85226 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2009-04-02 17:26
[Guido]
>> Looking back, I think Zope and Medusa should have adopted and evolved
>> their own copy of asynchat a long time ago...

[Jim]
> This statement is puzzling.  No big deal, but I'm curious why you say
> this.

ISTR that Zope has or had significant monkeypatches to at least one of
asyncore/asynchat. The resulting coupling between Zope and asyn* has
meant that the de-facto API of asyn* was much more than the documented
API. IMO that's a sign of a poorly designed API (in asyn*). If Zope
had had its own copy of asyn* (under a different name of course) that
relied only on lower-level APIs (sockets and select), it could have
evolved that copy directly without the need for monkeypatching.
msg85228 - (view) Author: Jim Fulton (j1m) * (Python committer) Date: 2009-04-02 17:40
On Apr 2, 2009, at 1:27 PM, Guido van Rossum wrote:

>
> Guido van Rossum <guido@python.org> added the comment:
>
> [Guido]
>>> Looking back, I think Zope and Medusa should have adopted and  
>>> evolved
>>> their own copy of asynchat a long time ago...
>
> [Jim]
>> This statement is puzzling.  No big deal, but I'm curious why you say
>> this.
>
> ISTR that Zope has or had significant monkeypatches to at least one of
> asyncore/asynchat.

Not that I'm aware of.  I did add the ability to pass in alternative  
map objects, which is the only change we needed that I'm aware of. I  
think I made that change before or soon after asyncore was added to  
the standard library.

> The resulting coupling between Zope and asyn* has
> meant that the de-facto API of asyn* was much more than the documented
> API.

If we were monkey patching it, it would be at our own risk, which is  
why we'd copy the module if we needed to.  That has its own problems  
of course. I rue the day I forked doctest. :(

> IMO that's a sign of a poorly designed API (in asyn*). If Zope
> had had its own copy of asyn* (under a different name of course) that
> relied only on lower-level APIs (sockets and select), it could have
> evolved that copy directly without the need for monkeypatching.

I've read a good argument that subclassing across implementation  
packages is a bad idea. If a framework offers features through  
subclassing, it should define the subclassing interface very  
carefully, which asyncore doesn't.

Jim
msg85233 - (view) Author: Tres Seaver (tseaver) * Date: 2009-04-02 18:24
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Guido van Rossum wrote:

> ISTR that Zope has or had significant monkeypatches to at least one of
> asyncore/asynchat. The resulting coupling between Zope and asyn* has
> meant that the de-facto API of asyn* was much more than the documented
> API. IMO that's a sign of a poorly designed API (in asyn*). If Zope
> had had its own copy of asyn* (under a different name of course) that
> relied only on lower-level APIs (sockets and select), it could have
> evolved that copy directly without the need for monkeypatching.

Zope does not monkeypatch asyncore or asynchat, and hasn't since at
least Zope 2.5 (the oldest checkout I have, first released 2002-01-25).

Tres.
- --
===================================================================
Tres Seaver          +1 540-429-0999         tseaver@agendaless.com
Agendaless Consulting                         http://agendaless.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFJ1QLqFXKVXuSL+CMRAhelAJ9yYgo1RXUhWR2cH8CjYRoXz/qsvACgg13O
BFAiRoYP8AWVgQVWBhVhB+4=
=wj2y
-----END PGP SIGNATURE-----
msg85251 - (view) Author: Josiah Carlson (josiahcarlson) * Date: 2009-04-02 19:35
I'm not defending the documentation, I'm merely reposting it.

The documentation for asyncore says, "The full set of methods that can 
be overridden in your subclass follows:"

The documentation for asynchat says, "To make practical use of the code 
you must subclass async_chat, providing meaningful 
collect_incoming_data() and found_terminator() methods. The 
asyncore.dispatcher methods can be used, although not all make sense in 
a message/response context."

How can we make the documentation better?  I'm too close to the 
documentation to really know how to improve it.  Ideas?
msg85258 - (view) Author: Jim Fulton (j1m) * (Python committer) Date: 2009-04-02 19:57
On Apr 2, 2009, at 3:35 PM, Josiah Carlson wrote:

>
> Josiah Carlson <josiahcarlson@users.sourceforge.net> added the  
> comment:
>
> I'm not defending the documentation, I'm merely reposting it.
>
> The documentation for asyncore says, "The full set of methods that can
> be overridden in your subclass follows:"
>
> The documentation for asynchat says, "To make practical use of the  
> code
> you must subclass async_chat, providing meaningful
> collect_incoming_data() and found_terminator() methods. The
> asyncore.dispatcher methods can be used, although not all make sense  
> in
> a message/response context."
>
> How can we make the documentation better?  I'm too close to the
> documentation to really know how to improve it.  Ideas?

Actually, the documentation is better than I remember it to be. The  
problem is that subclassing is a much more intimate interface between  
components that a call interface.  In the case of asyncore, the  
methods being overridden have non-trivial default implementations.  
Overriding methods often entails studying the base-class code to get  
an idea how it should be done. The subclassing interface for asynchat  
appears to be much cleaner, but even then, you need to study the base  
class code to make sure you haven't accidentally overridden any base  
class attributes.  I wish classes that exposed subclassing interfaces  
were more careful with their internal names.

Jim
msg104595 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-04-29 22:29
Assuming this is still desirable I'd really like to move forward with this issue.
The current situation is that we have two patches.

My patch
========

pros: 
 * affects asyncore.py only
 * (imho) cleaner, as it just adds one class
 * stable, as it has been used in pyftpdlib for over 3 years now

cons:
 * significantly slower compared to Josiah's "paired-heap" approach 


Josiah's patch
==============

pros:
 * significantly faster

cons:
 * affects asyncore.py and sched.py
 * sched.py is modified quite heavily, also it's not clear whether that has been done in a fully retro-compatible way or not, so a full review from someone who has experience with this module would be needed
 * it seems that sched.py gains brand new functionnalities which are not necessarily related with asyncore, hence tests and documentation should be added. Furthermore, belonging them to sched.py, maybe they should be treated in a separate issue


Both patches should no longer apply cleanly so they should be adjusted a little and the missing parts (full tests, documentation including example usage, etc...) completed.
It seems we both agree on the API, which is both simple and has the extra advantage of being the same as Twisted's.
Now it's only a matter of deciding what to do about the internal implementation.
msg104633 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-04-30 13:20
I agree with the points raised against Josiah's patch. I'm not sure O(n) cancellation is really a concern. The main focus of optimization should be the scheduler's loop itself, and both approaches have an O(log n) complexity there AFAICT. Also, the cancellation optimization could be backported into Giampaolo's patch.

One area tests should check for is when scheduling operations are done from a delayed call. Especially, a delayed call rescheduling itself.

By the way, it's too late for 2.7, so this is only for 3.2 now.
msg104640 - (view) Author: Daniel Stutzbach (stutzbach) (Python committer) Date: 2010-04-30 14:49
I like the idea of leveraging the sched module.  It encapsulates the priority queue, allowing the user to be agnostic to the underlying data structure.  If someday we have a data structure in the collections module that provides an efficient delete-key operation, we can switch.  Giampaolo's patch forever ties us to heapq.

That said, I believe Josiah's patch could be simplified considerably.  Here are two ideas, which can be evaluated separately:

- The performance improvements to sched should be part of a separate patch and listed under a separate issue in the tracker.

- Let the user leverage the existing scheduler API.  Cut out scheduled_task and call_later, which just wraps the scheduler API.  The user can simply call scheduled_tasks.enter() or scheduled_tasks.cancel().  It's one less API for them to learn and one less for us to maintain.

Also, fix one small bug:

- Add a function to create a sched.scheduler().  Several functions take an optional "tasks" parameter, but there's no way to allocate a scheduler without peeking at the implementation and duplicating how it allocates the global one.
msg105484 - (view) Author: Josiah Carlson (josiahcarlson) * Date: 2010-05-11 04:32
Some prodding from Giampaolo got me to pull out and simplify the sched.py changes here: issue8684 .

That should be sufficient to add scheduling behavior into async socket servers or otherwise.
msg105526 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-05-11 16:55
> Let the user leverage the existing scheduler API.  Cut out 
> scheduled_task and call_later, which just wraps the scheduler API.  
> The user can simply call scheduled_tasks.enter() or 
> scheduled_tasks.cancel().  It's one less API for them to learn and 
> one less for us to maintain.

I think a wrapper around sched.py is necessary.
Now that I wrote tests for it I realized its API is pretty rusty and old.


Adding a call:

scheduler = sched.scheduler(time.time, time.sleep)
scheduler.enter(10, 1, function, (arg,))

...vs:

asyncore.call_later(10, function, arg)


Cancelling a call:

scheduler = sched.scheduler(time.time, time.sleep)
event = scheduler.enter(10, 1, function, (arg,))
scheduler.cancel(event)

...vs:

event = asyncore.call_later(10, function, arg)
event.cancel()


Moreover, reset() and delay() methods are not implemented in sched.
By using call_later you can do:

event = asyncore.call_later(10, function, arg)
event.reset()
event.delay(10)

By using sched.py you'll have to recreate a new event from scratch (scheduler.cancel(event) -> calculate the new timeout, scheduler.enter(newtime, 1, function, (arg,)).

Other problems which comes to mind are: you can't easily know whether a call has already been cancelled, you can't manually fire it before the timeout has expired and I'm not even sure whether it's possible to pass kwargs to enter(), which is crucial (with call_later you can do it like this: asyncore.call_later(10, function, x, y, z='foo')).
msg105527 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-05-11 17:08
> Adding a call:
> 
> scheduler = sched.scheduler(time.time, time.sleep)
> scheduler.enter(10, 1, function, (arg,))
> 
> ...vs:
> 
> asyncore.call_later(10, function, arg)

I don't really see the difference. How hard it is to build a scheduler
object at startup and store it somewhere in your globals or on one of
your objects?

The main improvement I could see would be to make the arguments to
sched.scheduler() optional, and default to time.time and time.sleep.
msg105530 - (view) Author: Daniel Stutzbach (stutzbach) (Python committer) Date: 2010-05-11 17:56
On Tue, May 11, 2010 at 11:55 AM, Giampaolo Rodola'
<report@bugs.python.org> wrote:
> Moreover, reset() and delay() methods are not implemented in sched.
>
> Other problems which comes to mind are: you can't easily know whether a call has already been cancelled, you can't manually fire it before the timeout has expired and I'm not even sure whether it's possible to pass kwargs to enter(), which is crucial (with call_later you can do it like this: asyncore.call_later(10, function, x, y, z='foo')).

These are nice features, but wouldn't it make more sense to add them to sched?

That would provide them to other users of sched, while keeping the
asyncore code simpler.
msg141125 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2011-07-25 21:48
This patch is now available as a recipe for python 2.x:
http://code.activestate.com/recipes/577808-asyncore-scheduler/
msg149449 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2011-12-14 13:44
With issue13449 fixed I think we can now provide this functionnality by adding a specific section into asyncore doc which explains how to use asyncore in conjunction with sched module.
As such, asyncore.py itself won't need any change.
msg149451 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-12-14 13:58
> With issue13449 fixed I think we can now provide this functionnality by 
> adding a specific section into asyncore doc which explains how to use
> asyncore in conjunction with sched module.

How would it work?
msg149452 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2011-12-14 14:24
Now that I think of it maybe some kind of wrapper would still be necessary.
As of right now, we'd do something like this.
At the core we would have:

import asyncore, asynchat, sched

# global
scheduler = sched.scheduler()

while 1:
    asyncore.loop(timeout=1.0, count=1)  # count=1 makes loop() return after 1 loop
    scheduler.run(blocking=False)       
    

Then, every dispatcher can define a scheduled function of its own:


class Client(asynchat.async_chat):
    # an already connected client 
    # (the "connector" code is not included in this example)

    def __init__(self, *args, **kwargs):
        asynchat.async_chat.__init__(self, *args, **kwargs)
        self.set_terminator("\r\n")
        self.set_timeout()
        
    def set_timeout(self):
        self.timeout = scheduler.enter(30, 0, self.handle_timeout)
        
    def reset_timeout(self):
        scheduler.cancel(self.timeout)
        self.set_timeout()
    
    def found_terminator(self):
        scheduler.cancel(self.timeout)
        self.timeout = scheduler.enter(30, 0, self.handle_timeout)
        # do something with the received data...
        
    def handle_timeout(self):
        self.push("400 connection timed out\r\n")
        self.close()
        
    def close(self):
        scheduler.cancel(self.timeout)
        asynchat.async_chat.close(self)
msg149453 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-12-14 14:33
> while 1:
>     asyncore.loop(timeout=1.0, count=1)  # count=1 makes loop() return after 1 loop
>     scheduler.run(blocking=False)       

Isn't that both ugly and imprecise?
The right way to do it is to set the timeout of the select() call
according to the deadline of the next scheduled call in the scheduler.
But you probably need to modify asyncore for that.
msg155879 - (view) Author: Roundup Robot (python-dev) Date: 2012-03-15 12:05
New changeset 59f0e6de54b3 by Giampaolo Rodola' in branch 'default':
(sched) when run() is invoked with blocking=False return the deadline of the next scheduled call in the scheduler; this use case was suggested in http://bugs.python.org/issue1641#msg149453
http://hg.python.org/cpython/rev/59f0e6de54b3
msg183762 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-03-08 20:03
Where does this issue stand now?  Did the applied sched patch supersede the proposed asyncore patch? Is enhancing asyncore still on the table given Guido's proposed new module?
msg183763 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2013-03-08 20:08
A new implementation is part of Tulip (tulip/selectors.py); once Tulip
is further along it will be a candidate for inclusion in the stdlib
(as socket.py) regardless of whether tulip itself will be accepted. I
have no plans to work on asyncore.

On Fri, Mar 8, 2013 at 12:03 PM, Terry J. Reedy <report@bugs.python.org> wrote:
>
> Terry J. Reedy added the comment:
>
> Where does this issue stand now?  Did the applied sched patch supersede the proposed asyncore patch? Is enhancing asyncore still on the table given Guido's proposed new module?
>
> ----------
> nosy: +terry.reedy
> versions: +Python 3.4 -Python 3.3
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue1641>
> _______________________________________
msg183764 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2013-03-08 20:46
I'm not sure how many users asyncore has out there nowadays, but if it has to stay in the stdlib then I see some value in adding a scheduler to it because it is an essential component.

If this is still desirable I can restart working on a patch, although I'll have to go through some of the messages posted earlier in this topic and figure how's best to proceed: whether reusing sched.py or write a separate scheduler in asyncore.py.
msg200175 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2013-10-17 23:14
Now asyncio/tulip has landed in the 3.4 stdlib, asyncore will be effectively obsolete starting 3.4 (even if we don't mark it so). Its presence is required for backwards compatibility, but that doesn't mean we should encourage people to keep using it by adding features.

If you agree, please close this issue.
msg219303 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2014-05-28 21:37
asyncore documentation now starts with this note (which was approved by the asyncore maintainer):
"This module exists for backwards compatibility only. For new code we recommend using asyncio."

Since asyncio is now part of the stdlib, I don't think that it's worth to enhance asyncore. asyncore has design flaws like its poll() function which doesn't scale well with the number of file descriptors.

The latest patch for this issue was written 5 years ago, I don't think that many people are waiting for this feature in asyncore. Delayed calls are part of asyncio core, it's well designed and *efficient*.

So I'm now closing this issue. "Upgrade" your code to asyncio!
History
Date User Action Args
2014-05-28 21:37:06hayposetstatus: open -> closed

nosy: + haypo
messages: + msg219303

resolution: wont fix
2013-10-17 23:14:42gvanrossumsetmessages: + msg200175
2013-03-08 20:46:22giampaolo.rodolasetmessages: + msg183764
2013-03-08 20:08:10gvanrossumsetmessages: + msg183763
2013-03-08 20:03:42terry.reedysetnosy: + terry.reedy

messages: + msg183762
versions: + Python 3.4, - Python 3.3
2012-03-15 12:05:47python-devsetnosy: + python-dev
messages: + msg155879
2011-12-14 14:33:27pitrousetmessages: + msg149453
2011-12-14 14:24:37giampaolo.rodolasetmessages: + msg149452
2011-12-14 13:58:27pitrousetmessages: + msg149451
2011-12-14 13:44:08giampaolo.rodolasetmessages: + msg149449
2011-07-25 21:48:53giampaolo.rodolasetmessages: + msg141125
versions: + Python 3.3, - Python 3.2
2011-01-07 14:47:30mark.dickinsonsetnosy: + mark.dickinson
2010-05-11 17:56:25stutzbachsetmessages: + msg105530
2010-05-11 17:08:52pitrousetmessages: + msg105527
2010-05-11 16:55:33giampaolo.rodolasetmessages: + msg105526
2010-05-11 04:32:09josiahcarlsonsetmessages: + msg105484
2010-04-30 14:49:45stutzbachsetmessages: + msg104640
2010-04-30 13:20:30pitrousetstage: patch review
messages: + msg104633
versions: + Python 3.2, - Python 3.1, Python 2.7
2010-04-29 22:29:35giampaolo.rodolasetnosy: + pitrou, r.david.murray
messages: + msg104595
2009-04-02 19:57:38j1msetmessages: + msg85258
2009-04-02 19:35:08josiahcarlsonsetmessages: + msg85251
2009-04-02 18:24:54tseaversetmessages: + msg85233
2009-04-02 17:40:11j1msetmessages: + msg85228
2009-04-02 17:26:59gvanrossumsetmessages: + msg85226
2009-04-02 16:05:26tseaversetnosy: + tseaver
messages: + msg85216
2009-04-02 15:03:50j1msetmessages: + msg85207
2009-04-02 15:03:05j1msetnosy: + j1m
messages: + msg85206
2009-04-02 05:36:39stutzbachsetnosy: + stutzbach
2009-04-02 03:49:32gvanrossumsetmessages: + msg85154
2009-04-02 02:46:40mcdoncsetnosy: + mcdonc
messages: + msg85144
2009-04-01 23:22:55gvanrossumsetmessages: + msg85119
2009-04-01 23:12:44josiahcarlsonsetmessages: + msg85115
2009-04-01 23:06:33giampaolo.rodolasetmessages: + msg85109
2009-04-01 22:01:30gvanrossumsetmessages: + msg85101
2009-04-01 21:27:27josiahcarlsonsetmessages: + msg85098
2009-04-01 18:44:59gvanrossumsetmessages: + msg85070
2009-04-01 18:28:55josiahcarlsonsetmessages: + msg85066
2009-04-01 17:28:13gvanrossumsetmessages: + msg85055
2009-04-01 17:13:00josiahcarlsonsetmessages: + msg85052
2009-04-01 03:36:41gvanrossumsetmessages: + msg84972
2009-03-31 21:08:44josiahcarlsonsetfiles: - scheduler_partial.patch
2009-03-31 21:08:17josiahcarlsonsetfiles: + scheduler.patch

messages: + msg84905
2009-03-25 16:46:02kevinwatterssetnosy: + kevinwatters
2009-03-25 06:29:04intgrsetnosy: gvanrossum, akuchling, facundobatista, jafo, josiahcarlson, forest, giampaolo.rodola, djarb, markb, intgr
2009-03-25 06:04:06intgrsetnosy: + intgr
2009-03-03 23:16:28giampaolo.rodolasetfiles: - patch.diff
2009-03-03 23:10:51giampaolo.rodolasetfiles: + asyncore.patch
messages: + msg83109
2009-03-03 21:29:16josiahcarlsonsetfiles: + scheduler_partial.patch
messages: + msg83103
2009-03-03 21:28:30josiahcarlsonsetfiles: - scheduler_partial.patch
2009-03-03 21:26:16josiahcarlsonsetfiles: + scheduler_partial.patch
messages: + msg83102
2009-03-03 20:12:57gvanrossumsetmessages: + msg83094
2009-03-03 20:10:23giampaolo.rodolasetmessages: + msg83093
2009-03-03 19:10:20josiahcarlsonsetmessages: + msg83082
2009-03-03 18:44:26gvanrossumsetmessages: + msg83081
2009-03-03 18:17:35giampaolo.rodolasetmessages: + msg83080
2009-03-02 23:00:49forestsetmessages: + msg83045
2008-09-19 02:24:03josiahcarlsonsetmessages: + msg73416
2008-09-14 19:38:54giampaolo.rodolasetcomponents: + Library (Lib), - Installation
versions: + Python 3.1, Python 2.7, - Python 2.6
2008-09-14 19:37:10giampaolo.rodolasetfiles: - asyncore.py
2008-09-14 19:36:53giampaolo.rodolasetfiles: + asyncore.patch
messages: + msg73232
components: + Installation, - Library (Lib)
2008-07-03 16:44:17josiahcarlsonsetassignee: akuchling -> josiahcarlson
messages: + msg69206
2008-05-20 13:15:47markbsetnosy: + markb
2008-03-20 18:38:19forestsetnosy: + forest
2008-03-19 22:36:44djarbsetmessages: + msg64115
2008-03-19 21:25:28giampaolo.rodolasetmessages: + msg64103
2008-03-19 21:05:38jafosetnosy: + akuchling, jafo
messages: + msg64099
priority: normal
assignee: akuchling
keywords: + patch
type: enhancement
2008-02-14 16:40:03facundobatistasetnosy: + facundobatista
messages: + msg62398
2007-12-18 20:06:39gvanrossumsetnosy: + gvanrossum
messages: + msg58763
2007-12-18 13:46:03djarbsetnosy: + djarb
2007-12-17 16:25:57giampaolo.rodolasetfiles: + asyncore.py
2007-12-17 16:25:23giampaolo.rodolasetfiles: + patch.diff
2007-12-17 16:24:53giampaolo.rodolacreate