Add an AsyncWriter #519
Add an AsyncWriter #519
Conversation
|
|
||
| let errorAction = self.lock.withLock { () -> ErrorAction in | ||
| switch self._state { | ||
| case .buffering: |
There was a problem hiding this comment.
Is it intended that we drop all buffered elements if we fail self? If yes we should document that behaviour. Otherwise we could use Result<ByteBuffer?, Error> and enqueue the error properly.
There was a problem hiding this comment.
We should add a precondition that checks that the demand continuation is nil, otherwise we will never resume it.
| } | ||
| } | ||
|
|
||
| public func demand() async throws { |
There was a problem hiding this comment.
What is this method used for? Does it wait until someone calls next but the buffer is empty?
| } | ||
|
|
||
| private enum State { | ||
| case buffering(CircularBuffer<ByteBuffer?>, CheckedContinuation<Void, Error>?) |
There was a problem hiding this comment.
Do we need to know that this is a ByteBuffer? Otherwise I would prefer it to be generic to reduce the accessible API surface inside the implementation of AsyncSequenceWriter.
Co-authored-by: George Barnett <gbarnett@apple.com>
fabianfett
force-pushed
the
ff-add-async-writer
branch
from
d094a67 to
56f72d6
Compare
| @available(macOS 12.0, iOS 15.0, watchOS 8.0, tvOS 15.0, *) | ||
| class AsyncSequenceWriter<E>: AsyncSequence { | ||
| typealias AsyncIterator = Iterator | ||
| typealias Element = E |
There was a problem hiding this comment.
We can just call the generic Element and then don't need this typealias.
| case none | ||
| } | ||
|
|
||
| private func writeBufferOrEnd(_ byteBuffer: Element?) { |
There was a problem hiding this comment.
nit: rename byteBuffer to just element everywhere
fabianfett
force-pushed
the
ff-add-async-writer
branch
from
56f72d6 to
b9a2b21
Compare
|
Thanks for the thorough code review @dnadoba. Great catches. Hope all is fixed now. |
fabianfett
force-pushed
the
ff-add-async-writer
branch
from
b9a2b21 to
660426f
Compare
fabianfett
force-pushed
the
ff-add-async-writer
branch
from
660426f to
cbc2446
Compare
|
@swift-server-bot test this please |
| private var _state = State.buffering(.init(), nil) | ||
| private let lock = Lock() | ||
|
|
||
| public var hasDemand: Bool { |
There was a problem hiding this comment.
nit: drop the public as the class is internal (and elsewhere)
Motivation
This PR adds an AsyncWriter that we can be used to test our new async upload streaming API.
Changes
Result
We will be able to test async upload streaming API in #518.