aiohttp
aiohttp copied to clipboard
Addition of 'loop' Parameter Description in web.run_app Documentation
Is your feature request related to a problem?
Yes, the absence of a detailed description for the 'loop' parameter in the web.run_app function documentation is causing confusion among developers. Users are unclear about the purpose and usage of this parameter, which can lead to improper implementations or avoidable errors.
Describe the solution you'd like
I propose to add a comprehensive description of the 'loop' parameter including its purpose. This addition will provide clarity and assist developers in correctly utilizing the web.run_app function.
Describe alternatives you've considered
An alternative would be to include links to relevant sections of the documentation where the 'loop' concept is explained in detail. However, directly adding the description within the web.run_app section ensures that the information is immediately accessible where it's most needed.
Related component
Server
Additional context
No response
Code of Conduct
- [X] I agree to follow the aio-libs Code of Conduct
Hmm, the parameter should probably be removed. Like much of older asyncio APIs, loop parameters are generally not needed these days.
Since version 3.8+ run_app evaluates asyncio.new_event_loop() instead of asyncio.get_event_loop() if no loop from kwargs is avaliable so there actually are use cases.
There should not be, this is meant to work like asyncio.run() which does not take a loop parameter. My only hesitation in removing it, is to first introduce a way that this might become compatible with asyncio.Runner.
There should not be, this is meant to work like asyncio.run() which does not take a loop parameter. My only hesitation in removing it, is to first introduce a way that this might become compatible with asyncio.Runner.
I can try this issue, but first, let me clarify smth: web.run_app() isn't a coroutine, and changing it to be compatible with asyncio.Runner() and save backward compatibility will be tricky. One possible solution is to use web.AppRunner for async launching, as it is specifically designed for this purpose. Another option could be using web._run_app() but, I think web._run_app() isn't meant for external use, so the solution I see is maybe creating another async func that calls web._run_app() and can be asynchronously run. Let me know what you think, and I'll be able to start
Yes, my initial thoughts were more along the lines of introducing a new Runner, which would work like asyncio.Runner. Let me look again and I'll see if I can come up with a plan.
So, I was thinking about possibly subclassing asyncio.Runner, but the source code says not to do that: https://github.com/python/cpython/blob/67bba9dd0f5b9c2d24c2bc6d239c4502040484af/Lib/asyncio/runners.py#L46
In which case, I'd be tempted to just copy the code to implement the custom Runner. The expectation is that you'd be able to use this runner like asyncio.Runner for running regular coros with runner.run(), but also have a runner.run_app() method for running the application (or maybe an isinstance() check in runner.run()).
Probably not the easiest of tasks, but feel free to give it a go if you want.
So, I was thinking about possibly subclassing asyncio.Runner, but the source code says not to do that: https://github.com/python/cpython/blob/67bba9dd0f5b9c2d24c2bc6d239c4502040484af/Lib/asyncio/runners.py#L46
In which case, I'd be tempted to just copy the code to implement the custom Runner. The expectation is that you'd be able to use this runner like asyncio.Runner for running regular coros with
runner.run(), but also have arunner.run_app()method for running the application (or maybe an isinstance() check in runner.run()).Probably not the easiest of tasks, but feel free to give it a go if you want.
If this issue still relevant then I would like to try working on it. But before I start, I'd like to clarify some things. If I understand correctly, we first implement a new Runner, similar to asyncio.Runner, and only then do we remove the loop parameter from web.run_app?
and just to confirm, should the final user-facing api look something like this?
async def simple_coro():
pass
async def hello(request):
return web.Response(text="Hello, world")
app = web.Application()
app.add_routes([web.get('/', hello)])
if __name__ == '__main__':
with web.Runner() as runner:
runner.run(simple_coro())
runner.run_app(app, host='127.0.0.1', port=8080)
As for what you mentioned about combining the functionality into runner.run and using isinstance(), I believe it's better to keep runner.run and runner.run_app separate to avoid violating SRP and to keep the code more explicit. ?
If I understand correctly, we first implement a new
Runner, similar toasyncio.Runner, and only then do we remove theloopparameter fromweb.run_app?
Yes, focus on implementing the new Runner first. We can come back to the loop parameter in a later PR.
and just to confirm, should the final user-facing api look something like this?
Yes, I think that looks about correct.
I believe it's better to keep runner.run and runner.run_app separate to avoid violating SRP and to keep the code more explicit?
Yes, I'm fine with that. Should also keep it simpler to sync from cpython in future, as the only changes would be encapsulated in that method.