Skip to content

ruby : ignore "Downloading" output in test_log_suppress #3106

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 1 commit into from
May 1, 2025

Conversation

danbev
Copy link
Collaborator

@danbev danbev commented May 1, 2025

This commit adds a temporary fix to the test_log_suppress test in the Ruby bindings.

The motivation for this changes is that I suspect that the recent migration of the models to HuggingFace Xet has changed the way HTTP caching works for the models. This is causing the test in question to fail. This is a temporary fix so that CI is not broken while we investigate this further.


This is a temporary fix as I'm out today (public holiday) but I'll take a closer look at this tomorrow.

Example of failure:
https://github.com/ggml-org/whisper.cpp/actions/runs/14761084015/job/41466717129?pr=3103

This commit adds a temporary fix to the `test_log_suppress` test in the
Ruby bindings.

The motivation for this changes is that I suspect that the recent
migration of the models to HuggingFace Xet has changed the way HTTP
caching works for the models. This is causing the test in question to
fail. This is a temporary fix so that CI is not broken while we
investigate this further.
@danbev danbev requested a review from ggerganov May 1, 2025 04:54
@KitaitiMakoto
Copy link
Collaborator

KitaitiMakoto commented May 1, 2025

By quick and rough investigation, I guess this is caused because Hugging Face has become not to support HTTP if-modified-since field. Ruby bindings downloads model file and output "Downloading" and dots(".") for every access to the model when the response is not 304 Not Modified.

If the end of support for if-modified-since is permanent, we need stop the check via HTTP and just check if cache of model file.

@KitaitiMakoto
Copy link
Collaborator

KitaitiMakoto commented May 1, 2025

@danbev
This should fix the problem temporarily.

diff --git a/bindings/ruby/lib/whisper/model/uri.rb b/bindings/ruby/lib/whisper/model/uri.rb
index 47c23c52..5ffb07ef 100644
--- a/bindings/ruby/lib/whisper/model/uri.rb
+++ b/bindings/ruby/lib/whisper/model/uri.rb
@@ -41,6 +41,12 @@ module Whisper
 
       def cache
         path = cache_path
+
+        # 2025-05
+        # Hugging Face seems to stop support for if-modified-since
+        # So we cannot use it to check if cache file is fresh or not
+        return path if path.exist?
+
         headers = {}
         headers["if-modified-since"] = path.mtime.httpdate if path.exist?
         request @uri, headers

@KitaitiMakoto
Copy link
Collaborator

This seems purely Ruby-related problem. I'll take over investigation and fixing. Merge your temporary fix and enjoy you holiday!

@ggerganov
Copy link
Member

Ok, merging this for now and feel free to open an alternative fix if needed.

@ggerganov ggerganov merged commit edbd4cb into ggml-org:master May 1, 2025
52 checks passed
KitaitiMakoto added a commit to KitaitiMakoto/whisper.cpp that referenced this pull request May 1, 2025
KitaitiMakoto added a commit that referenced this pull request May 1, 2025
* Use cache file when model host doesn't support if-modified-since

* Update gem date

* Revert "ruby : ignore "Downloading" output in test_log_suppress (#3106)"

This reverts commit edbd4cb.
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