Ticket #1875 (closed enhancement: fixed )

Opened 3 years ago

Last modified 1 year ago

twisted.internet.task.deferLater()

Reported by: mithrandi Assigned to: exarkun
Type: enhancement Priority: highest
Milestone: Component: core
Keywords: Cc: jknight, exarkun, radix, jerub, mithrandi, strank, therve, yaubi
Branch: branches/task-deferlater-1875-2 Author: exarkun
Launchpad Bug:

Description

See attached patch.

Attachments

deferlater.diff (3.2 kB) - added by mithrandi 3 years ago.

Change History

  2006-06-28 23:46:11+00:00 changed by mithrandi

  • summary changed from {{{twisted.internet.task.deferLater()}}} to twisted.internet.task.deferLater()

  2006-06-28 23:59:38+00:00 changed by mithrandi

  • owner changed from exarkun to jerub

Attaching updated patch.

  2006-06-29 00:00:03+00:00 changed by mithrandi

  • attachment deferlater.diff added

  2006-07-20 22:21:44+00:00 changed by jerub

  • keywords deleted
  • owner changed from jerub to mithrandi

Okay, I've had a good look at the patch, and I'm satisfied that it does what it says it does. I have two concerns.

One: I want to run this past glyph or exarkun, just to make sure that this code is worth going into Twisted. It looks very similar to reactor.callLater, I feel there should be one obvious way of doing things. Should we deprecate callLater and point to deferLater? Or should we put guidelines in here for where deferLater should be used instead of callLater.

Two: def _callLater(self, *a, **kw): should probably be written: def __callLater(self, delay, func):. The function is private, and is only invoked like this: self._callLater(delay, lambda: d.callback(None)).

Please update the patch with the above modifications, and I'll take responsibility for merging the updated patch if/when I get approval/guidance.

  2006-07-20 22:28:18+00:00 changed by jerub

The double underscore in front of __callLater in my comment is a typo. It should be a single underscore.

  2006-07-20 23:33:31+00:00 changed by jerub

Branch created: task-deferlater-1875

I've altered the twisted.web2.test.test_http tests to import deferLater, and use that instead. This has managed to reduce the amount of code required, which is highly satisfactory.

More tests for deferLater would be good - we don't currently have a test for if the function raises an error and fires an errback.

  2006-07-21 02:30:23+00:00 changed by jknight

I'm confused, why does it do an addCallback for you? That seems unnecessary and redundant. I'd suggest using the function as it was in test_http. Simple, clean, no extraneous gunk.

def deferLater(secs):

d = defer.Deferred() reactor.callLater(secs, d.callback, None) return d

  2006-08-21 01:44:39+00:00 changed by radix

What James said.

  2006-08-21 01:48:53+00:00 changed by exarkun

Why is

deferLater(n).addCallback(lambda ign: f(x))

preferable to

deferLater(n, f, x)

?

  2006-08-26 16:06:19+00:00 changed by exarkun

  • cc set to jknight, exarkun, radix, jerub, mithrandi

How about a compromise? The function can default to lambda: None, then both of the above will work.

  2006-08-26 16:31:53+00:00 changed by jknight

It's preferable for simplicity of API. The deferred is the primary concept here, and it has a perfectly good API already which there's no good reason to hide.

  2006-09-09 13:15:23+00:00 changed by exarkun

It's not perfectly good. The feature it is missing in this case is invoking a callback without passing any arguments. This is what is addressed by allowing a function and arguments to be passed directly to deferLater.

  2006-09-09 23:51:52+00:00 changed by jknight

Same thing is true for all deferred callback functions. This shouldn't be special.

  2006-09-10 04:44:14+00:00 changed by exarkun

But there's never any possibility of a useful value being passed in here. The fact that the call is being made is the entirety of the information being delivered.

  2007-03-08 22:59:35+00:00 changed by strank

  • cc changed from jknight, exarkun, radix, jerub, mithrandi to jknight, exarkun, radix, jerub, mithrandi, strank

On the mailing list today, I asked about a way to get the result of a task scheduled with callLater. deferLater nearly solves that problem.

As for the API, it would be easy to allow both variants: simply change the callable argument to default to None::

    def __call__(self, delay, callable_=None, *args, **kw):
        d = defer.Deferred()
        if callable_ is not None:
            d.addCallback(lambda dummy: callable(*args, **kw))
        self._callLater(delay, lambda: d.callback(None))
        return d

If there has to be only one way to call it, I think that deferLater(n, f, x) is much better, for the same reasons as exarkun mentioned.

What I am missing is a way to get to the DelayedCall instance returned by callLater. I would simply add a task attribute to the returned Deferred::

        ...
        d.task = self._callLater(delay, lambda: d.callback(None))
        return d

but I am not sure if this is nice enough.

  2007-03-24 12:06:28+00:00 changed by therve

  • cc changed from jknight, exarkun, radix, jerub, mithrandi, strank to jknight, exarkun, radix, jerub, mithrandi, strank, therve

Can we reach an agreement on this? We can probably have the both interfaces, one basic and another a little more high-level?

  2007-05-08 09:22:44+00:00 changed by therve

ping!

  2007-06-08 10:24:28+00:00 changed by yaubi

  • cc changed from jknight, exarkun, radix, jerub, mithrandi, strank, therve to jknight, exarkun, radix, jerub, mithrandi, strank, therve, yaubi

  2007-08-06 12:47:10+00:00 changed by mithrandi

  • owner deleted

  2007-08-06 12:51:02+00:00 changed by exarkun

Hmm, I'm thinking about the issue raised by strank now. What if you want to cancel the deferLater's DelayedCall??

  2007-08-06 14:28:40+00:00 changed by jknight

Hmm, I think I heard something about adding cancelable Deferreds at some point. Of course, that still doesn't let you reschedule/etc, but that seems okay to me: the more verbose API is still available for such special uses.

  2008-02-11 19:09:39+00:00 changed by jknight

  • branch deleted
  • author deleted

If it helps, I'll just state I don't care anymore which of the API alternatives is added, they're really both fine. Just, please don't add both or a hybrid, in some attempt to please me.

  2008-02-11 20:24:42+00:00 changed by exarkun

  • branch set to branches/task-deferlater-1875-2
  • author set to exarkun

(In [22507]) Branching to 'task-deferlater-1875-2'

  2008-02-11 21:26:52+00:00 changed by exarkun

  • keywords set to review

Okay

  2008-02-11 21:31:21+00:00 changed by exarkun

  • keywords deleted

Missed a thing...

  2008-02-11 21:37:58+00:00 changed by exarkun

  • keywords set to review

Missed thing was r22515.

  2008-02-12 09:48:42+00:00 changed by therve

  • keywords deleted
  • owner set to exarkun
  • deferLater doesn't document its clock argument
  • in writing-standard.xhtml, the example is wrong because it doesn't pass the reactor as argument

  2008-02-13 02:36:10+00:00 changed by exarkun

  • keywords set to review
  • owner deleted

Doh, I messed that up pretty good. :)

Fixed those things.

  2008-02-14 15:17:41+00:00 changed by therve

  • keywords deleted
  • owner set to exarkun

Please merge.

  2008-02-15 02:02:33+00:00 changed by exarkun

  • status changed from new to closed
  • resolution set to fixed

(In [22548]) Merge task-deferlater-1875-2

Author: mithrandi, jerub, exarkun Reviewer: therve Fixes #1875

Add twisted.internet.task.deferLater, an API for scheduling a future call and getting a Deferred which is called back with its result.

Note: See TracTickets for help on using tickets.