-
-
Notifications
You must be signed in to change notification settings - Fork 970
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
Apply right name to Route
when created from methods
#1553
Conversation
Are there other cases we are forgetting? |
The only
But in the nature of the lambda function is anonymous, we can not read a name. After consulting the docs of the def get_name(endpoint: typing.Callable) -> str:
if inspect.isroutine(endpoint):
return endpoint.__name__
return endpoint.__class__.__name__ This might create issues with the lambda functions though: from starlette.responses import JSONResponse
from starlette.routing import Route
async def my_function(request):
return JSONResponse({'endpoint_type': 'function'})
class MyClass:
def __call__(self, request):
return JSONResponse({'endpoint_type': 'class'})
class MySpecialEndpointObject:
async def my_method(self, request):
return JSONResponse({'endpoint_type': 'method'})
@classmethod
async def my_classmethod(self, request):
return JSONResponse({'endpoint_type': 'classmethod'})
@staticmethod
async def my_staticmethod(self, request):
return JSONResponse({'endpoint_type': 'staticmethod'})
function_route = Route('/functionEndpoint', my_function)
class_route = Route('/classEndpoint', MyClass())
endpoint_obj = MySpecialEndpointObject()
method_route = Route('/methodEndpoint', endpoint_obj.my_method)
classmethod_route = Route('/classMethodEndpoint', endpoint_obj.my_classmethod)
staticmethod_route = Route('/staticMethodEndpoint', endpoint_obj.my_staticmethod)
lambda_route = Route('/lambdaEndpoint', lambda request: JSONResponse({'endpoint_type': 'lambda'}))
assert function_route.name == "my_function"
assert class_route.name == "MyClass"
assert method_route.name == "my_method"
assert classmethod_route.name == "my_classmethod"
assert staticmethod_route.name == "my_staticmethod"
assert lambda_route.name == "lambda" # AssertionError, because it will be: "<lambda>" |
I think it's okay if lambdas aren't handled correctly |
Can we add a test for it? |
@@ -84,7 +84,7 @@ async def app(scope: Scope, receive: Receive, send: Send) -> None: | |||
|
|||
|
|||
def get_name(endpoint: typing.Callable) -> str: | |||
if inspect.isfunction(endpoint) or inspect.isclass(endpoint): | |||
if inspect.isroutine(endpoint) or inspect.isclass(endpoint): |
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.
Does the behavior of lambda changes now?
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.
No:
import inspect
my_lambda = lambda: 1
assert inspect.isfunction(my_lambda) == inspect.isroutine(my_lambda)
I'm currently quite busy. But I'll add tests next week. |
@flxdot Are you be able to work on this soon or can I take it over? |
Friday afternoons seem to be only free time atm. Since |
Thanks for the tests! Yeah, no need to test specifically |
Good call reducing the clutter in the test. Thanks for the hint! |
Once the linter issues are resolved, we can merge it. |
Route
when created from methods
sorry for all that back and forth. Should have dug deeper into the GitHub action to see, what is run. |
Thanks @flxdot ! :) |
Fixes #1552.
Adapted existing code to fix wrong name of a Route when set automatically for routes with methods as endpoints.