moxin-org / moxin

Moxin: an AI LLM platform in pure Rust
https://www.moxin.app
Apache License 2.0
122 stars 17 forks source link

feat: Change download location #112

Closed ZhangHanDong closed 1 month ago

ZhangHanDong commented 1 month ago

Currently only tested on macOS, not yet tested on Linux.

jmbejar commented 1 month ago

Thanks for the PR. The overall approach looks good to me and it works, at least for the frontend part of the application.

The problem we have now is that needs a full application restart to have effect. I tested changing the downloads folder but all downloads started before restarting the application will still use the previous folder.

Here is how we pass the downloads folder to our Backend struct (placed in the moxin-backend crate): https://github.com/moxin-org/moxin/blob/main/src/data/store.rs#L68

One simply solution is to add a setter function to this Backend to modify this piece of data. We should test the case where downloads are happening at the time the folder change is made, but I'm pretty sure the current logic won't be impacted by this change.

Let me know if this backend aspect is clear enough or if we should help to wrap up the PR in a anyway.

kevinaboos commented 1 month ago

In my view, the simple solution of adding an on_completed() callback fn to the download metadata struct is the right option. That way, the downloaded file can move itself to the newly-chosen Model Downloads directory after the download has completed.

ZhangHanDong commented 1 month ago

@jmbejar @kevinaboos

My solution is: After manually updating the download directory, send a notification to the backend main loop to automatically update the download directory. This way, it is not necessary to restart the application.

However, there is an issue: if a model is currently being downloaded, it will continue to download to the previous directory after the update. Only new downloads will use the new directory. At least this won’t affect the user experience.

jmbejar commented 1 month ago

Great progress so far!

However, I was testing and I found we have issues in the case you're downloading a file at the time of changing the models' folder. The issue actually happens if you pause/resume a download initiated before changing folder. The download will resume in the new location, but for some reason the application still points to the older folder when the model is completed and you want to use it (resulting in an error).

I'm digging into this issue right now, so I will let you what would be a good fix for this.

jmbejar commented 1 month ago

I'm digging into this issue right now, so I will let you what would be a good fix for this.

@ZhangHanDong Based in your last message in Discord, I assume you're around it. So ignore my last sentence :) Let me know if you need help.

jmbejar commented 1 month ago

I'm digging into this issue right now, so I will let you what would be a good fix for this.

@ZhangHanDong Based in your last message in Discord, I assume you're around it. So ignore my last sentence :) Let me know if you need help.

In case it helps.. this change seems to work well enough.

--- a/moxin-backend/src/store/download_files.rs
+++ b/moxin-backend/src/store/download_files.rs
@@ -24,7 +24,7 @@ pub struct DownloadedFile {
 impl DownloadedFile {
     pub fn insert_into_db(&self, conn: &rusqlite::Connection) -> rusqlite::Result<()> {
         conn.execute(
-            "INSERT OR IGNORE INTO download_files (
+            "INSERT OR REPLACE INTO download_files (
                 id, model_id, name, size, quantization,
                 prompt_template, reverse_prompt,