Conversation
|
Needs more tests but we don't currently have any tests that test both lifespans and requests. I suppose this should be some sort of end to end test in |
Hmm... I think we can test it on |
Kludex
left a comment
There was a problem hiding this comment.
As I told you, I think this is a great idea. 👍
I'm just not sure about the new lifespan parameter on the HTTP protocol implementations. Maybe your previous idea about adding it to the ServerState may be better (just thinking here, do not change it yet).
Also, what about the WebSockets? The state is not included on the WebSocket scope?
|
I like having Lifespan be the thing creating it since it is “owned” by the lifespan. We could pass in |
|
Added support for websockets and tests for http, websockets and the lifespan itself. |
|
I think the main questions are:
|
|
The creation of the dict should be on the lifespan class, but I don't agree with the lifespan parameter on the protocol classes. I think it can be passed through the ServerState. The way I see it, the protocol classes should avoid as much as possible ASGI related concepts. After the startup runs the lifespan, we can pass the state to the server state object. Can't we? On the test suite, you made assertions inside the app. That doesn't work, as every exception inside the This is probably going to be an extension, so we need to provide the "lifespan.state" on the extensions on each scope on the protocol classes. And if is not, then we'll probably need to bump the spec - which I doubt this will happen. 👀 |
|
I moved to passing in the already copied dict to the protocols. This actually fixed a bug (or at least non-intentional behavior) where the state would be double copied for websockets. So now protocols have no references to lifespans. I used the name Working on fixing the assertions, thank you for pointing that out. |
|
We need to move this out of the extensions namespace into the top level namespace |
Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com>
|
hello, I am here because of renovate bot. I try to understand what this lifespan state is for and didn't find anything relevant in uvicorn doc. maybe you could link the discussion/issue this refers to ? |
You can read more about it here: |
No description provided.