- Notifications
You must be signed in to change notification settings - Fork136
implement new streamwriter api#291
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
5924d12 to0cb498aCompare| /// - 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Given that there is discussion about possibly breaking API in the nearish futureanyway, we may find we want to delay this until then to resolve this problem.
0cb498a to30a7286Compare| publicstructStreamWriter2{ | ||
| publicletallocator:ByteBufferAllocator | ||
| letonChunk:(IOData)->EventLoopFuture<Void> | ||
| letonComplete:EventLoopPromise<Void> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Given that there is discussion about possibly breaking API in the nearish futureanyway, we may find we want to delay this until then to resolve this problem.
ktoso commentedAug 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 |
Uh oh!
There was an error while loading.Please reload this page.
🚧 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