Skip to content

Use custom cache over FSCache if specified #1285

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

Merged
merged 3 commits into from
Apr 16, 2025
Merged

Conversation

PrafulB
Copy link
Contributor

@PrafulB PrafulB commented Apr 15, 2025

Currently, if both useFSCache and useCustomCache env options are set to true (perhaps because useFSCache is true by default and the user forgot to unset it), the code defaults to instantiating a FileCache instead of using the custom cache instance passed in by the user when getting model files. This commit changes the default to be the custom cache if specified.

Currently, if both `useFSCache` and `useCustomCache` env options are set to true (perhaps because `useFSCache` is true by default and the user forgot to unset it), the code defaults to instantiating a FileCache instead of using the custom cache implementation passed in by the user. This commit changes the default to be the custom cache if specified.
@xenova
Copy link
Collaborator

xenova commented Apr 15, 2025

Thanks! Should we maybe take this one step further and prefer this over browser cache too? (otherwise, similarly, the user would need to disable browser cache before enabling custom cache)

@PrafulB
Copy link
Contributor Author

PrafulB commented Apr 15, 2025

Of course! Totally missed that. 👍🏼 👍🏼

PrafulB added 2 commits April 14, 2025 23:31
If `useCustomCache` is set, try to use the custom cache before falling back to the browser or the fs caches.
Meant to handle cases where the user sets `useFSCache` from an environment that doesn't support `fs`.
@PrafulB
Copy link
Contributor Author

PrafulB commented Apr 15, 2025

Possibly outside the scope of this PR, but saw a TODO in the same place about throwing an error on fs cache being unavailable. Added a simple check for fs in my last commit. Seems okay from what I can see, but can revert if not needed right now.

Copy link
Collaborator

@xenova xenova left a comment

Choose a reason for hiding this comment

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

Thanks again!

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@xenova xenova merged commit 28ca8a8 into huggingface:main Apr 16, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants