From b6d15558ec8df6c15f25b1012e330802b4d1f830 Mon Sep 17 00:00:00 2001 From: Christopher Krah Date: Thu, 22 Oct 2026 21:18:52 +0200 Subject: [PATCH] fix(python): stop retrying a missing API key or provider package (#4521) `retry_with_exponential_backoff` retries every exception except an auth error (`python/python/lancedb/embeddings/utils.py:298-318`). For Cohere, Gemini, Jina and VoyageAI a missing API key raises from `api_key_not_found_help`, and a missing provider package raises `ImportError` everywhere. Neither can succeed on a retry, yet both sleep through all seven rounds and then surface wrapped in `Exception("Maximum number of retries...")`. Before, on `142d032`, a VoyageAI function with no key set (script below, `time.sleep` recorded, `random.seed(0)`): ``` $ python lance_retry_deterministic.py python/python raised: Exception('Maximum number of retries (7) exceeded.', ValueError('Please set the VOYAGEAI_API_KEY environment v attempts: 8 sleeps: [4, 13, 37, 93, 280, 788, 2811] slept 4026 s before a missing-API-key error surfaced ``` After: ``` $ python lance_retry_deterministic.py python/python raised: MissingApiKeyError('Please set the VOYAGEAI_API_KEY environment variable.') attempts: 1 sleeps: [] ```
lance_retry_deterministic.py ```python # loads the pure-Python embeddings package from the tree, native extension stubbed import os, random, sys, time, types TREE = sys.argv[1] pkg = types.ModuleType("lancedb"); pkg.__path__ = [TREE + "/lancedb"]; sys.modules["lancedb"] = pkg native = types.ModuleType("lancedb._lancedb"); native.__getattr__ = lambda name: None sys.modules["lancedb._lancedb"] = native import pyarrow as pa from lancedb.embeddings import get_registry random.seed(0) os.environ.pop("VOYAGE_API_KEY", None) slept = [] time.sleep = slept.append func = get_registry().get("voyageai").create(name="voyage-3") try: func.compute_source_embeddings_with_retry(pa.array(["hello"])) except Exception as e: print("raised:", repr(e)[:110]) print("attempts:", len(slept) + 1, "sleeps:", [round(s) for s in slept]) print("slept %.0f s before a missing-API-key error surfaced" % sum(slept)) ```
The message names the wrong variable: `voyageai.py:501` reads `VOYAGE_API_KEY`. I left that for a separate fix. `api_key_not_found_help` now raises `MissingApiKeyError`, a `ValueError` subclass, so existing `except ValueError` callers still catch it. That error and `ImportError` join the 401 exemption from #2995. A plain `ValueError` is still retried, because `bedrock.py:171` wraps boto3 throttling in one. No issue exists for this. OpenAI is not covered: with no key, `openai.OpenAI()` raises `OpenAIError`, which has no status code, so it still retries. I can follow up on that. ### Verification ``` # lance_stub: an empty stand-in for lancedb._lancedb, loaded from the source tree $ pytest -p lance_stub --noconftest python/tests/test_embeddings.py -k "retry or 401 or deterministic" -q 5 passed, 23 deselected in 0.59s # utils.py from 142d032, tests kept: 2 failed, 3 passed, 23 deselected in 0.74s $ pytest -p lance_stub --noconftest python/tests/test_voyageai_embeddings.py -q 18 passed in 0.18s # not run: the full suite or pyright; the native extension did not build here ``` Baseline on `142d032` has 2 of these tests. The new ones cover a missing key, a missing package, and a wrapped throttling error that must still retry. ruff 0.16.10 check and format are clean. @wjones127 you merged the 401 change. Is a `ValueError` subclass the way you want to mark this? Co-authored-by: Xuanwo --- python/python/lancedb/embeddings/utils.py | 13 ++++++- python/python/tests/test_embeddings.py | 45 +++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/python/python/lancedb/embeddings/utils.py b/python/python/lancedb/embeddings/utils.py index f76c9b1a5..fb2a94ecb 100644 --- a/python/python/lancedb/embeddings/utils.py +++ b/python/python/lancedb/embeddings/utils.py @@ -320,6 +320,11 @@ def _is_non_retryable_error(error: Exception) -> bool: if error.status_code in (401, 403): return True + # A missing API key or a missing optional dependency fails the same way on + # every attempt + if isinstance(error, (MissingApiKeyError, ImportError)): + return True + return False @@ -337,9 +342,15 @@ def url_retrieve(url: str): raise ConnectionError("could not download {} due to {}".format(url, err)) +class MissingApiKeyError(ValueError): + """Raised when an embedding provider's API key is not set.""" + + def api_key_not_found_help(provider): logging.error("Could not find API key for %s", provider) - raise ValueError(f"Please set the {provider.upper()}_API_KEY environment variable.") + raise MissingApiKeyError( + f"Please set the {provider.upper()}_API_KEY environment variable." + ) def is_flash_attn_2_available(): diff --git a/python/python/tests/test_embeddings.py b/python/python/tests/test_embeddings.py index d6d128845..2438bfe80 100644 --- a/python/python/tests/test_embeddings.py +++ b/python/python/tests/test_embeddings.py @@ -633,6 +633,51 @@ def test_openai_no_retry_on_401(mock_sleep): assert mock_sleep.call_count == 0 +@patch("time.sleep") +@pytest.mark.parametrize( + "error", + [ + pytest.param("missing_key", id="missing-api-key"), + pytest.param(ImportError("No module named 'voyageai'"), id="missing-package"), + ], +) +def test_no_retry_on_deterministic_errors(mock_sleep, error): + from lancedb.embeddings.utils import ( + api_key_not_found_help, + retry_with_exponential_backoff, + ) + + def func(): + if error == "missing_key": + api_key_not_found_help("voyageai") + raise error + + mock_func = MagicMock(side_effect=func) + wrapped_func = retry_with_exponential_backoff(mock_func, max_retries=3) + + with pytest.raises((ValueError, ImportError)): + wrapped_func() + + assert mock_func.call_count == 1 + assert mock_sleep.call_count == 0 + + +@patch("time.sleep") +def test_retry_on_wrapped_provider_error(mock_sleep): + """Providers such as Bedrock wrap throttling in a plain ValueError.""" + from lancedb.embeddings.utils import retry_with_exponential_backoff + + mock_func = MagicMock( + side_effect=[ + ValueError("Error raised by boto3 client: ThrottlingException"), + [0.1], + ] + ) + wrapped_func = retry_with_exponential_backoff(mock_func, max_retries=3) + + assert wrapped_func() == [0.1] + assert mock_func.call_count == 2 + assert mock_sleep.call_count == 1 def test_openai_bad_request_partial_batch_recovery(): # Regression test for issue #1677