LCORE-1858: degraded mode support#1781
Conversation
|
Warning Review limit reached
Your plan currently allows 1 review/hour. Refill in 49 minutes and 52 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR refactors Llama Stack version checking to accept configurable retry parameters and return the detected version, then integrates the updated logic into FastAPI startup with conditional degraded-mode fallback on connection failure. ChangesLlama Stack Connectivity on Startup
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
tisnik
force-pushed
the
lcore-1858-degraded-mode
branch
from
519fd44 to
58e50c3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@src/utils/llama_stack_version.py`:
- Around line 25-29: The docstring for check_llama_stack_version is missing a
Returns section; update the function's docstring (for check_llama_stack_version)
to include a Returns entry that states it returns an Optional[str] — the
detected Llama Stack version string on success or None if the version could not
be determined after retries — and briefly mention any error behavior if
applicable (e.g., that it does not raise on failure but returns None).
- Line 5: The retry helper get_llama_stack_version currently re-raises
APIConnectionError on final failure which makes the later "return None" branch
unreachable and conflicts with callers expecting Optional[str] (e.g.,
src/app/main.py); update get_llama_stack_version to honor the Optional[str]
contract by catching APIConnectionError on the last retry and returning None
instead of re-raising (or alternatively change the function signature to return
str and propagate the exception consistently); ensure you adjust the function's
return type annotation (Optional[str]) and remove unreachable code paths so the
caller's "version is None" branch can run.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 62055541-bba3-41ff-9b1b-e5382bba2291
📒 Files selected for processing (2)
src/app/main.pysrc/utils/llama_stack_version.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: pydocstyle
- GitHub Check: bandit
- GitHub Check: spectral
- GitHub Check: unit_tests (3.13)
- GitHub Check: unit_tests (3.12)
- GitHub Check: build-pr
- GitHub Check: integration_tests (3.13)
- GitHub Check: Pylinter
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E Tests for Lightspeed Evaluation job
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 2
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Use absolute imports for internal modules:from authentication import get_auth_dependency
Llama Stack imports: Usefrom llama_stack_client import AsyncLlamaStackClient
Checkconstants.pyfor shared constants before defining new ones
All modules must start with descriptive docstrings explaining purpose
Uselogger = get_logger(__name__)fromlog.pyfor module logging
All functions must have complete type annotations for parameters and return types, use modern syntax (str | int), and include descriptive docstrings
Use snake_case with descriptive, action-oriented names for functions (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns; return new data structures instead of modifying function parameters
Useasync deffor I/O operations and external API calls
Use standard log levels with clear purposes:debug()for diagnostic info,info()for program execution,warning()for unexpected events,error()for serious problems
All classes must have descriptive docstrings explaining purpose and use PascalCase with standard suffixes:Configuration,Error/Exception,Resolver,Interface
Abstract classes must use ABC with@abstractmethoddecorators
Follow Google Python docstring conventions with required sections: Parameters, Returns, Raises, and Attributes for classes
Files:
src/utils/llama_stack_version.pysrc/app/main.py
src/app/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/app/**/*.py: FastAPI dependencies: Import fromfastapimodule forAPIRouter,HTTPException,Request,status,Depends
Use FastAPIHTTPExceptionwith appropriate status codes for API endpoints and handleAPIConnectionErrorfrom Llama Stack
Files:
src/app/main.py
🔇 Additional comments (1)
src/app/main.py (1)
102-116: LGTM!
| async def check_llama_stack_version( | ||
| client: AsyncLlamaStackClient, | ||
| max_retries: int = _DEFAULT_MAX_RETRIES, | ||
| retry_delay: int = _DEFAULT_RETRY_DELAY, | ||
| ) -> None: | ||
| max_retries: int = DEFAULT_MAX_RETRIES, | ||
| retry_delay: int = DEFAULT_RETRY_DELAY, | ||
| ) -> Optional[str]: |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Document the new return value in the docstring.
This function now returns the detected version, but the docstring still has no Returns section, so the in-file contract is incomplete.
As per coding guidelines, "Follow Google Python docstring conventions with required sections: Parameters, Returns, Raises, and Attributes for classes".
🤖 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/llama_stack_version.py` around lines 25 - 29, The docstring for
check_llama_stack_version is missing a Returns section; update the function's
docstring (for check_llama_stack_version) to include a Returns entry that states
it returns an Optional[str] — the detected Llama Stack version string on success
or None if the version could not be determined after retries — and briefly
mention any error behavior if applicable (e.g., that it does not raise on
failure but returns None).
1 participant
Description
LCORE-1858: degraded mode support
Type of change
Tools used to create PR
Related Tickets & Documents
Summary by CodeRabbit
New Features
Bug Fixes