-
Notifications
You must be signed in to change notification settings - Fork 180
Open
Labels
bugSomething isn't workingSomething isn't working
Description
I've just run into this issue in the wild. It's not a bug as such, but perhaps there's a way to prevent it catching people out like it did me?
Consider this example:
from fastapi import APIRouter, FastAPI, Request
from fastapi_utils.cbv import cbv
router = APIRouter()
@cbv(router)
class Foo:
@router.get("/foo")
async def example(self, request: Request):
return request.url_for("example")
@cbv(router)
class Bar:
@router.get("/bar")
async def example(self, request: Request):
return request.url_for("example")
app = FastAPI()
app.include_router(router)
Both of the url_for("example")
calls return "/foo"
, whereas the expected behaviour might be for the second one to return "/bar"
instead.
Without CBVs, IDEs and linters would warn about the function example
being redefined, but it obviously doesn't when they're in two different classes. So the problem goes unnoticed until runtime, at which point the 'wrong' path is used.
If we made a change like this to cbv.py...
for route in cbv_routes:
router.routes.remove(route)
_update_cbv_route_endpoint_signature(cls, route)
route.name = cls.__name__ + '.' + route.name # <-------------- this line is new
cbv_router.routes.append(route)
router.include_router(cbv_router)
return cls
...then we could do url_for("Foo.example")
or url_for("Bar.example")
.
However:
- It would need to be backwards compatible, although I don't see any immediate problem with inserting the same route twice with two different names
- It wouldn't stop the problem happening if two different modules both had CBVs with the same name, each containing a method with the same name
- It wouldn't play nicely with
from x import Y as Z
, if this matters - it would still use the nameY.whatever
Do you think this is worth addressing somehow? Thanks!
Metadata
Metadata
Assignees
Labels
bugSomething isn't workingSomething isn't working