[GitHub] [arrow] westonpace commented on a change in pull request #12662: ARROW-15968: [C++] Update AsyncGenerator semantics to emit a terminal item only after all outstanding futures have completed

[GitBox <gi...@apache.org>]

westonpace commented on a change in pull request #12662:
URL: https://github.com/apache/arrow/pull/12662#discussion_r830312415



##########
File path: cpp/src/arrow/util/async_generator.h
##########
@@ -60,6 +60,9 @@ namespace arrow {
 // Readahead operators, and some other operators, may introduce queueing.  Any operators
 // that introduce buffering should detail the amount of buffering they introduce in their
 // MakeXYZ function comments.
+//
+// A generator should always be fully consumed before it is destroyed.
+// A generator should not emit a terminal item until it has finished all ongoing futures.

Review comment:
       Well I hope the generator can see the future because it returned it ;)
   
   I can try and fix up the warning.  I agree there is probably some ambiguity whether `emit` means "returning the future" or "completing a future" (this statement assumes it means the latter).
   
   Something like:
   
   ```
   A generator should ensure that all ongoing futures have completed before it completes a future with a terminal (error or end-of-stream) result.  All futures completed after that result should be the end-of-stream result.
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org