Skip to content
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

fix: 4096 - failed to get huggingface models #4133

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

louis-jan
Copy link
Contributor

@louis-jan louis-jan commented Nov 26, 2024

Describe Your Changes

  • Fix a bad implementation that fetches the Huggingface repository N + 1 times, where N = repo file list sizes. It’s easy to reach the limit rate issue by doing that, where it should make only one request to get all the file sizes information. Also, waiting N requests would cause a bad UX where users have to wait for a long time, it hits all the model download endpoints.

Results

  • Fast model repo information display, consistent from repo to repo
    CleanShot 2024-11-27 at 00 09 52

  • Only 1 request sent to get all file sizes
    CleanShot 2024-11-27 at 00 10 08

Fixes Issues

Changes made

The code changes involve the removal and refactoring of the getFileSize function across the codebase:

  1. Tests:

    • Removed the getFileSize unit test from core.test.ts and download.test.ts.
  2. Core Functionality:

    • Deleted the getFileSize function definition from core.ts and its export statement. The associated API endpoint reference in the DownloadRoute enum has also been removed.
  3. Downloader Class:

    • The getFileSize method in download.ts has been removed. Previously, it used the request library to fetch file sizes using HTTP HEAD requests.
  4. Hugging Face Integration:

    • The import of getFileSize in huggingface.ts and its related mock in huggingface.test.ts have been removed.
    • File size determination now uses the Hugging Face repository's API through an additional fetch call to retrieve the file tree, including sizes, instead of individually calling getFileSize.
  5. UI Messaging:

    • Updated the toaster message for failing to get Hugging Face models in ModelSearch/index.tsx, providing more user-friendly feedback regarding potential rate limiting.

Overall, the refactor centralizes file size retrieval to a single network call using existing repository data, reducing reliance on redundant and potentially heavy individual requests.

Copy link
Contributor

Barecheck - Code coverage report

Total: 69.14%

Your code coverage diff: -0.04% ▾

Uncovered files and lines
FileLines
core/src/browser/core.ts29, 104, 115, 122, 131
core/src/node/api/processors/download.ts18-20, 67, 72-73, 79-81, 96-97, 103-104, 109-110
web/utils/huggingface.ts23, 31, 92, 105

@louis-jan louis-jan merged commit 3a9a8da into dev Nov 26, 2024
22 of 31 checks passed
@louis-jan louis-jan deleted the fix/4096-failed-to-get-huggingface-models branch November 26, 2024 17:53
@github-actions github-actions bot added this to the v0.5.10 milestone Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants