gatherResults should have an optional consumeErrors

[dustin]

It's been suggested a few times that those who need to use gatherResults, but want any failures in the underlying Deferreds to be handled exclusively by the DeferredList should write their own gatherResults method.

What if this method had an optional consumeErrors=False keyword arugment?

[dustin]

bug5159-r1.patch

[dustin]

Links for the "few times" in the description:

•  http://www.twistedmatrix.com/pipermail/twisted-python/2008-November/018639.html
• #4627

[exarkun]

This seems like a duplicate of #5061.

[dustin]

Not quite (this is a simpler solution to a simpler problem), but I'm sorry I didn't find that bug before filing this one.

[exarkun]

Okay. Thanks for the quick follow-up (sorry about mine being so delayed).

1. The gatherResults docstring change for consumeErrors seems a little round-about to me. Do you think it would be better if it just described the behavior of consumeErrors? The current form of trying to proscribe usage seems too narrow to me; it might serve as part of a good example, but not as the canonical documentation for the parameter (I might even say DeferredList's docstring should be canonical, and gatherResults could just refer to that, but I also want to get rid of DeferredList ;).
2. The test method name should follow the test_something convention, rather than the older testSomething convention.
3. The test method should have a docstring.
4. The test method would probably be better if it tested the error handling behavior rather than just checking the flag. Maybe we'll be able to get away with re-implementing gatherResults in terms of the replacement for DeferredList someday, and this attribute won't even exist anymore; the test could be indifferent to that change if it only checks behavior.
5. It would also be very nice if we had some howto-style documentation that covered gatherResults and this new flag for it. Do you feel like putting together a very quick addition for one of the Deferred howtos to demonstrate this API and the difference between consumeErrors=False and consumeErrors=True? Fortunately doc/core/howto/defer.xhtml does talk about DeferredList and its usage of consumeErrors already, so an extension to that section might not be too hard.

Thanks again! I don't think there should be any problem merging a patch that addresses these issues.

[dustin]

1: I'm not entirely sure what you mean by "trying to proscribe usage" - does the attached address your concerns?

2-4: done

5: I added a new subsection under DeferredList, treating gatherResults as a shortcut. The docs can change later if gatherResults grows into something that stands alone.

[dustin]

bug5159-r3.patch

[exarkun]

(In [32389]) Branching to 'gatherresults-consumeerrors-5159'

[exarkun]

(In [32390]) Apply dustin's patch

refs #5159

[exarkun]

(In [32392]) Address simple coding standard points

1. Docstring formatting
2. More description of the behavior being tested in the docstring
3. camelCase instead of under_scores in variable names
4. assertIsInstance instead of failUnless(check(...))

refs #5159

[exarkun]

Thanks. I made some changes, some linked above, others:

1. Changed the docs to say >= 11.1 instead of > 11.0
2. Referred back to DeferredList in the new gatherResults prose docs for the meaning of the consumeErrors parameter.
3. Made the new test not return a Deferred, since it completes synchronously
4. Changed the wording in the news fragment to follow the style guide.

The  build results look good so far. Assuming the last one completes successfully, I'll merge. Thanks again!

[exarkun]

(In [32405]) Merge gatherresults-consumeerrors-5159

Author: dustin Reviewer: exarkun Fixes: #5159

Add a consumeErrors parameter to twisted.internet.defer.gatherResults with the same meaning as the parameter of the same name accepted by DeferredList.