Split async server running functions

[txomon]

Long story short

The requirement to have run_app as only execution of the server is not enough for some applications. It would be nice if this run_app functionality could be divided in two functions, start() and cleanup(), as many other libraries do. After all aiohttp is not the core of my app.

The idea is I want to be in charge of the main loop and control how and when it runs.

Expected behaviour

Not to modify existing applications when including aiohttp.

Actual behaviour

aiohttp get's to be the centre of the application by running run_forever() inside the only provided function to run the web server part.

Steps to reproduce

1. Write asyncio application
2. Try to include web interface using aiohttp
3. Modify application to be run with aiohttp as the centre of it, synchronously.

[arthurdarcet]

aiohttp does not need to be the center of your app. Have a look at what the run_app helper is doing: https://github.com/aio-libs/aiohttp/blob/master/aiohttp/web.py#L412

You can copy some of that logic in your code, and trigger your loop.run_forever whenever you want. But of course, you should also handle some "process-related" issues on your own, such as handling the SIGTERM/INT signals to allow aiohttp to shutdown gracefully.

[asvetlov]

Anyway keeping separate startup and shutdown functions in public api makes sense.
Honestly I cannot remember all needed steps without looking into source code

[alexmnazarenko]

In my project, I've implemented AsyncServer class for the very same reasons.

Something like this:

class AsyncServer:
    def __init__(...run_app params..)
    async def start()
    async def shutdown()
    async def wait()
    async def __aenter__()
    async def __aexit__()

Works fine )

Something like this will be nice to have right in the library.

[gjcarneiro]

I have to add my +1 to fixing this. IMHO, the best thing would be to have a run() coroutine method on aiohttp.web.Application(), so that my main app looks like (simplified):

async def my_suff():
   ...

def main():
    loop = asyncio.get_event_loop()
    app = web.Application()
    ....
    loop.run_until_complete(
        asyncio.gather(
            app.run("0.0.0.0", 80),
            run_my_stuff(),
        )
    )

Now wouldn't that be lovely? ;-)

[asvetlov]

No, it should be a separate class.
Think about running the same app on different hosts/ports and stopping the app.

[gjcarneiro]

In that case, I think something like @alexmnazarenko's AsyncServer would be great to have. @alexmnazarenko, is your code available somewhere?

[alexmnazarenko]

@gjcarneiro Yes, it is: https://github.com/datadvance/pRpc/blob/master/prpc/platform/ws_aiohttp.py

But it's not feature complete for inclusion in aiohttp itself. I've dropped support for UDP sockets as I don't need it for know. Still, it works well for me allowing to run multiple servers on different ports in a single asyncio app.

[cecton]

After all aiohttp is not the core of my app.

I don't get that part. During the lifetime of the main process you want your web service to be started and stopped? Can you explain a use case for that?

@txomon @alexmnazarenko can you explain the use case? I just checked the pRpc library which does provide the lib but doesn't really use it except for tests.

[txomon]

In my case, this was part of a bot, which had many other things running at the same time. If i wanted to expose an API, but it wasn't the main objective of the server, I would still need to set this as the main process.

The idea is that aiohttp shouldn't be expected to be the centre of the app.

[cecton]

Does the API you're exposing get not exposed at some point? Or is the API always running until the end of the process?

but it wasn't the main objective of the server

In the current state of the project, is it now possible to start your bot with and without API? Or did it evolve from a loop to a web service?

The idea is that aiohttp shouldn't be expected to be the centre of the app.

That I get 😁 but it seems to me that if aiohttp is not the center of the application, then the application exposes an API for some time and then remove it. This particular use case doesn't make much sense to me.

[txomon]

This specific bot has several configuration options, working in some cases using websockets, other times push http notifications and others both. Coupling the main process to the webserver process would make it need to have the webserver up regardless on the configuration.

Also, it has hot plugin (re)loading, and the webserver may be terminated and restarted to accommodate new endpoints.

[cecton]

Thx! Makes a lot more sense now. What do you need exactly in the web.run_app for your application? I suppose you will handle the signals yourself.

web.run_app provides:

