Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[v12.x backport] http2: make HTTP2ServerResponse more streams compliant #31444

Closed
wants to merge 1 commit into from
Closed

[v12.x backport] http2: make HTTP2ServerResponse more streams compliant #31444

wants to merge 1 commit into from

Conversation

sxa
Copy link
Member

@sxa sxa commented Jan 21, 2020

HTTP2ServerResponse.write would behave differently than
both http1 and streams. This PR makes it more compliant
with stream.Writable behaviour.

PR-URL: #30964
Refs: #29529

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@ronag
http2: make HTTP2ServerResponse more streams compliant
ba33c8f 
HTTP2ServerResponse.write would behave differently than
both http1 and streams. This PR makes it more compliant
with stream.Writable behaviour.

PR-URL: nodejs#30964
Refs: nodejs#29529
Backport-PR-URL: nodejs#31444
@nodejs-github-bot nodejs-github-bot added http2 Issues or PRs related to the http2 subsystem. v12.x labels Jan 21, 2020
@sxa
Copy link
Member Author

sxa commented Jan 21, 2020 •
edited
Loading

(I don't yet have authority to run node-test-pull-request on this)

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/28537/

@sxa sxa requested a review from targos January 21, 2020 18:35
@sxa
Copy link
Member Author

sxa commented Jan 22, 2020

Test at https://ci.nodejs.org/job/node-test-binary-windows-js-suites/1213/RUN_SUBSET=0,nodes=win2016-COMPILED_BY-vs2017/testReport/(root)/test/sequential_test_worker_prof_/ showing an error message of flaky (1) ... Ref: #26401 so unless someone says otherwise I assume this is ok

MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
@ronag @MylesBorins
http2: make HTTP2ServerResponse more streams compliant
f63e440 
HTTP2ServerResponse.write would behave differently than
both http1 and streams. This PR makes it more compliant
with stream.Writable behaviour.

Backport-PR-URL: #31444
PR-URL: #30964
Refs: #29529
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@MylesBorins
Copy link
Contributor

landed in f63e440

The flakes are not a big deal for CI, thanks for pointing it out. I udpated the commit message while landing, in the future it is fine to keep the original commit message and we can add a Backport-PR-URL: While landing

BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
@ronag @BethGriggs
http2: make HTTP2ServerResponse more streams compliant
92a9dac 
HTTP2ServerResponse.write would behave differently than
both http1 and streams. This PR makes it more compliant
with stream.Writable behaviour.

Backport-PR-URL: #31444
PR-URL: #30964
Refs: #29529
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@targos targos Awaiting requested review from targos

Assignees
No one assigned
Labels
http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sxa @nodejs-github-bot @MylesBorins @ronag