implement new streamwriter api#291
Uh oh!
There was an error while loading. Please reload this page.
Conversation
artemredkinforce-pushed the revamp_stream_writer branch 2 times, most recently from 5924d12 to 0cb498aCompare
| /// - length: Body size. Request validation will be failed with `HTTPClientErrors.contentLengthMissing` if nil, | ||
| /// unless `Transfer-Encoding: chunked` header is set. | ||
| /// - stream: Body chunk provider. | ||
| publicstaticfunc 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.
artemredkinforce-pushed the revamp_stream_writer branch from 0cb498a to 30a7286Compare
| publicstructStreamWriter2{ | ||
| publicletallocator:ByteBufferAllocator | ||
| letonChunk:(IOData)->EventLoopFuture<Void> | ||
| letonComplete: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. | ||
| publicstaticfunc 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.
ktoso commented Aug 20, 2020
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 |
3 participants
🚧 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