LCORE-285: Add LLM check in readines endpoint #136
Conversation
src/app/endpoints/health.py
Outdated
| return False | ||
|
|
||
| logger.debug("LLM connection verified - found %d LLM models", len(llm_models)) | ||
| return True |
There was a problem hiding this comment.
IMO this should call the LLM provider's client.providers.retrieve function to actually test the LLM.
This simply returns True if some LLM models have been registered; not that they actually work.
src/app/endpoints/health.py
Outdated
| return False | ||
|
|
||
|
|
||
| def index_is_ready() -> bool: |
There was a problem hiding this comment.
We do a similar check for (RAG) index too.
We, Ansible Lightspeed, were planning on contributing our code soon.
src/app/endpoints/health.py
Outdated
| get the list of available models from the configured LlamaStack client. | ||
| """ | ||
| try: | ||
| from client import get_llama_stack_client |
src/app/endpoints/health.py
Outdated
| client = get_llama_stack_client(llama_stack_config) | ||
|
|
||
| # Try to list models to verify LLM connection | ||
| models = client.models.list() |
There was a problem hiding this comment.
this just list the models, not doing any connection check. It works even when I turned off my WiFi (and the model is remote).
src/app/endpoints/health.py
Outdated
| logger.warning("No LLM models found in available models") | ||
| return False | ||
|
|
||
| logger.debug("LLM connection verified - found %d LLM models", len(llm_models)) |
There was a problem hiding this comment.
You haven't verified a connection only that there are more than zero LLMs registered.
src/app/endpoints/health.py
Outdated
| try: | ||
| llama_stack_config = configuration.llama_stack_configuration | ||
|
|
||
| # TODO: this seems to be an expensive operation (takes 1-2 seconds) |
| args = parser.parse_args() | ||
|
|
||
| configuration.load_configuration("lightspeed-stack.yaml") | ||
| configuration.load_configuration(args.config_file) |
There was a problem hiding this comment.
Yay. Finally I won't need to tweak the code 😘
There was a problem hiding this comment.
Oh, that should be a separate PR. Let me raise it.
There was a problem hiding this comment.
I already did :D. I'll just rebase here.
|
@tisnik fixed linting |
3 participants
Description
Add LLM check in readiness endpoint
Type of change
Related Tickets & Documents
Checklist before requesting a review
Testing