Skip to content

stream: async iterator improvements#31316

Closed
ronag wants to merge 5 commits into
nodejs:masterfrom
nxtedition:stream-async-iterator
Closed

stream: async iterator improvements#31316
ronag wants to merge 5 commits into
nodejs:masterfrom
nxtedition:stream-async-iterator

Conversation

@ronag
Copy link
Copy Markdown
Member

@ronag ronag commented Jan 11, 2020 •
edited
Loading

Includes a few different fixes separated into commits:

  • Adds support to create async iterators from v1 streams.
  • Normalize destroying non conforming streams.
  • Implement throw on the iterator.
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
Copy link
Copy Markdown
Member Author

ronag commented Jan 11, 2020

ping @addaleax
ronag
ronag commented Jan 11, 2020
View reviewed changes
Comment thread lib/internal/streams/async_iterator.js Outdated
@ronag ronag force-pushed the stream-async-iterator branch 2 times, most recently from f87b6e6 to 7558199 Compare January 11, 2020 16:27
@ronag ronag changed the title stream: add async iterator support for v1 streams stream: async iterator fixes Jan 11, 2020
@ronag ronag force-pushed the stream-async-iterator branch 6 times, most recently from a4f1b62 to 00d40b3 Compare January 11, 2020 17:30
@ronag ronag changed the title stream: async iterator fixes stream: async iterator improvements Jan 11, 2020
@ronag ronag force-pushed the stream-async-iterator branch from 00d40b3 to d8dbe8a Compare January 11, 2020 18:07
mcollina
mcollina reviewed Jan 11, 2020
View reviewed changes
Comment thread lib/internal/streams/async_iterator.js Outdated
Comment thread test/parallel/test-stream-readable-async-iterators.js Outdated
@ronag ronag force-pushed the stream-async-iterator branch 2 times, most recently from 1e6c55e to f1c4e5a Compare January 12, 2020 00:06
mcollina
mcollina reviewed Jan 12, 2020
View reviewed changes
Comment thread test/parallel/test-stream-readable-async-iterators.js Outdated
mcollina
mcollina reviewed Jan 12, 2020
View reviewed changes
Comment thread lib/internal/streams/legacy.js Outdated
@ronag ronag force-pushed the stream-async-iterator branch 2 times, most recently from 5c034b4 to 54a560b Compare January 12, 2020 11:44
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Jan 12, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/28400/
mcollina
mcollina approved these changes Jan 12, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm
@BridgeAR BridgeAR added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. stream Issues and PRs related to the stream subsystem. labels Jan 12, 2020
reklatsmasters
reklatsmasters reviewed Jan 12, 2020
View reviewed changes
Comment thread lib/internal/streams/async_iterator.js Outdated
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@ronag ronag closed this Jan 26, 2020
codebytere pushed a commit that referenced this pull request Feb 17, 2020
@ronag @codebytere
stream: add async iterator support for v1 streams
20d0a0e 
PR-URL: #31316
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
codebytere pushed a commit that referenced this pull request Feb 17, 2020
@ronag @codebytere
stream: normalize async iterator stream destroy
5a95fa4 
PR-URL: #31316
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
codebytere pushed a commit that referenced this pull request Feb 17, 2020
@ronag @codebytere
stream: implement throw for async iterator
3046648 
PR-URL: #31316
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
@codebytere codebytere mentioned this pull request Feb 17, 2020
Merged
@codebytere
Copy link
Copy Markdown
Member

codebytere commented Feb 29, 2020

@ronag the last two commits in this PR need manual backports
@MylesBorins MylesBorins mentioned this pull request Mar 9, 2020
Closed
7 tasks
@ronag ronag mentioned this pull request Mar 10, 2020
Closed
4 tasks
ronag added a commit to nxtedition/node that referenced this pull request Mar 10, 2020
@ronag
stream: simplify pipeline
0abd4f0 
PR-URL: nodejs#31316
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Backport-PR-URL: nodejs#32174
ronag added a commit to nxtedition/node that referenced this pull request Mar 10, 2020
@ronag
stream: re-use legacy destroyer
2578e5d 
PR-URL: nodejs#31316
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Backport-PR-URL: nodejs#32174
MylesBorins pushed a commit that referenced this pull request Mar 10, 2020
@ronag @MylesBorins
stream: simplify pipeline
7ce1cc9 
Backport-PR-URL: #32174
PR-URL: #31316
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 10, 2020
@ronag @MylesBorins
stream: re-use legacy destroyer
4b04bf8 
Backport-PR-URL: #32174
PR-URL: #31316
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Mar 10, 2020
Merged
@codebytere
Copy link
Copy Markdown
Member

codebytere commented Mar 15, 2020

@ronag should this go to v12.x, and if yes, can you open a manual backport? Given the recent issues on v13.x i'd like to play it safe so feel free to update the label to don't-land.
@targos targos removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 25, 2020
This was referenced Jun 16, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stream Issues and PRs related to the stream subsystem.