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.

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

stream: add suport for abort signal in finished() for webstreams #46403

Merged

Conversation

debadree25
Copy link
Member

In our original PR we hadn't added support for AbortSignal when using webstreams with finished() hence added the same here

Refs: #46205
Refs: #37354

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jan 29, 2023
@debadree25 debadree25 changed the title stream: add supoort for abort signal in finished() for webstreams stream: add suport for abort signal in finished() for webstreams Jan 29, 2023
@debadree25 debadree25 force-pushed the ft/abort-signal-webstreams-finish branch from eb31a9c to 4312987 Compare January 29, 2023 05:29
Copy link
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

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 30, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 30, 2023
@nodejs-github-bot
Copy link
Collaborator

@lpinca lpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 2, 2023
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Feb 2, 2023
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/46403
✔  Done loading data for nodejs/node/pull/46403
----------------------------------- PR info ------------------------------------
Title      stream: add suport for abort signal in finished() for webstreams (#46403)
Author     Debadree Chatterjee  (@debadree25)
Branch     debadree25:ft/abort-signal-webstreams-finish -> nodejs:main
Labels     needs-ci
Commits    2
 - stream: add suport for abort signal in finished() for webstreams
 - fixup! add tests
Committers 1
 - Debadree Chatterjee 
PR-URL: https://github.com/nodejs/node/pull/46403
Refs: https://github.com/nodejs/node/pull/46205
Refs: https://github.com/nodejs/node/pull/37354
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Matteo Collina 
Reviewed-By: Robert Nagy 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/46403
Refs: https://github.com/nodejs/node/pull/46205
Refs: https://github.com/nodejs/node/pull/37354
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Matteo Collina 
Reviewed-By: Robert Nagy 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sun, 29 Jan 2023 05:27:07 GMT
   ✔  Approvals: 3
   ✔  - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/46403#pullrequestreview-1274265207
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/46403#pullrequestreview-1274266823
   ✔  - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/46403#pullrequestreview-1274274475
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2023-01-30T11:22:45Z: https://ci.nodejs.org/job/node-test-pull-request/49247/
- Querying data for job/node-test-pull-request/49247/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
   f84de0ad4c..75080830a4  main       -> origin/main
✔  origin/main is now up-to-date
main is out of sync with origin/main. Mismatched commits:
 - 1f708d2037 stream: dont access Object.prototype.type during TransformStream init
 - 75080830a4 stream: dont access Object.prototype.type during TransformStream init
--------------------------------------------------------------------------------
HEAD is now at 75080830a4 stream: dont access Object.prototype.type during TransformStream init
   ✔  Reset to origin/main
- Downloading patch for 46403
From https://github.com/nodejs/node
 * branch                  refs/pull/46403/merge -> FETCH_HEAD
✔  Fetched commits as 75080830a40d..43129876236c
--------------------------------------------------------------------------------
[main 3cbb426140] stream: add suport for abort signal in finished() for webstreams
 Author: Debadree Chatterjee 
 Date: Sun Jan 29 09:48:41 2023 +0530
 1 file changed, 26 insertions(+), 3 deletions(-)
[main 3034793ca3] fixup! add tests
 Author: Debadree Chatterjee 
 Date: Sun Jan 29 10:52:07 2023 +0530
 1 file changed, 90 insertions(+)
   ✔  Patches applied
There are 2 commits in the PR. Attempting autorebase.
Rebasing (2/4)

Executing: git node land --amend --yes
⚠ Found Refs: #46205, skipping..
--------------------------------- New Message ----------------------------------
stream: add suport for abort signal in finished() for webstreams

Refs: #46205
PR-URL: #46403
Refs: #37354
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Matteo Collina matteo.collina@gmail.com
Reviewed-By: Robert Nagy ronagy@icloud.com

[detached HEAD b6ab6bb94a] stream: add suport for abort signal in finished() for webstreams
Author: Debadree Chatterjee debadree333@gmail.com
Date: Sun Jan 29 09:48:41 2023 +0530
1 file changed, 26 insertions(+), 3 deletions(-)
Rebasing (3/4)
Rebasing (4/4)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
fixup! add tests

PR-URL: #46403
Refs: #46205
Refs: #37354
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Matteo Collina matteo.collina@gmail.com
Reviewed-By: Robert Nagy ronagy@icloud.com

[detached HEAD f19ef3aaa8] fixup! add tests
Author: Debadree Chatterjee debadree333@gmail.com
Date: Sun Jan 29 10:52:07 2023 +0530
1 file changed, 90 insertions(+)

Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/4077785291

@lpinca lpinca added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Feb 2, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 2, 2023
@nodejs-github-bot nodejs-github-bot merged commit ebcc711 into nodejs:main Feb 2, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in ebcc711

MylesBorins pushed a commit that referenced this pull request Feb 18, 2023
Refs: #46205
PR-URL: #46403
Refs: #37354
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
@MylesBorins MylesBorins mentioned this pull request Feb 19, 2023
debadree25 added a commit to debadree25/node that referenced this pull request Feb 27, 2023
Refs: nodejs#46205
PR-URL: nodejs#46403
Refs: nodejs#37354
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
debadree25 added a commit to debadree25/node that referenced this pull request Feb 27, 2023
Refs: nodejs#46205
PR-URL: nodejs#46403
Refs: nodejs#37354
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
debadree25 added a commit to debadree25/node that referenced this pull request Feb 27, 2023
Refs: nodejs#46205
PR-URL: nodejs#46403
Refs: nodejs#37354
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
danielleadams pushed a commit that referenced this pull request Apr 11, 2023
Refs: #46205
PR-URL: #46403
Refs: #37354
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants