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

weather-dl now uses gsutil cp for file upload. #265

Merged
merged 13 commits into from
Dec 6, 2022
Merged

weather-dl now uses gsutil cp for file upload. #265

merged 13 commits into from
Dec 6, 2022

Conversation

alxmrs
Copy link
Collaborator

@alxmrs alxmrs commented Dec 2, 2022

A partial implementation of #256. My intention here is to see if this can speed up weather-dl requests.

A partial implementation of #256. My intention here is to see if this can speed up weather-dl requests.
@alxmrs alxmrs changed the title weather-dl now uses gsutil cp for file upload. weather-dl now uses gcloud storage alpha cp for file upload. Dec 2, 2022
Copy link
Collaborator

@mahrsee1997 mahrsee1997 left a comment

Choose a reason for hiding this comment

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

Overall PR LGTM. I'm happy to get it merged after resolving the above comments & fixing the tests.

Kudos !! This is a noteworthy change.

alxmrs and others added 2 commits December 2, 2022 17:16
Thanks @shoyer.

Co-authored-by: Stephan Hoyer <shoyer@google.com>
@alxmrs alxmrs changed the title weather-dl now uses gcloud storage alpha cp for file upload. weather-dl now uses gcloud alpha storage cp for file upload. Dec 3, 2022
@alxmrs alxmrs changed the title weather-dl now uses gcloud alpha storage cp for file upload. weather-dl now uses gsutil cp for file upload. Dec 5, 2022
@alxmrs alxmrs merged commit f347b70 into main Dec 6, 2022
@alxmrs alxmrs deleted the dl-gsutil-cp branch December 6, 2022 00:19
deepgabani8 pushed a commit that referenced this pull request Dec 9, 2022
A partial implementation of #256. Here, we copy data using `gsutil cp` instead of a python routine. This speeds things up, since `gsutil` will parallelize uploads of large files. 

* weather-dl now uses `gsutil cp` for file upload.

A partial implementation of #256. My intention here is to see if this can speed up weather-dl requests.

* Temporary: no gsutil version.

* Bump weather-dl version.

* pinning gsutil version.

* Use gcloud alpha storage cp, which is even faster :)

* Set up gcloud sdk, accounting for runtime auth issue.

* Added error handling to the subprocess call for copying.

Co-authored-by: Rahul Mahrsee <86819420+mahrsee1997@users.noreply.github.com>

* fix: added import.

* Changing subprocess invocation to be more secure.

Thanks @shoyer.

Co-authored-by: Stephan Hoyer <shoyer@google.com>

* nit: dst, not dest.

* nit: remove gcloud pip dependency.

* Using gsutil for now until we upgrade project deps.

Co-authored-by: Rahul Mahrsee <86819420+mahrsee1997@users.noreply.github.com>
Co-authored-by: Stephan Hoyer <shoyer@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants