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
fix(example): drive export end before finish writing the file #1798
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't we just switch end to finish I believe? I'd love to keep the change to a minimum and not introduce the Promise.all.
Thanks for hunting this down.
the 'end' event is the end of the response ( We can just ignore potential error with the response:
|
I believe |
@ttv20 👋 still want to see this over the finish line? I think you also need to sign the CLA. |
No. 'finish' is event of From node doc:
stream.Writable -> 'finish' event:
So I think it necessary to wait for |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
@ttv20 thank you for your work on this, it's much appreciated 😄 we want a sample that works! I just chatted with my colleague @callmehiphop, his recommendation was this simplification: await new Promise((resolve, reject) => {
res.data
.on('error', reject)
.pipe(dest)
.on('error', reject)
.on('finish', resolve);
}); I believe this would have the desired effect, where the |
drive export end before finish writing the file
Tested, Work as expected. |
@ttv20 this PR is great 👍 thanks for the patience. |
Codecov Report
@@ Coverage Diff @@
## master #1798 +/- ##
=======================================
Coverage 96.51% 96.51%
=======================================
Files 2 2
Lines 431 431
Branches 7 7
=======================================
Hits 416 416
Misses 15 15 Continue to review full report at Codecov.
|
@ttv20 thanks a ton for the contribution 😄 and for patience in it landing. |
Fixes #1788