• SIGTERM and SIGINT handling
• servers creations and closure
• startup, clean and shutdown hooks

[txomon]

The best for me would to actually have those features separated in different functions, that I can decide to use or not.

In my case, I don't make any difference between server creation/closure and the startup, clean and shutdown hooks.

I'm not familiar on the internals, but in my case, having functions that can be used like the following:

1. Run/await this function to init the lib
2. Await this function to start a server
3. Await this function to stop the server
4. Run/await this function to clean up the lib

Where steps 2 and 3 can be looped (to restart the service). Maybe you don't need to have steps 1/4, but just following on the design pattern of other libs, such as libcurl

[alexmnazarenko]

@cecton As of my usecases:

• Multiple apps on different ports + some async init/teardown
• Weird tests

Example of multiple apps usecase:
https://github.com/datadvance/pAgent/blob/master/pagent/__main__.py

Here I have an program that uses HTTP for everything - for RPC, for administration and more. RPC and admin apps must have different endpoints (admin is usually localhost only).

[cecton]

Cool! Thanks a lot for all your feedbacks.

I believe signal handling is out of the scope here so we should probably have a start() and shutdown() (or stop()?) functions that will trigger the hooks and create/remove the servers cleanly like @alexmnazarenko did on https://github.com/datadvance/pRpc/blob/master/prpc/platform/ws_aiohttp.py

I'm not too much into introducing a new object. @asvetlov do you think we can add the start and stop methods on the class Application directly?

Then I will reduce web.run_app to only handle the signals and start the appropriate methods.

Do you think that will cover it all? I skipped the async context and the wait method because I don't think they are any useful but let me know if I'm totally wrong.

@asvetlov cecton@3b8a782 here a sample

[alexmnazarenko]

That should help, 'wait' and other convenience stuff can be easily implemented externally. The main point for me is to avoid copying the whole run_app signal stuff (as it can change between versions).

[cecton]

Ok perfect then? I will make some tests and see if it works as expected.

What is the use case of a "wait" by the way? Because you should be able to achieve the same thing by using the cleanup hook. What also would be the use case for an async context like you implemented? In tests maybe?

When you say "signals" you mean the "signals"/hooks of the application right? Not the Unix signals?

[alexmnazarenko]

Yeap, "ok perfect" 😄

1. Wait is not critical. It does replace cleanup hook for me, just with less local variable hassle. But there are multiple ways to handle that, so I think "adding less API to the framework" is a strong enough concern to drop it.
2. Yes, I mean hooks. The internal hook implementation classes are actually called '*Signal', that's why I call them so )

PS in your code sample "await self.cleanup()" is called twice?

[cecton]

Yes it was just a sample 😫 I did it really fast to show to Andrew what I had in mind.

[asvetlov]

I not sure should we add new methods and state into Application or create a new class for running server.
For nested app these methods are useless and should not be called.

[cecton]

@asvetlov I haven't thought about that and that's a good point. From my point of view it seems that the logic of Application is well scoped in the class itself. It was weird to have it spread in a different method.

[alexmnazarenko]

New class is fine by me. As I wrote, I just want to remove hook/socket handling from my code as it's an 'unstable' copypaste from the framework.

Also, I forgot - my implementation had a nice bonus of returning 'resolved' bound endpoints. That's very helpful when you use port '0'.

[asvetlov]

Looks like we need a Pull Request.
Any hero?

[cecton]

@asvetlov I will but not now 😀 after office hours

[cecton]

@asvetlov so I make a different class or I put it in application?

[asvetlov]

I suggest a different class.
Application describes a web site and used on initialization stage, a new class runs the app on single host/port.
Hmm, as a consequence the runner should not call on_startup() and on_cleanup() signals, only on_shutdown.
Startup/cleanup should be called only once. Maybe we need separate methods for it.

[cecton]

@asvetlov maybe I can make a class that takes multiple Application classes and do the whole job? Everything will be called once.

[asvetlov]

Maybe it's good idea, let's look on API.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs.
If you feel like there's important points made in this discussion, please include those exceprts into that [new issue].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new