Repository navigation
[18.0][FIX] fastapi: pool apps by endpoint root path, not request path - #670
SparkDrago05 wants to merge 1 commit into
Conversation
|
All commit authors signed the Contributor License Agreement. |
|
Hi @lmignon, |
|
The failing test jobs are unrelated to this change: every 18.0 PR currently fails on |
lmignon
left a comment
lmignon
left a comment
There was a problem hiding this comment.
Thank you for the fix. LGTM.
| # Requests to different paths of an endpoint share the apps of its root | ||
| # path, instead of building one app per request path. |
There was a problem hiding this comment.
IMO this comment is useless.... Here you check that call to endpoints under the same root path are processed by the same app (only one app initialized by root path)
| # Pool apps by the endpoint's root path, not by the request path: | ||
| # otherwise each distinct path builds and keeps its own app. |
There was a problem hiding this comment.
Comment useless... it's the the what not the why.... Here we want to ensure that only one app exists by endpoint's root path.
lmignon
left a comment
lmignon
left a comment
There was a problem hiding this comment.
Can you fix the comments?
FastApiDispatcher passed the full request path as the key of the app pool. Every distinct path under an endpoint, including unknown paths and path parameters, built and kept its own FastAPI application until the registry cache was invalidated, so worker memory grew with the number of distinct paths requested, even unauthenticated ones. Resolve the endpoint's root path once per request and use it as the pool key and for the uid lookup. Fixes OCA#656 Assisted-by: Claude Opus 5.5 Signed-off-by: SparkDrago05 <huzaifa.farooq05@gmail.com>
0af2b57 to
6dd4696
Compare
|
The 19.0 forward port is in #677. |
Fixes #656.
The app pool was keyed on the full request path, so every distinct URL built and kept its own FastAPI app. It is now keyed on the endpoint's root path.
Tested on 18.0 with the steps from the issue (53 requests): 51 apps kept before, 1 after. A new test covers it, and the
fastapitests pass (24/24).Assisted-by: Claude Opus 5.5