implement new streamwriter api#291
Conversation
5924d12 to
0cb498a
Compare
| /// - length: Body size. Request validation will be failed with `HTTPClientErrors.contentLengthMissing` if nil, | ||
| /// unless `Transfer-Encoding: chunked` header is set. | ||
| /// - stream: Body chunk provider. | ||
| public static func stream2(length: Int? = nil, _ stream: @escaping (StreamWriter2) -> Void) -> Body { |
There was a problem hiding this comment.
I we add method with the same name but different type we will break compilation, because some calls could become ambiguous 😢
There was a problem hiding this comment.
Given that there is discussion about possibly breaking API in the nearish future anyway, we may find we want to delay this until then to resolve this problem.
0cb498a to
30a7286
Compare
| public struct StreamWriter2 { | ||
| public let allocator: ByteBufferAllocator | ||
| let onChunk: (IOData) -> EventLoopFuture<Void> | ||
| let onComplete: EventLoopPromise<Void> |
There was a problem hiding this comment.
Is there a reason we're writing this as a callback-taking type rather than just holding onto the task object?
There was a problem hiding this comment.
and "forward" all write requests to it's channel? that might be a good idea, will try that, thanks!
| /// - length: Body size. Request validation will be failed with `HTTPClientErrors.contentLengthMissing` if nil, | ||
| /// unless `Transfer-Encoding: chunked` header is set. | ||
| /// - stream: Body chunk provider. | ||
| public static func stream2(length: Int? = nil, _ stream: @escaping (StreamWriter2) -> Void) -> Body { |
There was a problem hiding this comment.
Given that there is discussion about possibly breaking API in the nearish future anyway, we may find we want to delay this until then to resolve this problem.
|
Just as a heads up, the main development branch has been changed to This PR has been re-targeted to main and should just work. However when performing rebases etc please keep this in mind -- you may want to fetch the main branch and rebase onto the |
🚧 This is slightly WIP.
Revamps
StreamWriter.Motivation:
There are two issues with current
StreamWriterAPI:EventLoopFutureis completed, which is not ideal.Modifications:
StreamWriterAPIStreamWriter2(really need help with the naming here) with the following API:Result:
Closes #194
Closes #264