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

core: ending streams manually is unnecessary, unless they errored #809

Conversation

stephenplusplus
Copy link
Contributor

Fixes #805

This goes over all of our streams and only manually ends or destroys them when it's because of an error. It turns out, calling destroy was akin to smashing a wrecking ball through a user's pipeline. end is usually preferred for its gracefulness, but we don't appear to need either. Streams end up "ending" themselves when they're ready, and they usually know best when that is.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 18, 2015
@stephenplusplus
Copy link
Contributor Author

@leibale would you mind trying this out?

rm -rf node_modules
npm install stephenplusplus/gcloud-node#spp--core-only-end-streams-with-error

@leibale
Copy link
Contributor

leibale commented Aug 18, 2015

listen to end, finish or complete?

@leibale
Copy link
Contributor

leibale commented Aug 18, 2015

it's working with finish :)

@stephenplusplus
Copy link
Contributor Author

Yep, it should work like:

file.createReadStream()
  .on('complete', function() {}) // File fully read
  .pipe(fs.createWriteStream('./file'))
  .on('finish', function() {}) // File fully written

@leibale
Copy link
Contributor

leibale commented Aug 18, 2015

working like a charm, thanks :)

pls add @leibale when you merge this to production

callmehiphop added a commit that referenced this pull request Aug 18, 2015
…ms-with-error

core: ending streams manually is unnecessary, unless they errored
@callmehiphop callmehiphop merged commit 1e00f7f into googleapis:master Aug 18, 2015
@callmehiphop
Copy link
Contributor

Sweet, thanks!

@leibale this has been merged in but we still need to do a release. I think we have one coming very soon!

sofisl pushed a commit that referenced this pull request Nov 10, 2022
- [ ] Regenerate this pull request now.

PiperOrigin-RevId: 468790263

Source-Link: googleapis/googleapis@873ab45

Source-Link: googleapis/googleapis-gen@cb6f37a
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiY2I2ZjM3YWVmZjJhMzQ3MmU0MGE3YmJhY2U4YzY3ZDc1ZTI0YmVlNSJ9
@release-please release-please bot mentioned this pull request Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement. core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants