Opened 10 years ago

Closed 6 years ago

Last modified 6 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

Description

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/defer.py", line 746, in gotResult
    _inlineCallbacks(r, g, deferred)
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/twisted/internet/defer.py", line 733, in _inlineCallbacks
    deferred.callback(e.value)
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/twisted/internet/defer.py", line 239, in callback
    self._startRunCallbacks(result)
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/twisted/internet/defer.py", line 304, in _startRunCallbacks
    self._runCallbacks()
--- <exception caught here> ---
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/twisted/internet/defer.py", 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/defer.py", line 746, in gotResult
    _inlineCallbacks(r, g, deferred)
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/twisted/internet/defer.py", line 733, in _inlineCallbacks
    deferred.callback(e.value)
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/twisted/internet/defer.py", line 238, in callback
    assert not isinstance(result, Deferred)
exceptions.AssertionError: 

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

Attachments (1)

inlineCallbacks-docs-2674.patch (1.3 KB) - added by indigo 6 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 6 years ago by indigo

Documentation additions.

comment:2 Changed 6 years ago by indigo

Keywords: review added
Owner: indigo deleted

comment:3 Changed 6 years ago by Glyph

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

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

comment:5 Changed 6 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 6 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 6 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.