Alternative lifespan state api

[adriangb]

The first commit reverts the current unreleased API, the second adds the proposed API. I can do the same thing for the docs.

I think one of the interesting things this opens up is that by having a concrete type for the user's state we could use https://peps.python.org/pep-0696/ in the future to do:

from typing_extensions import TypeVar  # support any Python version
from starlette.datastructures import State

StateType = TypeVar("StateType", default=State)

class Request(Generic[StateType]):
    ...

So users can fully type their use of state with minimal boilerplate:

class State(TypedDict):
    foo: int

@asynccontextmanager
async def lifespan(app) -> State:
    yield State(foo=1)

async def endpoint(request: Request[State]) -> Response:
    request.state["foo"] + 1  # type checks correctly

Or if they are adding more keys to the request:

class LifespanState(TypedDict):
    foo: int

class State(LifespanState):
   user: str | None

@asynccontextmanager
async def lifespan(app) -> LifespanState:
    yield State(foo=1)

async def endpoint(request: Request[State]) -> Response:
    request.state["foo"] + 1  # type checks correctly

Or something like that.

[Kludex]

What you are proposing is to not add the state to startup and shutdown at all?

What you mention about the PEP-696 can still be done without this PR getting in.

[adriangb]

What you are proposing is to not add the state to startup and shutdown at all?

Yes. I think that's pretty reasonable.

What you mention about the PEP-696 can still be done without this PR getting in.

Yes it can but with the current API you can't really use a type in your lifespan which I think makes it a bit less interesting.

(well you can, but you'd be doing something like state.update(MyTypedDict(foo=1)), which is kinda weird)

[Kludex]

I'm against not supporting startup and shutdown.

If you'd like, we can revert the two previous commits (implementation, and docs), and discuss a bit more about what we want to do.

I'll personally not be in favor of removing or not supporting state on startup and shutdown, but maybe we can just change the signature from () -> None to (state) -> None instead of introspecting on 1.0?

[adriangb]

I'm against not supporting startup and shutdown.

We could always add that later, with the current API...

[Kludex]

Only documentation is missing here, and we can go with it. 👍

[Kludex]

Shouldn't it be ASGIApp instead of Starlette?

[adriangb]

Well we always pass in a Starlette object right? What good does it do to receive a reference to a generic ASGIApp object?

[Kludex]

I was thinking about FastAPI when I asked that, when the super() is called on Router, they will have an issue over there, I guess? Also, because this module has ASGI related types.

In any case, not that important.

[adriangb]

Yeah probably need a self: SelfType, lifespan: Lifespan[SelfType] or something. Or users can just cast it. Also I don't think there's any FastAPI specific methods they'd want to access from the lifepsan. And with this state thing there's almost no reason to even use the app argument anyway...

[Kludex]

After this PR, we can deprecate on_shutdown and on_startup, with something like:

The on_startup and on_shutdown parameters are deprecated, and they
will be removed on version 1.0. Use the lifespan parameter instead.
See more about it on https://www.starlette.io/events/.

And maybe changing the name events.md by lifespan.md... 😅

I can do this follow up, I just need this to be merged. 👍

[Kludex]

I'm going to do a follow-up with the deprecation of on_startup and on_shutdown.

@adriangb Please check #2065 (comment), and my last commit to see if you are fine with it.

[Kludex]

@florimondmanca like this

[FaydSpeare]

It would be nice to give a way for users to have type hints on their request.state. There's an example in the Lifespan section of the starlette docs that uses a TypedDict, but inside your routes that doesn't offer you any type hints. It also doesn't make a lot of sense, since you're accessing attributes on state (state.count), which is not how you access fields of a dictionary (or TypedDict).

I was just trying to give myself type hints by creating a subclass of Request, MyRequest, and using the MyState TypedDict as the type for its state attribute. This already required # type: ignore[assignment]. But when you go to use MyRequest in your routes, type hints will now tell you that req.state.count doesn't exists, but req.state["count"] does, which isn't the case.
Is TypedDict not essentially an incorrect type for starlette State?

You could use a dataclass instead of a TypedDict, but the typing for the lifespan won't accept that.

So it would be quite nice to have something like Request[MyState] which allows you to do this.