update check_ollama_model_exists using ollama.list api.#549
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the check_ollama_model_exists function in api/ollama_patch.py to use the official ollama Python client instead of making raw HTTP requests with requests. The review feedback highlights that the new exact-match logic breaks the original tag-agnostic model matching behavior, and that the exception handling catches ConnectionError instead of the httpx.RequestError and ollama.ResponseError exceptions raised by the underlying client. Suggestions have been provided to restore the tag-agnostic matching and to import httpx to properly handle connection and API errors.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| is_available = any(model_name == model.model for model in ret.models) | ||
| if is_available: | ||
| logger.info("Ollama model '%s' is available", model_name) | ||
| else: | ||
| logger.warning(f"Could not check Ollama models, status code: {response.status_code}") | ||
| return False | ||
| except requests.exceptions.RequestException as e: | ||
| logger.warning( | ||
| "Ollama model '%s' is not available. Available models: %s. ", | ||
| model_name, | ||
| str([model.model for model in ret.models])) |
There was a problem hiding this comment.
The new matching logic performs an exact match (model_name == model.model), which breaks the original tag-agnostic matching behavior. For example, if a user checks for llama3 but the installed model is llama3:latest, the check will incorrectly return False.\n\nTo preserve the original behavior, we should compare the base names (by splitting on :).
| is_available = any(model_name == model.model for model in ret.models) | |
| if is_available: | |
| logger.info("Ollama model '%s' is available", model_name) | |
| else: | |
| logger.warning(f"Could not check Ollama models, status code: {response.status_code}") | |
| return False | |
| except requests.exceptions.RequestException as e: | |
| logger.warning( | |
| "Ollama model '%s' is not available. Available models: %s. ", | |
| model_name, | |
| str([model.model for model in ret.models])) | |
| model_base = model_name.split(':')[0] | |
| available_models = [model.model for model in ret.models] | |
| is_available = any(model_base == m.split(':')[0] for m in available_models) | |
| if is_available: | |
| logger.info("Ollama model '%s' is available", model_name) | |
| else: | |
| logger.warning( | |
| "Ollama model '%s' is not available. Available models: %s.", | |
| model_name, | |
| str(available_models)) |
| except ConnectionError as e: | ||
| logger.warning(f"Could not connect to Ollama to check models: {e}") | ||
| return False |
There was a problem hiding this comment.
The ollama library uses httpx under the hood, which raises httpx.RequestError (not the built-in ConnectionError) for connection issues and timeouts. Additionally, API errors raise ollama.ResponseError. Catching ConnectionError will fail to intercept these exceptions, causing them to fall through to the generic except Exception block.\n\nWe should catch httpx.RequestError and ollama.ResponseError instead.
| except ConnectionError as e: | |
| logger.warning(f"Could not connect to Ollama to check models: {e}") | |
| return False | |
| except (httpx.RequestError, ollama.ResponseError) as e: | |
| logger.warning(f"Could not connect to Ollama to check models: {e}") | |
| return False |
There was a problem hiding this comment.
Since the timeout argument is set, the actual exception should be httpx.ConnectTimeout instead.
fixed in da71585
| Returns: | ||
| bool: True if model exists, False otherwise | ||
| """ | ||
| import ollama |
Summary
refactor
check_ollama_model_existsusing ollama providedlistapi.