-
Notifications
You must be signed in to change notification settings - Fork 121
Make downloads resumable across app sessions #187
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
base: main
Are you sure you want to change the base?
Make downloads resumable across app sessions #187
Conversation
@ardaatahan given that I know you guys are using this functionality, do you have any thoughts? |
90a189d
to
988ee59
Compare
I've resolved the conflicts from the latest merged PR. @FL33TW00D, @pcuenca, do you have any thoughts on this? The lack of resumable downloads across app sessions is one of the biggest pain points for users of my app Local Chat, and I imagine the same is true for other apps that use swift-transformers. Multi-gigabyte downloads often take several minutes to complete, and if the app goes to the background or is terminated, the download needs to start over again from zero. This solution uses UserDefaults to store data on downloads in progress. Do think this behavior should be optional, or is it acceptable for all users of swift-transformers? |
Great work, @DePasqualeOrg! This is a very valuable addition to the repo. I'd like to suggest considering an approach similar to what the Hugging Face Hub library uses, which has a well-tested implementation for resumable downloads. This approach supports downloading files from where they left off when the app is restarted and the download is reinitiated (cc: @FL33TW00D, @pcuenca):
This approach is quite elegant and has several advantages:
Here are some relevant links to the
Let me know what you all think, and don't hesitate to reach out if I can help with anything! |
988ee59
to
77df21d
Compare
Thanks for your feedback, @ardaatahan! That approach makes a lot more sense, and I should have checked how this is implemented in Python first. I've revised my implementation and am saving the |
72958ed
to
a5a5875
Compare
@DePasqualeOrg I'd recommend keeping |
a5a5875
to
2b9b873
Compare
Maintaining a parallel directory structure inside a If someone wants to add functionality for a separate download directory, they're welcome to do so, but this is already a huge improvement on the current behavior, which doesn't support resuming downloads across app sessions. |
This is indeed a huge improvement over the current behavior, and I appreciate the work you've put into making resumable downloads better! I do have reservations about storing |
The same directory structure needs to be maintained in the download directory. Some repos have nested directory structures. You'll end up with empty directories in the download directory after downloads complete. If you don't want to leave those empty directories there, you'll need to figure out how to delete them without interfering with other downloads. I think it's unlikely that users are iterating through files in the model directory without checking file extensions. The I think this solution will work fine for the vast majority of users of swift-transformers. I'll let the maintainers of this package decide whether the more complex solution you suggested is necessary, and if so, someone else can implement it. This is the solution that I'm offering, which can either be merged as is or built upon. |
@pcuenca, @FL33TW00D, I suggest that if no one wants to implement the more complex solution that @ardaatahan suggested, we review and merge this soon, since it solves one of the biggest pain points that users of this library face. |
@DePasqualeOrg, I'm planning to work on this throughout the weekend to implement the incomplete download mechanism I mentioned, I'll keep you posted. |
@ardaatahan Thanks a lot, but no need to work on the weekend in my opinion! Happy to help whenever you get started 🤗 |
@DePasqualeOrg, @FL33TW00D, @pcuenca, I'm done adding the Hugging Face Hub style incomplete download mechanism. I'll do some more testing and cleanup the code and the PR should be ready before end of this week! |
@DePasqualeOrg FYI — I opened a PR to your local branch with changes that improve incomplete download handling. Could you give it a review? |
I'm seeing some weird behavior in my initial tests. I've tried downloading multiple models at the same time as well as interrupting the download before it completes and resuming after relaunching the app. I'm not always seeing the files I expect to see in the temporary download folder. I don't have time to test this in depth, so I'm going to leave it to @pcuenca and others. I'm personally happy with the solution that I offered. |
92303cb
to
4b6ebd4
Compare
Also, it looks like you're not cleaning up the parent directories of the temporary files, so you end up with a lot of empty directories in the |
This PR adds support for resumable downloads that persist across app sessions. When a download is interrupted, it will resume from where it left off when the app is restarted and the download is initiated. The implementation uses stable file hashing, UserDefaults for state persistence, and HTTP Range requests to efficiently download only the missing portions of files.