Do not overwrite "path" and "root_path" scope keys#2352
Conversation
|
I'm going to try to make this PR cleaner, but it already solves the problem. |
"path" and "root_path" scope keys
frankie567
left a comment
There was a problem hiding this comment.
Apart from my question above, LGTM! You have filled the holes I left in my previous PR :)
Still not convinced about the naming current_root_path and current_path, but that's minor 😄 Suggestion (not ideal): route_root_path / route_path.
| "path_params": path_params, | ||
| "app_root_path": scope.get("app_root_path", root_path), | ||
| "root_path": root_path + matched_path, | ||
| "path": remaining_path, |
There was a problem hiding this comment.
Don't we need to keep the original root_path and path in the child scope?
There was a problem hiding this comment.
Aren't they always there anyway? Those lines were just modifying them, no?
There was a problem hiding this comment.
Yes, you're right. My point was if we were using this child_scope as the new scope for inner routers, but it seems actually that it's always merged with the root scope.
6ae76f3 to
f684f3b
Compare
|
@encode/maintainers can someone review this, please? |
abersheeran
left a comment
There was a problem hiding this comment.
It looks very nice, I like the regular expressions here.
path now includes root_path so we need a different way to remove it. It seems like route_root_path gives this information. See Kludex/starlette#2361 Kludex/starlette#2352 Kludex/starlette#1336
path now includes root_path so we need a different way to remove it. It seems like route_root_path gives this information. See Kludex/starlette#2361 Kludex/starlette#2352 Kludex/starlette#1336
Fixes the change in behaviour in starlette v0.33 via PR [#2352](Kludex/starlette#2352)
FastAPI has fixed the broken starlette interaction in 0.109.2: https://fastapi.tiangolo.com/release-notes/#01092 Starlette handling of root_path has changed in 0.33.0: Kludex/starlette#2352. Prior to this, it was incorrectly stripping the root_path from the path field (which we need to mirror). Added compatibility logic to handle both of these. --------- Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com> Co-authored-by: Lonnie Liu <95255098+aslonnie@users.noreply.github.com>
scope["root_path"]andscope["path"]differs from ASGI spec #1336