Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: allow files in directories to be downloaded onto local machine #2199

Merged
merged 15 commits into from
May 21, 2024

Conversation

hochoy
Copy link
Contributor

@hochoy hochoy commented May 18, 2023

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary) (n/a)

Fixes #2200 🦕

@product-auto-label product-auto-label bot added size: s Pull request size is small. api: storage Issues related to the googleapis/nodejs-storage API. labels May 18, 2023
@ddelgrosso1 ddelgrosso1 added the owlbot:run Add this label to trigger the Owlbot post processor. label May 18, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label May 18, 2023
@ddelgrosso1 ddelgrosso1 added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels May 18, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label May 18, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 18, 2023
@hochoy hochoy marked this pull request as ready for review May 18, 2023 20:20
@hochoy hochoy requested review from a team as code owners May 18, 2023 20:20
@hochoy
Copy link
Contributor Author

hochoy commented May 18, 2023

@ddelgrosso1 , I tried running samples-test and system-test but based on the logs, it appears those require an active project in order to do an e2e/live test against real GCP infrastructure.

Is that accurate?
How expensive is that if I run it on my own project? Should I be triggering it here in the repo instead (and leverage your repo's cloudbuild/github actions infra)?

@hochoy
Copy link
Contributor Author

hochoy commented May 18, 2023

Also, I would like to add a test to check that the files are truly downloaded into the local machine or memory.
Would you recommend I give that a shot? Or is that overkill?

@ddelgrosso1
Copy link
Contributor

@ddelgrosso1 , I tried running samples-test and system-test but based on the logs, it appears those require an active project in order to do an e2e/live test against real GCP infrastructure.

Is that accurate? How expensive is that if I run it on my own project? Should I be triggering it here in the repo instead (and leverage your repo's cloudbuild/github actions infra)?

That is accurate. I wouldn't worry about running these on your own, they get run in the CI pipeline each time a commit is pushed. However, unit tests should work without issue locally.

Also, I would like to add a test to check that the files are truly downloaded into the local machine or memory. Would you recommend I give that a shot? Or is that overkill?

If you feel up to it a unit test can probably be created to test this. I can look to see if we have any similar tests elsewhere that might serve as a guide.

One thing I will do is to cleanup the the JS Docs to make it abundantly clear that not supplying a prefix will result in the files downloaded to memory.

@product-auto-label product-auto-label bot added size: xs Pull request size is extra small. and removed size: s Pull request size is small. labels May 28, 2023
@product-auto-label product-auto-label bot added size: s Pull request size is small. and removed size: xs Pull request size is extra small. labels May 29, 2023
@product-auto-label product-auto-label bot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels May 29, 2023
@ddelgrosso1 ddelgrosso1 added the owlbot:run Add this label to trigger the Owlbot post processor. label May 21, 2024
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label May 21, 2024
@ddelgrosso1 ddelgrosso1 added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels May 21, 2024
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label May 21, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 21, 2024
Copy link
Contributor

@ddelgrosso1 ddelgrosso1 left a comment

Choose a reason for hiding this comment

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

LGTM

@ddelgrosso1 ddelgrosso1 merged commit 9f62429 into googleapis:main May 21, 2024
16 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
api: storage Issues related to the googleapis/nodejs-storage API. size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

transferManager.downloadManyFiles not writing files locally
5 participants