Skip to content

Add an async throwing source #1252

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 1, 2021
Merged

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Aug 27, 2021

Motivation:

AsyncThrowingStream provides an implementation of AsyncSequence
which allows the holder to provide new values to that sequence from
within a closure provided to the initializer. This API doesn't fit our
needs: we must be able to provide the values 'from the outside' rather
than during initialization.

Modifications:

  • Add an AsyncThrowingSource, an implementation of AsyncSequence
    which may have values provided to it via yield(_:) and terminated
    with finish() or finish(throwing:).
  • Add tests and a few AsyncSequence helpers.

Result:

We have an AsyncSequence implementation which can have values provided
to it.

@glbrntt glbrntt added the 🔨 semver/patch No public API change. label Aug 27, 2021
@glbrntt glbrntt requested a review from fabianfett August 27, 2021 09:10
@glbrntt
Copy link
Collaborator Author

glbrntt commented Aug 27, 2021

cc @simonjbeaumont

@@ -0,0 +1,59 @@
/*
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was copied from #1245

@glbrntt
Copy link
Collaborator Author

glbrntt commented Aug 27, 2021

cc @Davidde94

Comment on lines 49 to 56
/// Provide an element to the sequence.
///
/// - Parameter element: The element to provide to the sequence.
/// - Returns: A `YieldResult` indicating whether the sequence accepted the element or not.
@discardableResult
internal func yield(_ element: Element) -> YieldResult {
return self.storage.yield(element)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Who would use the yield method?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a ChannelHandler

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is called by a channel handler, this might become interesting with the generic element here.

fileprivate init(storage: AsyncThrowingSourceStorage<Element, Failure>) {
self.storage = storage
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are missing a deinit here, I'm afraid. If the consumer is not interested in your messages anymore, they might want to cancel the stream.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The best way to achieve this, is to back the iterator by a class that references the storage.

Comment on lines 90 to 92
internal func makeAsyncIterator() -> Iterator {
return Iterator(storage: self.storage)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens, if the consumer never calls makeAsyncIterator and the stream goes out of scope?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then any buffered elements would be dropped; but that's somewhat expected if they never call makeAsyncIterator? Feels like I'm missing your point here...

XCTAssertEqual(elements, 0)
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add tests:

  • user creates iterator but never consumes iterator
  • user never creates iterator

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate? Not sure I understand what we would actually be asserting in each case.

Motivation:

`AsyncThrowingStream` provides an implementation of `AsyncSequence`
which allows the holder to provide new values to that sequence from
within a closure provided to the initializer. This API doesn't fit our
needs: we must be able to provide the values 'from the outside' rather
than during initialization.

Modifications:

- Add an `AsyncThrowingSource`, an implementation of `AsyncSequence`
  which may have values provided to it via `yield(_:)` and terminated
  with `finish()` or `finish(throwing:)`.
- Add tests and a few `AsyncSequence` helpers.

Result:

We have an `AsyncSequence` implementation which can have values provided
to it.
Comment on lines 128 to 132
internal func consumeNextElement() async throws -> Element? {
return try await withCheckedThrowingContinuation {
self.consumeNextElement(continuation: $0)
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this should be @inlinable?

Comment on lines 42 to 44
internal func next() async throws -> Element? {
return try await self.storage.consumeNextElement()
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be @inlinable?

@glbrntt glbrntt merged commit a14c464 into grpc:async-await Sep 1, 2021
@glbrntt glbrntt deleted the gb-async-stream branch September 1, 2021 09:26
simonjbeaumont pushed a commit to simonjbeaumont/grpc-swift that referenced this pull request Sep 3, 2021
Motivation:

`AsyncThrowingStream` provides an implementation of `AsyncSequence`
which allows the holder to provide new values to that sequence from
within a closure provided to the initializer. This API doesn't fit our
needs: we must be able to provide the values 'from the outside' rather
than during initialization.

Modifications:

- Add a `PassthroughMessageSequence`, an implementation of 
  `AsyncSequence` which consumes messages from a
  `PassthroughMessagesSource`.
- The source  may have values provided to it via `yield(_:)` and terminated
  with `finish()` or `finish(throwing:)`.
- Add tests and a few `AsyncSequence` helpers.

Result:

We have an `AsyncSequence` implementation which can have values provided
to it.
glbrntt added a commit to glbrntt/grpc-swift that referenced this pull request Sep 16, 2021
Motivation:

`AsyncThrowingStream` provides an implementation of `AsyncSequence`
which allows the holder to provide new values to that sequence from
within a closure provided to the initializer. This API doesn't fit our
needs: we must be able to provide the values 'from the outside' rather
than during initialization.

Modifications:

- Add a `PassthroughMessageSequence`, an implementation of 
  `AsyncSequence` which consumes messages from a
  `PassthroughMessagesSource`.
- The source  may have values provided to it via `yield(_:)` and terminated
  with `finish()` or `finish(throwing:)`.
- Add tests and a few `AsyncSequence` helpers.

Result:

We have an `AsyncSequence` implementation which can have values provided
to it.
glbrntt added a commit to glbrntt/grpc-swift that referenced this pull request Sep 16, 2021
Motivation:

`AsyncThrowingStream` provides an implementation of `AsyncSequence`
which allows the holder to provide new values to that sequence from
within a closure provided to the initializer. This API doesn't fit our
needs: we must be able to provide the values 'from the outside' rather
than during initialization.

Modifications:

- Add a `PassthroughMessageSequence`, an implementation of 
  `AsyncSequence` which consumes messages from a
  `PassthroughMessagesSource`.
- The source  may have values provided to it via `yield(_:)` and terminated
  with `finish()` or `finish(throwing:)`.
- Add tests and a few `AsyncSequence` helpers.

Result:

We have an `AsyncSequence` implementation which can have values provided
to it.
glbrntt added a commit that referenced this pull request Sep 16, 2021
Motivation:

`AsyncThrowingStream` provides an implementation of `AsyncSequence`
which allows the holder to provide new values to that sequence from
within a closure provided to the initializer. This API doesn't fit our
needs: we must be able to provide the values 'from the outside' rather
than during initialization.

Modifications:

- Add a `PassthroughMessageSequence`, an implementation of 
  `AsyncSequence` which consumes messages from a
  `PassthroughMessagesSource`.
- The source  may have values provided to it via `yield(_:)` and terminated
  with `finish()` or `finish(throwing:)`.
- Add tests and a few `AsyncSequence` helpers.

Result:

We have an `AsyncSequence` implementation which can have values provided
to it.
glbrntt added a commit that referenced this pull request Nov 26, 2021
Motivation:

`AsyncThrowingStream` provides an implementation of `AsyncSequence`
which allows the holder to provide new values to that sequence from
within a closure provided to the initializer. This API doesn't fit our
needs: we must be able to provide the values 'from the outside' rather
than during initialization.

Modifications:

- Add a `PassthroughMessageSequence`, an implementation of 
  `AsyncSequence` which consumes messages from a
  `PassthroughMessagesSource`.
- The source  may have values provided to it via `yield(_:)` and terminated
  with `finish()` or `finish(throwing:)`.
- Add tests and a few `AsyncSequence` helpers.

Result:

We have an `AsyncSequence` implementation which can have values provided
to it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants