-
Notifications
You must be signed in to change notification settings - Fork 54
LCORE-433: Support for all OpenAPI metadata #387
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LCORE-433: Support for all OpenAPI metadata #387
Conversation
WalkthroughAdds OpenAPI metadata (summary, contact, servers) to the FastAPI app and schema generation, updates the generated OpenAPI JSON accordingly, documents the local base URL in Markdown files, and adds email-validator to dependencies. No changes to routes, schemas, or runtime behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev
participant Script as generate_openapi_schema.py
participant App as FastAPI App
participant OpenAPI as get_openapi
participant File as docs/openapi.json
Dev->>Script: Run schema generation
Script->>App: Access app metadata (title, description, version, summary, contact, servers)
Script->>OpenAPI: get_openapi(..., summary, contact, servers)
OpenAPI-->>Script: OpenAPI document (with metadata)
Script->>File: Write updated JSON
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (4)
src/app/main.py (1)
26-33: OpenAPI contact details and summary: consider org-wide alias and clearer summary
- Summary duplicates description; consider a concise, value-focused summary (e.g., “Public API for LCS”).
- The contact block exposes a personal email. Prefer a team/role alias to avoid PII and onboarding/offboarding maintenance.
docs/openapi.md (1)
10-10: Fix markdownlint MD034: convert bare URL to a Markdown linkConvert the bare URL in the table to a proper link (and consider removing the trailing slash to match server URL).
Apply:
-| http://localhost:8080/ | Locally running service | +| [http://localhost:8080](http://localhost:8080) | Locally running service |docs/output.md (1)
10-10: Fix markdownlint MD034: convert bare URL to a Markdown linkAlign with lint hint and main docs. Also consider removing the trailing slash for consistency with OpenAPI servers.
Apply:
-| http://localhost:8080/ | Locally running service | +| [http://localhost:8080](http://localhost:8080) | Locally running service |docs/openapi.json (1)
5-5: Trim redundant summary; keep it concise and distinct from descriptionThe summary currently duplicates the description. Prefer a short, punchy summary and keep details in description.
Apply this diff:
- "summary": "Lightspeed Core Service (LCS) service API specification.", + "summary": "Lightspeed Core Service (LCS) API.",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
docs/openapi.json(1 hunks)docs/openapi.md(1 hunks)docs/output.md(1 hunks)pyproject.toml(1 hunks)scripts/generate_openapi_schema.py(1 hunks)src/app/main.py(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/output.md
10-10: Bare URL used
(MD034, no-bare-urls)
docs/openapi.md
10-10: Bare URL used
(MD034, no-bare-urls)
⏰ 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). (1)
- GitHub Check: e2e_tests
| "contact": { | ||
| "name": "Pavel Tisnovsky", | ||
| "url": "https://github.com/tisnik/", | ||
| "email": "ptisnovs@redhat.com" | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid publishing a personal email in OpenAPI contact metadata
Including an individual’s corporate email in public API docs can create PII/privacy concerns and spam. Prefer a role-based alias or omit email until one exists.
Option A — switch to a team contact and project URL:
- "contact": {
- "name": "Pavel Tisnovsky",
- "url": "https://github.com/tisnik/",
- "email": "ptisnovs@redhat.com"
- },
+ "contact": {
+ "name": "Lightspeed Core Maintainers",
+ "url": "https://github.com/lightspeed-core/lightspeed-stack"
+ },Option B — remove the email field and keep current name/url:
- "contact": {
- "name": "Pavel Tisnovsky",
- "url": "https://github.com/tisnik/",
- "email": "ptisnovs@redhat.com"
- },
+ "contact": {
+ "name": "Pavel Tisnovsky",
+ "url": "https://github.com/tisnik/"
+ },📝 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.
| "contact": { | |
| "name": "Pavel Tisnovsky", | |
| "url": "https://github.com/tisnik/", | |
| "email": "ptisnovs@redhat.com" | |
| }, | |
| "contact": { | |
| "name": "Lightspeed Core Maintainers", | |
| "url": "https://github.com/lightspeed-core/lightspeed-stack" | |
| }, |
| "contact": { | |
| "name": "Pavel Tisnovsky", | |
| "url": "https://github.com/tisnik/", | |
| "email": "ptisnovs@redhat.com" | |
| }, | |
| "contact": { | |
| "name": "Pavel Tisnovsky", | |
| "url": "https://github.com/tisnik/" | |
| }, |
🤖 Prompt for AI Agents
In docs/openapi.json around lines 7 to 11, the OpenAPI contact metadata exposes
a personal corporate email; replace it with a role-based/team alias or remove
the email field entirely. Either update "name" to a team name (e.g., "Project
Team" or "API Team"), set "url" to the project/team URL, and use a role-based
address such as "api-team@yourcompany.com", or simply delete the "email"
property while keeping the existing name and url; ensure the JSON remains valid
after the change.
| "servers": [ | ||
| { | ||
| "url": "http://localhost:8080/", | ||
| "description": "Locally running service" | ||
| } | ||
| ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Provide multi-environment servers and/or use templated server variables
Shipping only a localhost server in published OpenAPI can mislead consumers and some tooling will attempt to call localhost by default. Recommend adding a generic templated server and keeping localhost for dev.
Apply this diff:
- "servers": [
- {
- "url": "http://localhost:8080/",
- "description": "Locally running service"
- }
- ],
+ "servers": [
+ {
+ "url": "{scheme}://{host}{basePath}",
+ "description": "Default environment",
+ "variables": {
+ "scheme": { "default": "https", "enum": ["http", "https"] },
+ "host": { "default": "api.example.com" },
+ "basePath": { "default": "/" }
+ }
+ },
+ {
+ "url": "http://localhost:8080",
+ "description": "Locally running service"
+ }
+ ],To verify that the new metadata (summary/contact/servers) is sourced from code and the checked-in docs aren’t drifting from the generator, run:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for OpenAPI generation and FastAPI app metadata in the repo..."
echo "— Files with get_openapi or FastAPI instantiation —"
rg -n -A3 -B3 'get_openapi|FastAPI\(' || true
echo
echo "— Potential contact/servers wiring in Python —"
rg -n -A2 -B2 'contact=|servers=|summary=' || true
echo
echo "— Current contact and servers in docs/openapi.json —"
rg -n -A3 -B1 '"contact"|^ {0,}"servers": \[' docs/openapi.json || trueLength of output: 9029
Update FastAPI servers in application code
The OpenAPI JSON is generated from the FastAPI app’s servers setting in src/app/main.py, so manual edits to docs/openapi.json will be overwritten. Please update the app instantiation to include a templated default server alongside localhost, then regenerate the spec.
• File: src/app/main.py (around lines 38–40)
• After updating, run scripts/generate_openapi_schema.py to refresh docs/openapi.json
Suggested diff in src/app/main.py:
--- a/src/app/main.py
@@ app = FastAPI(
- servers=[{"url": "http://localhost:8080/", "description": "Locally running service"}],
+ servers=[
+ {
+ "url": "{scheme}://{host}{basePath}",
+ "description": "Default environment",
+ "variables": {
+ "scheme": {"default": "https", "enum": ["http", "https"]},
+ "host": {"default": "api.example.com"},
+ "basePath": {"default": "/"}
+ }
+ },
+ {
+ "url": "http://localhost:8080/",
+ "description": "Locally running service"
+ }
+ ],🤖 Prompt for AI Agents
In src/app/main.py around lines 38–40, the FastAPI app is missing a templated
default server entry and currently only sets localhost, causing
docs/openapi.json to be overwritten when manually edited; modify the
FastAPI(...) instantiation to include a servers list with both the existing
localhost server and a templated default server (e.g. url
"https://{hostname}{basePath}" with appropriate variables/defaults), save, then
run scripts/generate_openapi_schema.py to regenerate docs/openapi.json so the
OpenAPI spec contains the new templated server alongside localhost.
| "authlib>=1.6.0", | ||
| "openai==1.99.1", | ||
| "sqlalchemy>=2.0.42", | ||
| "email-validator>=2.2.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Validate the need for email-validator as a runtime dependency
FastAPI/OpenAPI contact email does not require this package. Keep it only if you use Pydantic’s EmailStr (runtime validation). Otherwise, consider removing it or moving it to an extra.
Run this repo scan to confirm actual usage before merging:
🏁 Script executed:
#!/bin/bash
# Find references to EmailStr or email validation usage
rg -n --hidden --glob '!**/dist/**' --glob '!**/build/**' --glob '!**/node_modules/**' $'\\bEmailStr\\b|email-validator'Length of output: 321
🏁 Script executed:
#!/bin/bash
# Search for direct imports or usage of the email-validator library
rg -n -E --hidden --glob '!**/dist/**' --glob '!**/build/**' --glob '!**/node_modules/**' '\b(email_validator|validate_email)\b'Length of output: 537
Consider removing the email-validator dependency
A search of the codebase shows no imports or uses of Pydantic’s EmailStr nor any direct calls into the email-validator API. Unless you’re planning to add runtime email validation, you can safely remove this from your main dependencies (or move it into an optional feature/extras group).
• File to update:
- pyproject.toml, line 38
- Remove or relocate:
- "email-validator>=2.2.0",
📝 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.
| "email-validator>=2.2.0", |
🤖 Prompt for AI Agents
In pyproject.toml around line 38, the dependency "email-validator>=2.2.0"
appears unused; remove that line from the main [tool.poetry.dependencies] (or
equivalent) section, or move it into an optional extras group (e.g.,
[tool.poetry.extras] or a dev/test extras group) so it is not installed by
default; update the dependency list accordingly and run dependency resolution
(poetry update / pip-tools regen) and tests to ensure nothing breaks.
| open_api = get_openapi( | ||
| title=app.title, | ||
| summary=app.summary, | ||
| version=app.version, | ||
| openapi_version=app.openapi_version, | ||
| description=app.description, | ||
| routes=app.routes, | ||
| license_info=app.license_info, | ||
| servers=app.servers, | ||
| contact=app.contact, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Confirm get_openapi supports summary and forward other available metadata
- Verify that your FastAPI version supports the
summarynamed arg inget_openapi(recent versions do). - Consider also forwarding
terms_of_serviceandtagsso the schema captures all top-level metadata when available.
Suggested change:
open_api = get_openapi(
title=app.title,
- summary=app.summary,
+ summary=getattr(app, "summary", None),
version=app.version,
openapi_version=app.openapi_version,
description=app.description,
routes=app.routes,
+ terms_of_service=getattr(app, "terms_of_service", None),
+ tags=getattr(app, "openapi_tags", None),
license_info=app.license_info,
servers=app.servers,
contact=app.contact,
)To verify signature support:
🌐 Web query:
Does fastapi.openapi.utils.get_openapi accept a 'summary' parameter, and from which FastAPI version?
💡 Result:
Yes. fastapi.openapi.utils.get_openapi accepts a summary parameter, and it’s supported starting from FastAPI 0.99.0 (with OpenAPI 3.1.0 support). The docs note: “The parameter summary is available in OpenAPI 3.1.0 and above, supported by FastAPI 0.99.0 and above.”[1]
- The official “Extending OpenAPI” guide lists get_openapi parameters including title, version, openapi_version, summary, description, and routes, and explicitly calls out the availability of summary from FastAPI 0.99.0 onward[1].
- The metadata tutorial also states that API-level summary is “Available since OpenAPI 3.1.0, FastAPI 0.99.0.”[6]
Citations:
- 1: https://fastapi.tiangolo.com/how-to/extending-openapi/
- 2: https://fastapi.tiangolo.com/reference/fastapi/
- 3: https://fastapi.xiniushu.com/hy/advanced/extending-openapi/
- 4: https://fastapi.tiangolo.com/tutorial/schema-extra-example/
- 5: https://fastapi.tiangolo.com/em/how-to/extending-openapi/
- 6: https://fastapi.tiangolo.com/tutorial/metadata/
- 7: https://www.linode.com/docs/guides/documenting-a-fastapi-app-with-openapi/
- 8: OpenAPI schema: Missing description with Query parameters fastapi/fastapi#5162
- 9: Request parameters model not exported to docs (and OpenAPI Specification) fastapi/fastapi#11543
- 10: https://fastapi.tiangolo.com/reference/openapi/docs/
Confirm get_openapi() parameters and include top-level metadata
get_openapi supports the summary parameter starting in FastAPI 0.99.0 (OpenAPI 3.1.0). To ensure your schema captures all available metadata, forward terms_of_service and tags (from app.openapi_tags) as well.
Locations to update:
- scripts/generate_openapi_schema.py, lines 68–78 (the
get_openapi()call)
Suggested diff:
scripts/generate_openapi_schema.py
@@ -68,6 +68,8 @@
open_api = get_openapi(
title=app.title,
- summary=app.summary,
+ summary=getattr(app, "summary", None),
version=app.version,
openapi_version=app.openapi_version,
description=app.description,
routes=app.routes,
+ terms_of_service=getattr(app, "terms_of_service", None),
+ tags=getattr(app, "openapi_tags", None),
license_info=app.license_info,
servers=app.servers,
contact=app.contact,
)Note: The summary argument is supported in FastAPI ≥ 0.99.0 (OpenAPI 3.1.0).
📝 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.
| open_api = get_openapi( | |
| title=app.title, | |
| summary=app.summary, | |
| version=app.version, | |
| openapi_version=app.openapi_version, | |
| description=app.description, | |
| routes=app.routes, | |
| license_info=app.license_info, | |
| servers=app.servers, | |
| contact=app.contact, | |
| ) | |
| open_api = get_openapi( | |
| title=app.title, | |
| summary=getattr(app, "summary", None), | |
| version=app.version, | |
| openapi_version=app.openapi_version, | |
| description=app.description, | |
| routes=app.routes, | |
| terms_of_service=getattr(app, "terms_of_service", None), | |
| tags=getattr(app, "openapi_tags", None), | |
| license_info=app.license_info, | |
| servers=app.servers, | |
| contact=app.contact, | |
| ) |
🤖 Prompt for AI Agents
In scripts/generate_openapi_schema.py around lines 68 to 78, the
get_openapi(...) call currently passes title, summary, version, openapi_version,
description, routes, license_info, servers, and contact but omits top-level
metadata; update the call to also pass terms_of_service=app.terms_of_service and
tags=app.openapi_tags (and confirm your FastAPI version supports the summary
parameter, i.e., FastAPI >= 0.99.0) so the generated schema includes
terms_of_service and tag metadata.
| servers=[ | ||
| {"url": "http://localhost:8080/", "description": "Locally running service"} | ||
| ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
❓ Verification inconclusive
Avoid hard-coded localhost server and trailing slash in OpenAPI servers
- Hard-coding “http://localhost:8080/” may leak into production docs and mislead clients. Prefer a relative server “/” or derive from configuration/env during schema generation.
- Drop the trailing slash to avoid double-slash joins in some clients/reverse proxies.
Minimal change:
- servers=[
- {"url": "http://localhost:8080/", "description": "Locally running service"}
- ],
+ servers=[
+ {"url": "http://localhost:8080", "description": "Locally running service"}
+ ],Optional improvement (outside this hunk): compute at runtime
# e.g., prefer relative base for portability
servers=[{"url": "/", "description": "Same origin"}]
# or, if you must include localhost for docs only, gate it via env/configRemove hard-coded localhost server URL and trailing slash
Hard-coding "http://localhost:8080/" can leak into production docs and a trailing slash may produce // in some client/proxy URL joins. As a minimal fix, drop the trailing slash; for a more robust solution, source the base URL from configuration or use a relative server path.
- File:
src/app/main.py(lines 38–40)
- servers=[
- {"url": "http://localhost:8080/", "description": "Locally running service"}
- ],
+ servers=[
+ {"url": "http://localhost:8080", "description": "Locally running service"}
+ ],Optional refactor ideas (outside this hunk):
# Use same-origin for portability
servers = [{"url": "/", "description": "Same origin"}]
# Or drive via env/config for docs only
servers = [{"url": os.getenv("API_BASE_URL", "/"), "description": "API base URL"}]🤖 Prompt for AI Agents
In src/app/main.py around lines 38-40, the OpenAPI servers entry hard-codes
"http://localhost:8080/" and includes a trailing slash; update this to avoid
leaking localhost into production and to remove the trailing slash by sourcing
the base URL from configuration (e.g., use os.getenv("API_BASE_URL", "/") as the
URL value) or use a relative "/" server entry instead; if you choose env-based
approach, add the necessary import for os and ensure the default value is "/" so
no trailing slash appears.
Description
LCORE-433: Support for all OpenAPI metadata
Type of change
Related Tickets & Documents
Summary by CodeRabbit
Documentation
Chores