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: finish pipeline if dst closes before src #43701

Merged
merged 2 commits into from
Jul 11, 2022

Conversation

ronag
Copy link
Member

@ronag ronag commented Jul 6, 2022

If the destination stream is closed before the source has completed
the pipeline should finnish with premature close.

Fixes: #43682

@ronag ronag added the stream Issues and PRs related to the stream subsystem. label Jul 6, 2022
@ronag ronag requested review from mcollina and lpinca July 6, 2022 11:25
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/streams

@ronag
Copy link
Member Author

ronag commented Jul 6, 2022

@nodejs/streams @vweevers

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jul 6, 2022
If the destination stream is closed before the source has completed
the pipeline should finnish with premature close.

Fixes: nodejs#43682
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

@ronag ronag added request-ci Add this label to start a Jenkins CI on a PR. and removed needs-ci PRs that need a full CI run. labels Jul 6, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 6, 2022
@ronag ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 6, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 6, 2022
@ronag ronag added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 7, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 7, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/43701
✔  Done loading data for nodejs/node/pull/43701
----------------------------------- PR info ------------------------------------
Title      stream: finish pipeline if dst closes before src (#43701)
Author     Robert Nagy  (@ronag)
Branch     ronag:pipeline-finish -> nodejs:main
Labels     stream
Commits    2
 - stream: finish pipeline if dst closes before src
 - fixup
Committers 1
 - Robert Nagy 
PR-URL: https://github.com/nodejs/node/pull/43701
Fixes: https://github.com/nodejs/node/issues/43682
Reviewed-By: Matteo Collina 
Reviewed-By: Luigi Pinca 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/43701
Fixes: https://github.com/nodejs/node/issues/43682
Reviewed-By: Matteo Collina 
Reviewed-By: Luigi Pinca 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - fixup
   ℹ  This PR was created on Wed, 06 Jul 2022 11:25:18 GMT
   ✔  Approvals: 2
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/43701#pullrequestreview-1029932092
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/43701#pullrequestreview-1029938147
   ✖  This PR needs to wait 27 more hours to land
   ✖  Last GitHub CI failed
   ✖  No Jenkins CI runs detected
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/2628008197

@nodejs-github-bot nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Jul 7, 2022
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@mcollina mcollina added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jul 11, 2022
@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 Jul 11, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/43701
✔  Done loading data for nodejs/node/pull/43701
----------------------------------- PR info ------------------------------------
Title      stream: finish pipeline if dst closes before src (#43701)
Author     Robert Nagy  (@ronag)
Branch     ronag:pipeline-finish -> nodejs:main
Labels     stream, commit-queue-squash
Commits    2
 - stream: finish pipeline if dst closes before src
 - fixup
Committers 1
 - Robert Nagy 
PR-URL: https://github.com/nodejs/node/pull/43701
Fixes: https://github.com/nodejs/node/issues/43682
Reviewed-By: Matteo Collina 
Reviewed-By: Luigi Pinca 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/43701
Fixes: https://github.com/nodejs/node/issues/43682
Reviewed-By: Matteo Collina 
Reviewed-By: Luigi Pinca 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - fixup
   ℹ  This PR was created on Wed, 06 Jul 2022 11:25:18 GMT
   ✔  Approvals: 2
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/43701#pullrequestreview-1029932092
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/43701#pullrequestreview-1029938147
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-07-10T18:43:08Z: https://ci.nodejs.org/job/node-test-pull-request/45277/
- Querying data for job/node-test-pull-request/45277/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/2648025585

@mcollina
Copy link
Member

@ronag I don't know why the commit bot is failing, you might want to land this manually.

@ronag ronag added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 11, 2022
@ronag
Copy link
Member Author

ronag commented Jul 11, 2022

I don't have all the node tool setup on my computer. @Trott would you mind helping me with landing this and #43685. I promise to sort out my setup once I'm back from vacation.

@targos
Copy link
Member

targos commented Jul 11, 2022

@mcollina it's failing because "Commits were pushed since the last review".

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 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 Jul 11, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 11, 2022
@nodejs-github-bot nodejs-github-bot merged commit e4bf5dc into nodejs:main Jul 11, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in e4bf5dc

targos pushed a commit that referenced this pull request Jul 12, 2022
If the destination stream is closed before the source has completed
the pipeline should finnish with premature close.

Fixes: #43682

PR-URL: #43701
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pipeline() doesn't finish with sandwiched duplex stream
5 participants