Skip to content

LCORE-1142: Docstrings for endpoint handlers#1033

Merged
tisnik merged 3 commits intolightspeed-core:mainfrom
tisnik:docstrings-for-endpoint-handlers
Jan 21, 2026
Merged

LCORE-1142: Docstrings for endpoint handlers#1033
tisnik merged 3 commits intolightspeed-core:mainfrom
tisnik:docstrings-for-endpoint-handlers

Conversation

@tisnik
Copy link
Contributor

@tisnik tisnik commented Jan 21, 2026

Description

LCORE-1142: Docstrings for endpoint handlers

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement

Tools used to create PR

  • Assisted-by: N/A
  • Generated by: CodeRabbitAI

Related Tickets & Documents

  • Related Issue #LCORE-1142

Summary by CodeRabbit

Documentation

  • Updated API endpoint documentation to clarify response behavior and error handling.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 21, 2026

Walkthrough

Three endpoint handler files receive docstring updates documenting response behavior and error conditions. The /authorized endpoint's response omission of authentication tokens, the /config endpoint's configuration loading process, and the /info endpoint's possible HTTP error outcomes are now documented. No functional or control flow changes are introduced.

Changes

Cohort / File(s) Summary
Endpoint Handler Documentation Updates
src/app/endpoints/authorized.py, src/app/endpoints/config.py, src/app/endpoints/info.py
Docstring additions clarifying endpoint behaviors: /authorized response omits authentication tokens; /config loads configuration before returning; /info details possible HTTPError outcomes (500 for unreachable Llama Stack, potential 401/403 for unauthorized access). No runtime behavior changes.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding docstrings to endpoint handler files (authorized.py, config.py, info.py).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/app/endpoints/info.py`:
- Around line 47-51: Update the docstring in src/app/endpoints/info.py to
reflect the actual HTTP status used: change the description that currently says
"status 500" to "status 503 Service Unavailable" (or explicitly reference
ServiceUnavailableResponse) and note that the raised HTTPException includes a
detail object containing `response` and `cause` when unable to connect to Llama
Stack; ensure this text aligns with the get_info_responses mapping and the
ServiceUnavailableResponse used in the function (e.g., where
ServiceUnavailableResponse is raised).
🧹 Nitpick comments (1)
src/app/endpoints/config.py (1)

47-48: LGTM with optional enhancement.

The docstring accurately describes the configuration loading check. For consistency with the info.py docstring which documents its Raises section, consider also documenting the potential HTTPException with status 500 when configuration loading fails (as indicated by the get_config_responses dictionary at line 31).

📝 Optional: Add Raises section for completeness
     Ensures the application configuration is loaded before returning it.

+    Raises:
+        HTTPException: with status 500 if the configuration is not loaded.
+
     Returns:
         ConfigurationResponse: The loaded service configuration response.

Comment on lines +47 to +51
Raises:
HTTPException: with status 500 and a detail object
containing `response` and `cause` when unable to connect to
Llama Stack. It can also return status 401 or 403 for
unauthorized access.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Incorrect HTTP status code in docstring.

The docstring states "status 500" but the actual code raises a 503 Service Unavailable (via ServiceUnavailableResponse at line 81). This matches the get_info_responses dictionary at line 31 which declares 503: ServiceUnavailableResponse.

📝 Suggested fix
     Raises:
-        HTTPException: with status 500 and a detail object
+        HTTPException: with status 503 and a detail object
         containing `response` and `cause` when unable to connect to
         Llama Stack. It can also return status 401 or 403 for
         unauthorized access.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Raises:
HTTPException: with status 500 and a detail object
containing `response` and `cause` when unable to connect to
Llama Stack. It can also return status 401 or 403 for
unauthorized access.
Raises:
HTTPException: with status 503 and a detail object
containing `response` and `cause` when unable to connect to
Llama Stack. It can also return status 401 or 403 for
unauthorized access.
🤖 Prompt for AI Agents
In `@src/app/endpoints/info.py` around lines 47 - 51, Update the docstring in
src/app/endpoints/info.py to reflect the actual HTTP status used: change the
description that currently says "status 500" to "status 503 Service Unavailable"
(or explicitly reference ServiceUnavailableResponse) and note that the raised
HTTPException includes a detail object containing `response` and `cause` when
unable to connect to Llama Stack; ensure this text aligns with the
get_info_responses mapping and the ServiceUnavailableResponse used in the
function (e.g., where ServiceUnavailableResponse is raised).

@tisnik tisnik merged commit 6132941 into lightspeed-core:main Jan 21, 2026
20 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant