Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.2k
http: outgoing cork#29053
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
http: outgoing cork #29053
Uh oh!
There was an error while loading. Please reload this page.
Conversation
961eefa to 0ddf57bCompareronag commented Aug 8, 2019
Not sure exactly how corking a http2 stream works. Would that cork other responses as well? Or does the multiplexing happen on a lower level? |
Uh oh!
There was an error while loading. Please reload this page.
92fa16b to d54f0a5Comparejasnell commented Aug 22, 2019
multiplexing happens on the native layer. corking an Http2Stream has no impact on any other Http2Stream instances. |
addaleax left a comment
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.
LGTM, but this needs a rebase
650eeab to c650a63CompareUh oh!
There was an error while loading. Please reload this page.
c650a63 to 796f541Comparenodejs-github-bot commented Nov 4, 2019
ronag commented Nov 4, 2019
Sorry, there is a test failure. Right now I can't build node due to:
I'll fix this once I've figured out how to build Node again. |
ronag commented Nov 4, 2019
addaleax commented Nov 5, 2019
796f541 to 7290bf1Compareronag commented Nov 6, 2019
rebased |
ronag commented Nov 11, 2019
@Trott failed on compile V8? |
nodejs-github-bot commented Nov 11, 2019
Trott commented Nov 12, 2019 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Test failures in test-http-response-cork: NaN !== undefined at Server.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux/nodes/alpine-latest-x64/test/parallel/test-http-response-cork.js:15:10) |
7290bf1 to b5a611fCompareronag commented Nov 12, 2019 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Hm, that was a minor bug in |
nodejs-github-bot commented Nov 12, 2019
Trott commented Nov 12, 2019
@nodejs/http This could use more reviews. |
addaleax left a comment
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.
LGTM but I’d put the _stream_duplex.js changes in a separate commit (while landing)
PR-URL: #29053 Reviewed-By: Anna Henningsen <[email protected]>
PR-URL: #29053 Reviewed-By: Anna Henningsen <[email protected]>
addaleax commented Nov 13, 2019
PR-URL: #29053 Reviewed-By: Anna Henningsen <[email protected]>
PR-URL: #29053 Reviewed-By: Anna Henningsen <[email protected]>
PR-URL: #29053 Reviewed-By: Anna Henningsen <[email protected]>
PR-URL: #29053 Reviewed-By: Anna Henningsen <[email protected]>
PR-URL: #29053 Reviewed-By: Anna Henningsen <[email protected]>
PR-URL: #29053 Reviewed-By: Anna Henningsen <[email protected]>
Implements cork/uncork on
OutgoingMessageto make it more streamlike.Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes