fix(llm): replace retry with tenacity in ollama.py#367
Conversation
imbajin
left a comment
There was a problem hiding this comment.
Blocking: yes. Summary: The Ollama retry policy changes need deterministic regression coverage. Evidence: uv run ruff check . passed; existing Ollama tests are external-service tests and do not cover the new Tenacity retry predicate.
| self.async_client = ollama.AsyncClient(host=f"http://{host}:{port}", **kwargs) | ||
|
|
||
| @retry(tries=3, delay=1) | ||
| @retry( |
There was a problem hiding this comment.
This changes both the retry library and the retry predicate/delay for generate() and agenerate(), but the PR does not add a mock-based test for retryable Ollama/httpx failures or for non-retryable errors. The module guidance asks code changes to cover changed behavior; please add a deterministic unit test that stubs client.chat / async_client.chat so CI proves the new Tenacity policy is wired correctly without requiring an Ollama service.
There was a problem hiding this comment.
second commit adds TestOllamaClientRetryPolicy: mock-based, no live service, covers retryable/non-retryable exceptions and transient recovery for both generate() and agenerate().
imbajin
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: No obvious issues found in the current head. Evidence: git diff --check passed; Ollama retry policy tests passed (9 passed, 2 skipped); ruff check and format passed for touched files.
## Summary `ollama.py` was using the abandoned `retry` PyPI package (last maintained 2016) while `openai.py` and `litellm.py` in the same directory already use `tenacity`. This PR brings `ollama.py` in line with the established patterns. Closes apache#365
Summary
ollama.pywas using the abandonedretryPyPI package (last maintained 2016)while
openai.pyandlitellm.pyin the same directory already usetenacity.This PR brings
ollama.pyin line with the established patterns.Changes
hugegraph-llm/src/hugegraph_llm/models/llms/ollama.pyfrom retry import retrywithtenacityimports@retry(tries=3, delay=1)withstop_after_attempt(3),wait_exponential(min=4, max=10), andretry_if_exception_type((ollama.ResponseError, httpx.ConnectError, httpx.TimeoutException))— matching the pattern in
openai.pyexcept Exceptionto the specific retriable exception typesprint(f"Retrying LLM call {e}")withlog.error(...)(consistentwith
openai.py;logwas already imported and used in the same file)agenerate_streaming,which has no
@retrydecorator"""Comment"""placeholder docstrings with real descriptionshugegraph-llm/pyproject.toml"retry", add"tenacity"as an explicit dependencypyproject.toml"retry~=0.9.2"constraint with"tenacity~=8.5.0"Closes #365