-
Notifications
You must be signed in to change notification settings - Fork 621
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
feat(sdk): enable uploading ranges of files in core #7660
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7660 +/- ##
==========================================
+ Coverage 74.26% 75.85% +1.58%
==========================================
Files 500 500
Lines 55656 54061 -1595
==========================================
- Hits 41334 41007 -327
+ Misses 13915 12646 -1269
- Partials 407 408 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
db1195f
to
3525de7
Compare
if task.Offset > 0 || (task.Length > 0 && task.Length < stat.Size()) { | ||
if task.Offset+task.Length > stat.Size() { | ||
ft.logger.Error("file transfer: upload: offset + length is greater than the file size", | ||
"offset", task.Offset, "length", task.Length, "file size", stat.Size()) | ||
task.Length = int64(stat.Size() - task.Offset) | ||
} | ||
sectionReader := io.NewSectionReader(file, task.Offset, task.Length) | ||
reader = sectionReader | ||
task.Size = task.Length |
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.
i think in any case this requires explanation it's not clear why you do the math you do
why it doesn't include the range
Description
Add
Offset
andLength
fields to thefiletransfer.Task
type, and alterDefaultFileTransfer.Upload
so that if those are non-zero only the specified range is uploaded.This will be used for multipart uploads (draft: #7659).
Question: the
Task
takes a file path, rather than a file handle. This means that to upload ranges, it needs to open the file (create a file handle) for each range it needs to upload, and large files (>1TB) can be broken up into up to 10,000 pieces. Is this a problem?Testing
Added two small unit tests for functionality.