CWFHEALTH-5046: Add retry logic to MCP server registration#2163
CWFHEALTH-5046: Add retry logic to MCP server registration#2163willianrampazzo wants to merge 1 commit into
Conversation
Add retry on APIConnectionError to _register_mcp_toolgroups_async(), preventing lightspeed-stack from crashing when Llama Stack is briefly unreachable during MCP server registration at startup. Reuses the existing max_retries and retry_delay config from llama_stack settings, following the same pattern as check_llama_stack_version(). CWFHEALTH-5046: Add retry logic to MCP server registration Signed-off-by: Willian Rampazzo <willianr@redhat.com>
WalkthroughMCP toolgroup registration now accepts configurable retry settings in both client modes. The registration helper retries ChangesMCP registration retry flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Configuration
participant register_mcp_servers_async
participant _register_mcp_toolgroups_async
participant MCPToolgroupsAPI
Configuration->>register_mcp_servers_async: provide max_retries and retry_delay
register_mcp_servers_async->>_register_mcp_toolgroups_async: invoke with retry configuration
_register_mcp_toolgroups_async->>MCPToolgroupsAPI: list existing toolgroups
MCPToolgroupsAPI-->>_register_mcp_toolgroups_async: toolgroups or APIConnectionError
_register_mcp_toolgroups_async->>MCPToolgroupsAPI: register missing toolgroups
MCPToolgroupsAPI-->>_register_mcp_toolgroups_async: success or APIConnectionError
_register_mcp_toolgroups_async->>_register_mcp_toolgroups_async: sleep and retry after connection error
🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/common.py (1)
46-68: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsolidate the identical registration call to avoid duplication.
Since the registration step is identical for both the library and service clients, you can consolidate the
_register_mcp_toolgroups_asynccall after theif/elseblock to DRY up the code.♻️ Proposed refactor
if configuration.llama_stack.use_as_library_client: # Library client - use async interface client = cast( AsyncLlamaStackAsLibraryClient, AsyncLlamaStackClientHolder().get_client() ) await client.initialize() - await _register_mcp_toolgroups_async( - client, - configuration.mcp_servers, - logger, - max_retries=configuration.llama_stack.max_retries, - retry_delay=configuration.llama_stack.retry_delay, - ) else: # Service client - also use async interface client = AsyncLlamaStackClientHolder().get_client() - await _register_mcp_toolgroups_async( - client, - configuration.mcp_servers, - logger, - max_retries=configuration.llama_stack.max_retries, - retry_delay=configuration.llama_stack.retry_delay, - ) + + await _register_mcp_toolgroups_async( + client, + configuration.mcp_servers, + logger, + max_retries=configuration.llama_stack.max_retries, + retry_delay=configuration.llama_stack.retry_delay, + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/common.py` around lines 46 - 68, Consolidate the duplicated _register_mcp_toolgroups_async invocation in the client setup flow: keep client selection and library-only initialize() inside the if/else, then call _register_mcp_toolgroups_async once afterward using the selected client and existing configuration arguments.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/utils/common.py`:
- Around line 46-68: Consolidate the duplicated _register_mcp_toolgroups_async
invocation in the client setup flow: keep client selection and library-only
initialize() inside the if/else, then call _register_mcp_toolgroups_async once
afterward using the selected client and existing configuration arguments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ac427fe8-2424-4a9d-8ae3-ef40d1e3fada
📒 Files selected for processing (2)
src/utils/common.pytests/unit/utils/test_common.py
📜 Review details
⏰ Context from checks skipped due to timeout. (6)
- GitHub Check: Pylinter
- GitHub Check: integration_tests (3.13)
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: radon
- GitHub Check: Pyright
- GitHub Check: spectral
🧰 Additional context used
📓 Path-based instructions (6)
**/*
📄 CodeRabbit inference engine (Custom checks)
**/*: Flag meaningful O(n^2)+ algorithms on non-trivial inputs, including handlers and Kubernetes list operations.
Flag N+1 patterns that list items and then query once per item, including Kubernetes API and database access.
Flag expensive work inside loops, including API calls, JSON parsing, and regex compilation.
Flag unbounded growth in caches, watchers, or buffers when eviction or limits are missing.
Flag missing pagination or limits on list operations and API endpoints.
Flag secrets or tokens logged in plaintext or hardcoded in source.
Flag API endpoints missing authentication or authorization.
Flag injection vulnerabilities, including SQL injection, command injection, and path traversal.
Flag sensitive data leaked in API responses, WebSocket messages, or logs.
Flag Kubernetes Secrets and Red Hat secrets missing OwnerReferences.
Files:
src/utils/common.pytests/unit/utils/test_common.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use absolute imports for internal Python modules.
Start every module with a descriptive docstring and uselogger = get_logger(__name__)for module logging.
Define shared constants in the centralconstants.pymodule, use descriptive comments, and annotate constants withFinal[type].
Use complete type annotations for function parameters and return values, modern union syntax, andtyping_extensions.Selffor model validators.
Document all modules, classes, and functions using Google-style Python docstrings, including applicable Parameters, Returns, Raises, and Attributes sections.
Use descriptive snake_case, action-oriented function names such asget_,validate_, andcheck_.
Avoid modifying input parameters in place; return a newly created data structure instead.
Useasync deffor I/O operations and external API calls.
Use standard logger levels appropriately: debug for diagnostics, info for normal execution, warning for unexpected conditions, and error for serious failures.
Name classes with PascalCase and use descriptive suffixes such asConfiguration,Error/Exception,Resolver, andInterfacewhere applicable.
Use ABC and@abstractmethodfor abstract interfaces; provide complete, specific type annotations for class attributes and avoidAny.
Files:
src/utils/common.pytests/unit/utils/test_common.py
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Configuration models must extend
ConfigurationBase, whoseextra="forbid"behavior rejects unknown fields; use types such asOptional[FilePath],PositiveInt, andSecretStrwhere appropriate.
Files:
src/utils/common.py
**/*.{py,yaml,yml,json,toml}
📄 CodeRabbit inference engine (AGENTS.md)
Never commit secrets or keys; use environment variables for sensitive data.
Files:
src/utils/common.pytests/unit/utils/test_common.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use pytest for unit tests, shared fixtures in
conftest.py,pytest-mockfor mocks, andpytest.mark.asynciofor asynchronous tests; do not use unittest.
Files:
tests/unit/utils/test_common.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain at least 60% unit-test coverage and 10% integration-test coverage.
Files:
tests/unit/utils/test_common.py
🧠 Learnings (2)
📚 Learning: 2026-06-24T13:45:37.249Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 1971
File: src/utils/markdown_repair.py:31-36
Timestamp: 2026-06-24T13:45:37.249Z
Learning: In the lightspeed-stack repository, docstrings must use the section header name "Parameters:" (not "Args:") for function arguments, even if the project references Google Python docstring conventions. Ensure docstrings follow the project’s established "Parameters:" header format for any documented function parameters.
Applied to files:
src/utils/common.pytests/unit/utils/test_common.py
📚 Learning: 2026-07-06T15:26:18.398Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2071
File: src/models/config.py:2416-2422
Timestamp: 2026-07-06T15:26:18.398Z
Learning: In this repo’s Python code under src/**, don’t treat differences in string concatenation style as a style inconsistency when Black has effectively forced (or made clearer) use of explicit `+` string concatenation in multi-line logger/string expressions. If adjacent-literal implicit concatenation is avoided/changed specifically to accommodate Black’s formatting in these call sites, accept the `+` usage and don’t recommend converting it solely for consistency with nearby blocks that use implicit concatenation.
Applied to files:
src/utils/common.py
🔇 Additional comments (2)
src/utils/common.py (1)
10-13: LGTM!Also applies to: 87-89, 99-137
tests/unit/utils/test_common.py (1)
6-6: LGTM!Also applies to: 18-18, 419-612
willianrampazzo
changed the title
1 participant
Add retry on APIConnectionError to _register_mcp_toolgroups_async(), preventing lightspeed-stack from crashing when Llama Stack is briefly unreachable during MCP server registration at startup. Reuses the existing max_retries and retry_delay config from llama_stack settings, following the same pattern as check_llama_stack_version().
CWFHEALTH-5046: Add retry logic to MCP server registration
Description
Type of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit
Reliability
Tests