Skip to content
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

Addition of 'loop' Parameter Description in web.run_app Documentation #8028

Open
1 task done
libormatejek opened this issue Jan 12, 2024 · 6 comments
Open
1 task done

Comments

@libormatejek
Copy link

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

  • I agree to follow the aio-libs Code of Conduct
@Dreamsorcerer
Copy link
Member

Hmm, the parameter should probably be removed. Like much of older asyncio APIs, loop parameters are generally not needed these days.

@libormatejek
Copy link
Author

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.

@Dreamsorcerer
Copy link
Member

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.

@DavidRomanovizc
Copy link
Contributor

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

@Dreamsorcerer
Copy link
Member

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.

@Dreamsorcerer
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants