fix: creating snapshots of S3 objects #164
Merged
+65
−71
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes the creation of snapshots for S3 objects by addressing concurrency issues in the download error handling and adding support for S3 directory objects (zero-byte objects with trailing slashes).
Key changes:
- Simplified error handling in concurrent S3 downloads by using a buffered channel and proper goroutine synchronization
- Refactored duplicate S3 download logic into a reusable helper function
- Added special handling for S3 directory objects to prevent file creation errors
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/util/s3.go | Fixed error channel buffering and simplified error propagation pattern in the S3 download bucket function |
| internal/snapshot/s3.go | Refactored duplicate download logic into processDownloadObject helper function and added directory object handling |
| .gitignore | Added .DS_Store to ignore macOS system files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cv711
force-pushed
the
fix/snapshot-s3
branch
from
956aae9 to
da28027
Compare
cv711
force-pushed
the
fix/snapshot-s3
branch
from
5382f7e to
7c30631
Compare
TrayserCassa
approved these changes
Dec 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request refactors and improves the S3 backup logic for asset snapshots, focusing on code reuse, error handling, and concurrency management. The main change is the extraction of the S3 object download logic into a reusable function, which simplifies the backup process for both private and public buckets. Additionally, the S3 downloader's concurrency and error propagation mechanisms are improved for reliability.
Refactoring and code reuse:
processDownloadObject, which is now used for both private and public bucket backups. This reduces code duplication and centralizes file handling and error reporting. [1] [2]Error handling and logging improvements:
S3 downloader concurrency and error propagation:
DownloadBucketmethod inS3Downloaderto ensure that the error channel is properly closed after all goroutines complete, and that the first encountered error is returned immediately to halt further processing. Also added explicit comments and improved semaphore handling for clarity. [1] [2] [3] [4]Minor improvements:
stringspackage import to support directory/file checks in the new download logic.