Opened 15 years ago

Closed 11 years ago

Last modified 11 years ago

#2674 defect closed fixed (fixed)

t.i.defer.inlineCallbacks documentation issues

Reported by: indigo Owned by: Jean-Paul Calderone
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch: branches/inlineCallbacks-docs-2674
branch-diff, diff-cov, branch-cov, buildbot
Author: indigo


inlineCallbacks currently allows one to yield a value that is not a Deferred instance without an error. When this is done, it seems to .send() the yielded value back to the generator immediately. I am not sure this is useful behavior, since if you can yield it, you could assign it. The only one case is maybe functions that might return a deferred sometimes; then "yield" serves a function similar to maybeDeferred.

I don't think that's a very common case. It seems more likely to me that yielding a non-deferred is a mistake and should raise an error. Also, it might be useful to have the capability to yield other things from the generator that instruct inlineCallbacks to do other things. For example, call next() in a thread, or maybe give it to runInteraction of an adbapi.ConnectionPool. This is not possible with the current behavior.

Arguments against the usefulness of the current behavior aside, it should at least be documented, if not depreciated or changed.

Also, I found it somewhat surprising that "returnValue(Deferred())" causes an error:

Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/twisted/internet/", line 746, in gotResult
    _inlineCallbacks(r, g, deferred)
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/twisted/internet/", line 733, in _inlineCallbacks
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/twisted/internet/", line 239, in callback
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/twisted/internet/", line 304, in _startRunCallbacks
--- <exception caught here> ---
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/twisted/internet/", line 317, in _runCallbacks
    self.result = callback(self.result, *args, **kw)
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/twisted/internet/", line 746, in gotResult
    _inlineCallbacks(r, g, deferred)
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/twisted/internet/", line 733, in _inlineCallbacks
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/twisted/internet/", line 238, in callback
    assert not isinstance(result, Deferred)

especially since .addCallback(lambda _: Deferred()) is not an error.

Attachments (1)

inlineCallbacks-docs-2674.patch (1.3 KB) - added by indigo 11 years ago.
Documentation additions.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 15 years ago by Glyph

Owner: changed from Glyph to indigo

"documentation patches welcome"

Changed 11 years ago by indigo

Documentation additions.

comment:2 Changed 11 years ago by indigo

Keywords: review added
Owner: indigo deleted

comment:3 Changed 11 years ago by Glyph

Author: glyph
Branch: branches/inlineCallbacks-docs-2674

(In [32720]) Branching to 'inlineCallbacks-docs-2674'

comment:5 Changed 11 years ago by Jean-Paul Calderone

Author: glyphindigo
Keywords: review removed
Owner: set to Jean-Paul Calderone

Thanks. This looks like a good documentation improvement. I'll leave discussion of whether the functionality should be extended for another ticket. :)

The doc builder is happy, so I'll apply this to trunk.

comment:6 Changed 11 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(In [32740]) Merge inlineCallbacks-docs-2674

Author: indigo Reviewer: exarkun Fixes: #2674

Expand the inlineCallbacks API documentation to explain the behavior of yielding a non-Deferred and that returnValue will not accept a Deferred.

comment:7 Changed 11 years ago by Glyph

Isn't it also a bug that this is an AssertionError and not something more informative / better documented at runtime?

Note: See TracTickets for help on using tickets.