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

Jump to bottom

feat(storage/transfermanager): add DownloadDirectory #10430

Merged
merged 9 commits into from
Jul 3, 2024
Merged

feat(storage/transfermanager): add DownloadDirectory #10430

merged 9 commits into from
Jul 3, 2024

Conversation

BrennaEpp
Copy link
Contributor

@BrennaEpp BrennaEpp commented Jun 25, 2024 •
edited
Loading

mvp

Follow-up PRs:

  • Clean up file on failure
  • Fail if the file already exists instead of overwriting
  • Skipifexists
  • strip prefix option
  • empty object handling (if necessary)

@product-auto-label product-auto-label bot added the api: storage Issues related to the Cloud Storage API. label Jun 25, 2024
@BrennaEpp
feat(storage/transfermanager): add DownloadDirectory
5ecd24b 
Prototype.

Follow-up PRs:
- Cleanup on failure
- Skipifexists + option to fail if exists?
- strip prefix ? (strip prefix on local files)
- if empty object, create a directory instead ?
- possibly more integration tests
@BrennaEpp BrennaEpp marked this pull request as ready for review June 25, 2024 20:55
@BrennaEpp BrennaEpp requested review from a team as code owners June 25, 2024 20:55
@BrennaEpp BrennaEpp requested a review from tritone June 25, 2024 22:27
tritone
tritone reviewed Jun 26, 2024
View reviewed changes
Copy link
Contributor

@tritone tritone left a comment

Choose a reason for hiding this comment

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

A few minor things; generally looks good

tritone
tritone reviewed Jun 28, 2024
View reviewed changes
@BrennaEpp BrennaEpp requested a review from tritone June 28, 2024 23:38
tritone
tritone approved these changes Jul 3, 2024
View reviewed changes
Copy link
Contributor

@tritone tritone left a comment

Choose a reason for hiding this comment

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

One more minor suggestion, otherwise looks good.

storage/transfermanager/downloader.go Outdated
Callback func([]DownloadOutput)

// OnObjectDownload will run after every finished object download.
// It can only be set if the Downloader has the [WithCallbacks] option set.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could probably loosen this requirement? A user could want to run in sync mode and still use this to track/log progress.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@BrennaEpp BrennaEpp requested a review from tritone July 3, 2024 17:20
@BrennaEpp BrennaEpp enabled auto-merge (squash) July 3, 2024 17:54
@BrennaEpp BrennaEpp merged commit 0d0e5dd into googleapis:main Jul 3, 2024
8 checks passed
@release-please release-please bot mentioned this pull request Jul 3, 2024
Merged
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@tritone tritone tritone approved these changes

Assignees
No one assigned
Labels
api: storage Issues related to the Cloud Storage API.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@BrennaEpp @tritone