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
bugfix: ASGI support for newer python versions > 3.9 #11541
base: main
Are you sure you want to change the base?
Conversation
|
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.
NB: I am not a maintainer
Quite a good solution, although I personally would do this differently. Either way, this would fix my issue (see #11545); it would be great, if you could mention it with the "closes" keyword :)
return self.response | ||
|
||
async def run_asgi(self, asgi_instance): |
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.
Looks like this should be run_asgi_instance
as it appears line 215 is trying to call it?
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.
@winglian - just checking to see if you saw this?
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.
This should solve the issue. I have a couple more nitpicks, though, to (hopefully) make the code more readable :)
asgi_task = loop.create_task(asgi_instance) | ||
loop.run_until_complete(asgi_task) | ||
else: | ||
asyncio.run(self.run_asgi_instance(asgi_instance)) |
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.
Do we actually need a separate function? From what I see in the handler, at this point in time, we already know that app
(or app.__call__
) is a coroutine. Can't we just run it directly instead?
asyncio.run(self.run_asgi_instance(asgi_instance)) | |
asyncio.run(asgi_instance) |
@@ -191,23 +191,33 @@ def __init__(self, scope): | |||
self.state = ASGICycleState.REQUEST | |||
self.app_queue = None | |||
self.response = {} | |||
self._legacy_asyncio = sys.version_info < (3, 10) |
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 fan of creating a class member variable just for checking. How about we declare a local variable at the top of the __call__
function instead?
using any version of python > 3.9 results in
<class 'TypeError'>
when trying to launch an application like FastAPIedit: closes #11545