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

[Docs] Lifespan ref implementation #241

Open
srkunze opened this issue Feb 20, 2021 · 5 comments · May be fixed by #242
Open

[Docs] Lifespan ref implementation #241

srkunze opened this issue Feb 20, 2021 · 5 comments · May be fixed by #242

Comments

@srkunze
Copy link

srkunze commented Feb 20, 2021

Hi everyone. Looking at https://asgi.readthedocs.io/en/latest/specs/lifespan.html , I saw the following example implementation of the lifespan type.

        while True:
            message = await receive()
            if message['type'] == 'lifespan.startup':
                ... # Do some startup here!
                await send({'type': 'lifespan.startup.complete'})
            elif message['type'] == 'lifespan.shutdown':
                ... # Do some shutdown here!
                await send({'type': 'lifespan.shutdown.complete'})
                return

My naive implementation would consist of four flat lines of code only (receive, send, receive, send).

What is the reason for the while True loop and the fail-safe message handling here? It's seems like I am missing something.

@andrewgodwin
Copy link
Member

That's generally because you are going to be receiving more than just lifespan events, and to allow forwards-compatibility if we add more message types (e.g. imagine we added lifespan.pre-startup).

receive() doesn't receive a specific message type nor is there any guarantees on ordering, so it has to be in a loop like this.

@srkunze
Copy link
Author

srkunze commented Feb 21, 2021

I’m a somewhat new to the world of asyncio, so please bear with me.

That's generally because you are going to be receiving more than just lifespan events

The loop from the docs is specifically for lifespan. So, I don’t understand why this loop helps other event types.

to allow forwards-compatibility if we add more message types (e.g. imagine we added lifespan.pre-startup).

Wouldn’t I then add another receive/send pair in my implementation when that happens?

It somehow makes me feel uneasy that this loop+ifs can hide potential bugs quite easily.

nor is there any guarantees on ordering, so it has to be in a loop like this.

Shouldn’t pre-startup not run before startup and startup not before shutdown? I mean I would write my code based on these assumptions (e.g. regarding variable states).

@andrewgodwin
Copy link
Member

When you write an app, you have to handle all specs combined, as you have a single main loop - you'd almost never just write a lifespan app, you'd probably combine it with HTTP or something. Thus, you definitely don't know what's coming down the receive(). Think of it as an opaque queue where you know the set of types that might come down it, but not the order.

What I'd recommend to avoid bugs in a situation like this is an else: clause that raises an error when it sees an unknown message; that way, the loop is just handling unknown ordering rather than also letting through unknown message types.

@srkunze
Copy link
Author

srkunze commented Feb 22, 2021

I get the feeling, we might be talking past each other. This is the complete code example from the docs:

image

As I understand it, the highlighted loop (while True) is just for the lifespan protocol, which only contains two message types to be received and processed in this order.

So, my suggestion is the following:

async def app(scope, receive, send):
    if scope['type'] == 'lifespan':
        message = await receive()
        assert message['type'] == 'lifespan.startup'
        ... # Do some startup here!
        await send({'type': 'lifespan.startup.complete'})

        message = await receive()
        assert message['type'] == 'lifespan.shutdown'
        ... # Do some shutdown here!
        await send({'type': 'lifespan.shutdown.complete'})
    else:
        pass # Handle other types

Am I missing something here?

@andrewgodwin
Copy link
Member

Oh, you're right, sorry - the loop should be outside the if statement there. That does need fixing!

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

Successfully merging a pull request may close this issue.

2 participants