SWIFT-719 Add a forEach method to async cursors and change streams #408
Conversation
| * Calls the provided closure with each event in the change stream as it arrives. | ||
| * | ||
| * - Returns: | ||
| * An `EventLoopFuture<Void>` which will succeed once the change stream is killed via `kill`. |
There was a problem hiding this comment.
this feels like a weird way to put this as at first read it seems unrelated to what this method does, but I'm not sure how else to say this, any ideas?
There was a problem hiding this comment.
The change stream can be closed via "invalidate" events too (docs). Might be better to just say "... which will resolve/complete once the change stream is closed."
| */ | ||
| public var isAlive: Bool { | ||
| return self.asyncChangeStream.isAlive | ||
| public func isAlive() throws -> Bool { |
There was a problem hiding this comment.
this throws now, but the only case I can think of that it would throw in is if you closed the client and then called this method.
which is actually true for basically all our methods but we don't document that anywhere else, so I wasn't sure if it was worth mentioning.
There was a problem hiding this comment.
What about just catching the error and returning false?
There was a problem hiding this comment.
very good point. updated both this and the cursor isAlive method to do that
patrickfreed
left a comment
patrickfreed
left a comment
There was a problem hiding this comment.
a couple comments on docs, the code looks good
| * Calls the provided closure with each event in the change stream as it arrives. | ||
| * | ||
| * - Returns: | ||
| * An `EventLoopFuture<Void>` which will succeed once the change stream is killed via `kill`. |
There was a problem hiding this comment.
The change stream can be closed via "invalidate" events too (docs). Might be better to just say "... which will resolve/complete once the change stream is closed."
| */ | ||
| public var isAlive: Bool { | ||
| return self.asyncChangeStream.isAlive | ||
| public func isAlive() throws -> Bool { |
There was a problem hiding this comment.
What about just catching the error and returning false?
kmahar
force-pushed
the
SWIFT-719/cursor-forEach
branch
from
3aafc01 to
f2d51fc
Compare
|
|
||
| /// The ID used by the server to track the cursor over time. If all of the cursor's results were returnable in a | ||
| /// batch, or if the cursor contained no results, this value will be nil. | ||
| public let id: Int64? |
There was a problem hiding this comment.
as we discussed yesterday, this is now retrieved initially and stored rather than computing it
There was a problem hiding this comment.
One little nit: maybe say "a single batch", or "the first batch"? Seems just a tad clearer to me.
There was a problem hiding this comment.
woops, I meant to say that, this phrasing is definitely weird. fixed
patrickfreed
left a comment
patrickfreed
left a comment
There was a problem hiding this comment.
one tiny nit otherwise lgtm!
|
|
||
| /// The ID used by the server to track the cursor over time. If all of the cursor's results were returnable in a | ||
| /// batch, or if the cursor contained no results, this value will be nil. | ||
| public let id: Int64? |
There was a problem hiding this comment.
One little nit: maybe say "a single batch", or "the first batch"? Seems just a tad clearer to me.
Adds
forEachto both types and does some internal refactoring, will explain in inline comments