Migrate API routes to versioned router structure#39
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the API routing structure to introduce versioned endpoints (/api/v1) and separates global routes from versioned routes for better organization and maintainability.
Key Changes:
- Introduced a versioned API router (
/api/v1) that consolidates admin and document routes - Created a base router for non-versioned global endpoints (root and health check)
- Added dedicated admin endpoints for health checks and ping operations
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| RAGManager/main.py | Updated to include separate base and versioned API routers, removed inline root and health endpoints |
| RAGManager/app/api/routes/init.py | Created versioned API router structure that includes admin and documents routers |
| RAGManager/app/api/routes/base.py | Added base router with root and health check endpoints |
| RAGManager/app/api/routes/admin.py | Created admin-specific endpoints for health and ping checks |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| @router.get("/health") | ||
| def health_check(): | ||
| return {"message": "200 running..."} |
There was a problem hiding this comment.
The health check message '200 running...' is inconsistent with similar endpoints. Consider using a more standardized response format like {\"status\": \"ok\"} to match the admin health endpoint pattern.
| return {"message": "200 running..."} | |
| return {"status": "ok"} |
| router = APIRouter(tags=["base"]) | ||
|
|
||
| @router.get("/") | ||
| async def root(): |
There was a problem hiding this comment.
The root endpoint is defined as async but doesn't perform any async operations. Consider removing the async keyword for consistency with the health_check endpoint, or make health_check async for consistency.
| async def root(): | |
| def root(): |
🔍 PR Validation Results
|
Germanadjemian
left a comment
Germanadjemian
left a comment
There was a problem hiding this comment.
Duplicated endpoints in admin.py
Routing Structure Improvements:
/api/v1) inapp/api/routes/__init__.py, which includes theadminanddocumentsrouters for better route organization.app/api/routes/base.pyfor global (non-versioned) routes such as the root endpoint and a health check.Admin Endpoints:
adminrouter inapp/api/routes/admin.pywith/admin/healthand/admin/pingendpoints for administrative health checks.Main Application Setup:
main.pyto include the new base and versioned routers, replacing direct inclusion of document routes and removing redundant root and health check endpoints. [1] [2] [3]