FileLu: Update code to support multipart upload and performance improvements.#9108
FileLu: Update code to support multipart upload and performance improvements.#9108ncw merged 4 commits intorclone:masterfrom
Conversation
|
i hope this fix get included next time so i do not need to compile rclone manually ;) |
ncw
left a comment
ncw
left a comment
There was a problem hiding this comment.
Hi @kingston125 sorry for the delay in looking at this.
I put some things to look at inline.
Do the integration tests pass properly with this change?
Can you reorganize this into two commits only please
- First your performance improvements in
Update - Second your multipart upload code
Thanks
Nick
backend/filelu/filelu_client.go
Outdated
| var result MultipartInitResponse | ||
|
|
||
| err := f.pacer.Call(func() (bool, error) { | ||
| req, err := http.NewRequestWithContext(ctx, "GET", apiURL, nil) |
There was a problem hiding this comment.
Not sure why you aren't using lib/rest here?
In fact you don't appear to be using it anywhere in the backend :-(
There was a problem hiding this comment.
I have done this, and I can update other functions to use lib/rest if required. Maybe in another pull request?
There was a problem hiding this comment.
If we can get the whole backend using lib/rest then the code will be shorter and it will be easier to maintain. lib/rest has a few extra rclone special features in too.
So if you have the time to rework it to use lib/rest in a new PR that would be great.
|
Hi @ncw I have fixed and updated the code. Please let me know if I’ve missed anything, and I will do my best to resolve it. Your detailed comments are very helpful and have helped me a lot in navigating and fixing the issue. Thank you. |
Avoid buffering the entire file in memory during download, especially for large files.
ncw
left a comment
ncw
left a comment
There was a problem hiding this comment.
Thanks @kingston125 that is great. 4 nice easy to read commits :-)
I've just reworded the commits so they start with filelu: and I've also made each commit compile in sequence which involved a small amount of re-arrangement.
I'll just let the CI complete then I'll merge.
Thank you
3 participants
What is the purpose of this change?
Adds support for uploading large files using multipart uploads and performance improvements.
Was the change discussed in an issue or in the forum before?
No
Checklist