Skip to content

Conversation

louis-menlo
Copy link
Contributor

@louis-menlo louis-menlo commented Jul 21, 2025

Describe Your Changes

This PR is to address #5793, where cancelled download models should delete tmp files. Otherwise users will get confused when so many model folders left on their disk.

CleanShot.2025-07-22.at.02.02.02.mp4

Upcoming enhancement:

  • FE can have an option to make it resumable (there is a flag to do so)

Fixes Issues

Self Checklist

  • Added relevant comments, esp in complex areas
  • Updated docs (for bug fixes / features)
  • Created issues for follow-up changes or refactoring needed

Important

Temporary files are now deleted when a download is canceled and not resumable in download_files() and _download_files_internal().

  • Behavior:
    • In download_files() and _download_files_internal(), temporary files are deleted if a download is canceled and not resumable.
    • Adds a resume parameter to _download_files_internal() to control resumable downloads.
  • Functions:
    • Modifies _download_files_internal() to check should_resume before deleting temporary files.
    • Updates download_files() to pass false for resume parameter.
  • Misc:
    • Adds a TODO comment for future resumable download support.

This description was created by Ellipsis for 19807b9. You can customize this summary. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Changes requested ❌

Reviewed everything up to 2e3a8c8 in 2 minutes and 36 seconds. Click for details.
  • Reviewed 82 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src-tauri/src/core/utils/download.rs:259
  • Draft comment:
    The addition of the 'resume' parameter and renaming to 'should_resume' is clear, but note that it’s currently hardcoded as false. Ensure cleanup logic is revisited when resumable downloads are enabled.
  • Reason this comment was not posted:
    Confidence changes required: 20% <= threshold 50% None

Workflow ID: wflow_xrZadbNaiwwru51V

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link
Contributor

github-actions bot commented Jul 21, 2025

Barecheck - Code coverage report

Total: 36.94%

Your code coverage diff: -0.01% ▾

✅ All code changes are covered

@louis-menlo louis-menlo force-pushed the fix/tmp-file-should-be-removed-on-cancel-download branch from 1cc4b76 to 19807b9 Compare July 23, 2025 01:36
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Changes requested ❌

Reviewed 19807b9 in 2 minutes and 21 seconds. Click for details.
  • Reviewed 82 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

Workflow ID: wflow_ZPEqrCggeFoIGk0f

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@louis-menlo louis-menlo merged commit 3afdd0f into release/v0.6.6 Jul 23, 2025
92 of 102 checks passed
@louis-menlo louis-menlo deleted the fix/tmp-file-should-be-removed-on-cancel-download branch July 23, 2025 05:52
@github-project-automation github-project-automation bot moved this to QA in Jan Jul 23, 2025
@github-actions github-actions bot added this to the v0.6.6 milestone Jul 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

feat: Add function to remove unfinished downloaded model (aka cancel download)
2 participants