Alternative lifespan state api by adriangb · Pull Request #2065 · Kludex/starlette

adriangb · 2023-03-05T20:57:45Z

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.

This reverts commit da6461b.

Kludex · 2023-03-05T21:09:19Z

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 · 2023-03-05T21:11:33Z

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 · 2023-03-05T21:25:55Z

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 · 2023-03-05T21:27:06Z

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 · 2023-03-08T09:07:36Z

starlette/types.py

+StatelessLifespan = typing.Callable[["Starlette"], typing.AsyncContextManager[None]]
+StatefulLifespan = typing.Callable[
+    ["Starlette"], typing.AsyncContextManager[typing.Mapping[str, typing.Any]]
+]


Shouldn't it be ASGIApp instead of Starlette?

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

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.

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 · 2023-03-08T15:04:45Z

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 · 2023-03-09T19:27:16Z

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 · 2023-03-12T13:27:41Z

starlette/testclient.py


    async def lifespan(self) -> None:
-        scope = {"type": "lifespan"}
+        scope = {"type": "lifespan", "state": self.app_state}


@florimondmanca like this

FaydSpeare · 2023-07-20T08:09:37Z

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.

Kludex/starlette#2065

Revert "Support lifespan state (Kludex#2060)"

44159d0

This reverts commit da6461b.

adriangb requested a review from Kludex March 5, 2023 20:57

adriangb force-pushed the alternative-lifespan-state-api branch from ec2eafa to 4e5ec27 Compare March 5, 2023 21:06

adriangb force-pushed the alternative-lifespan-state-api branch from 4e5ec27 to 5e36c4c Compare March 5, 2023 21:09

new implementation

14fd988

adriangb force-pushed the alternative-lifespan-state-api branch from 5e36c4c to 14fd988 Compare March 5, 2023 21:12

Kludex reviewed Mar 8, 2023

View reviewed changes

Kludex added 2 commits March 9, 2023 20:15

Merge branch 'master' into alternative-lifespan-state-api

cf72969

Add documentation about lifespan state

fc1182c

Kludex approved these changes Mar 9, 2023

View reviewed changes

adriangb merged commit 92ab71e into Kludex:master Mar 9, 2023

adriangb deleted the alternative-lifespan-state-api branch March 9, 2023 20:04

Kludex mentioned this pull request Mar 9, 2023

Deprecate on_startup and on_shutdown events #2070

Merged

Kludex reviewed Mar 12, 2023

View reviewed changes

zanieb mentioned this pull request Mar 13, 2023

Switch from deprecated server on_startup/shutdown hooks to lifespan PrefectHQ/prefect#8786

Closed

1 task

adriangb mentioned this pull request Jul 20, 2023

Try to make Request generic over State #2218

Closed

magentalabs-serviceagent-1 pushed a commit to magenta-aps/fastrampqi that referenced this pull request Aug 24, 2023

fix: [#57083] Starlette Lifespan Context

0e9a262

Kludex/starlette#2065

magentalabs-serviceagent-1 pushed a commit to magenta-aps/fastrampqi that referenced this pull request Aug 25, 2023

fix: [#57083] Starlette Lifespan Context

98c9ea0

Kludex/starlette#2065

magentalabs-serviceagent-1 pushed a commit to magenta-aps/fastrampqi that referenced this pull request Aug 25, 2023

fix: [#57083] Starlette Lifespan Context

2c943e4

Kludex/starlette#2065

abhiTronix mentioned this pull request May 15, 2024

[Proposal]: Deprecate on_startup and on_shutdown in favor of lifespan abhiTronix/vidgear#397

Closed

4 tasks

Uh oh!

Conversation

adriangb commented Mar 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kludex commented Mar 5, 2023

Uh oh!

adriangb commented Mar 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kludex commented Mar 5, 2023

Uh oh!

adriangb commented Mar 5, 2023

Uh oh!

Kludex left a comment

Choose a reason for hiding this comment

Uh oh!

Kludex Mar 8, 2023

Choose a reason for hiding this comment

Uh oh!

adriangb Mar 9, 2023

Choose a reason for hiding this comment

Uh oh!

Kludex Mar 9, 2023

Choose a reason for hiding this comment

Uh oh!

adriangb Mar 9, 2023

Choose a reason for hiding this comment

Uh oh!

Kludex commented Mar 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kludex commented Mar 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kludex Mar 12, 2023

Choose a reason for hiding this comment

Uh oh!

FaydSpeare commented Jul 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

adriangb commented Mar 5, 2023 •

edited

Loading

adriangb commented Mar 5, 2023 •

edited

Loading

Kludex commented Mar 8, 2023 •

edited

Loading

Kludex commented Mar 9, 2023 •

edited

Loading

FaydSpeare commented Jul 20, 2023 •

edited

Loading