_repair_model_cache attempted snapshot_download exactly once; a single transient failure (the very cause of an interrupted download) returned False and sent the user back to a manual delete-and-reinstall. Wrap the re-fetch in a bounded retry loop (3 attempts default, linear backoff) — snapshot_download resumes between attempts so retries are cheap and idempotent. Counts/backoff are env-tunable (OMNIVOICE_MODEL_REPAIR_RETRIES / _BACKOFF_S) for restricted networks and set to zero-backoff in tests. Offline mode + the actionable fallback message are unchanged. Tests: retry-then-succeed self-heals, exhausted-retries returns False after N attempts, single-attempt tunable, backoff disabled so the suite stays fast. Co-authored-by: mergetest <test@local> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
mergetest
Claude Opus 4.8
parent
de3d83f14b
commit
9fc18dbd89
+4
-1
@@ -322,7 +322,10 @@ across dub, generate, and design (a corrupt-binary failure no longer poses as
|
||||
and retries the load automatically. Offline mode (`HF_HUB_OFFLINE`) is
|
||||
respected — repair never makes a network call the user opted out of — and if
|
||||
the re-fetch still can't fix it, the actionable delete-and-reinstall message
|
||||
is preserved as the fallback. (#581)
|
||||
is preserved as the fallback. (#581) The repair now also **retries** the
|
||||
re-fetch (3 attempts, resuming each time) so a single transient blip — the very
|
||||
thing that interrupts a download in the first place — doesn't bounce you back
|
||||
to a manual reinstall; tune with `OMNIVOICE_MODEL_REPAIR_RETRIES`. (#739)
|
||||
- **Dubbing a YouTube URL no longer dies on a transient "Broken pipe."**
|
||||
Pasting a video link could fail outright with `download: Unable to download
|
||||
video: [Errno 32] Broken pipe` — a broken pipe raised while the write side of
|
||||
|
||||
@@ -578,7 +578,6 @@ def _repair_model_cache(checkpoint: str) -> bool:
|
||||
except Exception as imp_err: # pragma: no cover - huggingface_hub is a hard dep
|
||||
logger.warning("Cannot import snapshot_download to repair cache: %s", imp_err)
|
||||
return False
|
||||
logger.info("Auto-repairing incomplete model cache for %s …", checkpoint)
|
||||
dl_kwargs: dict = {"repo_id": checkpoint}
|
||||
endpoint = os.environ.get("HF_ENDPOINT")
|
||||
if endpoint:
|
||||
@@ -586,22 +585,50 @@ def _repair_model_cache(checkpoint: str) -> bool:
|
||||
if os.name == "nt":
|
||||
# Match the install path (download.py): avoid symlinks on Windows.
|
||||
dl_kwargs["local_dir_use_symlinks"] = False
|
||||
try:
|
||||
snapshot_download(**dl_kwargs)
|
||||
except TypeError:
|
||||
# Older/newer huggingface_hub may not accept local_dir_use_symlinks
|
||||
# on a cache-only call — retry without the optional knob.
|
||||
dl_kwargs.pop("local_dir_use_symlinks", None)
|
||||
|
||||
def _attempt() -> None:
|
||||
"""One snapshot_download, tolerating an hf_hub that rejects the optional
|
||||
symlink knob. Lets real failures (network, gated repo, disk) propagate."""
|
||||
try:
|
||||
snapshot_download(**dl_kwargs)
|
||||
except TypeError:
|
||||
# Older/newer huggingface_hub may not accept local_dir_use_symlinks
|
||||
# on a cache-only call — retry without the optional knob.
|
||||
dl_kwargs.pop("local_dir_use_symlinks", None)
|
||||
snapshot_download(**dl_kwargs)
|
||||
|
||||
# Bounded retries (#739): an incomplete cache *is* an interrupted download, so
|
||||
# a single transient blip mid-repair shouldn't drop the user back to a manual
|
||||
# delete-and-reinstall. snapshot_download resumes between attempts (present,
|
||||
# correctly-sized blobs are skipped by hash), so each retry continues where
|
||||
# the last left off — cheap and idempotent. Counts/backoff are env-tunable
|
||||
# for restricted networks and kept fast (backoff=0) in tests.
|
||||
try:
|
||||
retries = max(1, int(os.environ.get("OMNIVOICE_MODEL_REPAIR_RETRIES", "3")))
|
||||
except ValueError:
|
||||
retries = 3
|
||||
try:
|
||||
backoff = max(0.0, float(os.environ.get("OMNIVOICE_MODEL_REPAIR_BACKOFF_S", "2")))
|
||||
except ValueError:
|
||||
backoff = 2.0
|
||||
|
||||
logger.info(
|
||||
"Auto-repairing incomplete model cache for %s (up to %d attempt(s)) …",
|
||||
checkpoint, retries,
|
||||
)
|
||||
for attempt in range(1, retries + 1):
|
||||
try:
|
||||
_attempt()
|
||||
logger.info("Auto-repair of %s completed; retrying model load.", checkpoint)
|
||||
return True
|
||||
except Exception as e:
|
||||
logger.warning("Auto-repair of %s failed: %s", checkpoint, e)
|
||||
return False
|
||||
except Exception as e:
|
||||
logger.warning("Auto-repair of %s failed: %s", checkpoint, e)
|
||||
return False
|
||||
logger.info("Auto-repair of %s completed; retrying model load.", checkpoint)
|
||||
return True
|
||||
logger.warning(
|
||||
"Auto-repair of %s attempt %d/%d failed: %s",
|
||||
checkpoint, attempt, retries, e,
|
||||
)
|
||||
if attempt < retries and backoff:
|
||||
time.sleep(backoff * attempt)
|
||||
return False
|
||||
|
||||
|
||||
_DEFAULT_OMNIVOICE_CHECKPOINT = "k2-fsa/OmniVoice"
|
||||
|
||||
@@ -142,8 +142,52 @@ def test_repair_returns_false_when_download_fails(model_manager, monkeypatch):
|
||||
"""A failed re-fetch (no network, gated repo) returns False, never raises."""
|
||||
import huggingface_hub
|
||||
|
||||
calls = []
|
||||
|
||||
def boom(**kwargs):
|
||||
calls.append(kwargs)
|
||||
raise OSError("network down")
|
||||
|
||||
monkeypatch.setattr(huggingface_hub, "snapshot_download", boom)
|
||||
monkeypatch.setenv("OMNIVOICE_MODEL_REPAIR_BACKOFF_S", "0") # no real sleeps
|
||||
monkeypatch.setenv("OMNIVOICE_MODEL_REPAIR_RETRIES", "3")
|
||||
assert model_manager._repair_model_cache("test/checkpoint") is False
|
||||
# #739: a transient failure must be retried, not given up on after one try.
|
||||
assert len(calls) == 3
|
||||
|
||||
|
||||
def test_repair_retries_then_succeeds(model_manager, monkeypatch):
|
||||
"""#739: a flaky connection that drops twice then completes must self-heal —
|
||||
the repair retries snapshot_download and returns True, so the user is never
|
||||
sent to a manual delete-and-reinstall for a transient blip."""
|
||||
import huggingface_hub
|
||||
|
||||
attempts = {"n": 0}
|
||||
|
||||
def flaky(**kwargs):
|
||||
attempts["n"] += 1
|
||||
if attempts["n"] < 3:
|
||||
raise OSError("connection reset")
|
||||
return "/cache/test/checkpoint"
|
||||
|
||||
monkeypatch.setattr(huggingface_hub, "snapshot_download", flaky)
|
||||
monkeypatch.setenv("OMNIVOICE_MODEL_REPAIR_BACKOFF_S", "0")
|
||||
monkeypatch.setenv("OMNIVOICE_MODEL_REPAIR_RETRIES", "3")
|
||||
assert model_manager._repair_model_cache("test/checkpoint") is True
|
||||
assert attempts["n"] == 3
|
||||
|
||||
|
||||
def test_repair_retries_are_env_tunable(model_manager, monkeypatch):
|
||||
"""A restricted network can lower/raise the attempt count; a single attempt
|
||||
must still work (no off-by-one that skips the only try)."""
|
||||
import huggingface_hub
|
||||
|
||||
calls = []
|
||||
monkeypatch.setattr(
|
||||
huggingface_hub, "snapshot_download",
|
||||
lambda **k: calls.append(k) or (_ for _ in ()).throw(OSError("down")),
|
||||
)
|
||||
monkeypatch.setenv("OMNIVOICE_MODEL_REPAIR_BACKOFF_S", "0")
|
||||
monkeypatch.setenv("OMNIVOICE_MODEL_REPAIR_RETRIES", "1")
|
||||
assert model_manager._repair_model_cache("test/checkpoint") is False
|
||||
assert len(calls) == 1
|
||||
|
||||
Reference in New Issue
Block a user