-
Notifications
You must be signed in to change notification settings - Fork 179
Fixes Register Endpoint for Router of Instance of APIWebSocketRoute #358
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Fixes Register Endpoint for Router of Instance of APIWebSocketRoute #358
Conversation
5d17034
to
bed8cfb
Compare
Hey @yuval9313, could you or another maintainer take a look in this merge request please? Thanks a lot, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are also missing test case
fastapi_utils/cbv.py
Outdated
@@ -108,12 +108,16 @@ def _register_endpoints(router: APIRouter, cls: Type[Any], *urls: str) -> None: | |||
_allocate_routes_by_method_name(router, url, function_members) | |||
router_roles = [] | |||
for route in router.routes: | |||
if not isinstance(route, APIRoute): | |||
raise ValueError("The provided routes should be of type APIRoute") | |||
if not isinstance(route, APIRoute) and not isinstance(route, APIWebSocketRoute): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry, this is really nit pick but I prefer not (condition a or condition b)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a problem at all, i'll happily make the change :)
b12a8cd
to
a224089
Compare
Hello, I've added a test case in the unittests for the websocket example, it tests the three basic operations on the websocket connection (can accept connection, can send_text, receive_text and can close). I hope this test is okay :) |
790d46a
to
5e2b161
Compare
Hello @yuval9313 , can we resume this MR? I'd love to have this merged |
Versions
Python Version: 3.13
Package Versions:
Bug
When creating a Route of type
APIWebSocketRoute
the following error is thrown:This error can be replicated using the following example:
Solution
The solution i recommend is to just add the class to instance check of the
_register_endpoints
function and then add it with a fixedroute_method
of"WS"
since theAPIWebSocketRoute
does not have amethods
atribute.Thank you very much,
Vitor Hideyoshi.