Opened 10 years ago

Closed 5 years ago

Last modified 5 years ago

#2674 defect closed fixed (fixed)

t.i.defer.inlineCallbacks documentation issues

Reported by: indigo Owned by: exarkun
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 5 years ago.
Documentation additions.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 10 years ago by glyph

  • Owner changed from glyph to indigo

"documentation patches welcome"

Changed 5 years ago by indigo

Documentation additions.

comment:2 Changed 5 years ago by indigo

  • Keywords review added
  • Owner indigo deleted

comment:3 Changed 5 years ago by glyph

  • Author set to glyph
  • Branch set to branches/inlineCallbacks-docs-2674

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

comment:5 Changed 5 years ago by exarkun

  • Author changed from glyph to indigo
  • Keywords review removed
  • Owner set to exarkun

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 5 years ago by exarkun

  • Resolution set to fixed
  • Status changed from new to closed

(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 5 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.