Recommend using lifespan state instead of app.state

[adriangb]

Should we change the docs to recommend using the newly added lifespan state instead of app.state?

[sm-Fifteen]

Is it supported by any ASGI servers besides the very latest version (i.e: released yesterday) version of Uvicorn? That's going to be a caveat to reccomending it until support for this can be more or less assumed.

As for replacing app.state, isn't there some way app.state can be made to wrap scope['state'], depending on whether it's supported or not? Both fill the same role and really are just dicts. I actually just realized that can't happen because current lifespan event handlers can only set app.state by first obtaining the app from global scope, so the state of lifespan events is a completely new thing with no equivalent.

def open_database_connection_pools(app: Starlette):
    app.state.db_pool = ...

def close_database_connection_pools(app: Starlette):
    app.state.db_pool.dispose()


app = Starlette()

app.add_event_handler('startup', partial(open_database_connection_pools, app))
app.add_event_handler('shutdown', partial(close_database_connection_pools, app))

So nevermind that part about app.state wrapping scope.state. I'd cautiously reccomend using lifespan state, but also add "lifespan state may not be supported on all ASGI servers yet" to the list of known issues around lifespan like "lifespan propagation to mounted applications is not guaranteed" and "lifespan teardown may not run is the app is not cleanly shutdown" that make it... not as practical at the moment as it should be.

[adriangb]

Honestly if we add it to hypercorn, which I'm planning on doing in the next couple weeks, it will be supported on the latest version of every ASGI server that I know of is actually used in the wild. We should add minimum version numbers to the error messages. If the server doesn't support it users will immediately get an error on startup, so it's not super risky in the sense that it will deterministically cause a deploy to fail immediately with a clear error message.

[Kludex]

Is your suggestion to deprecate app.state?

[adriangb]

Personally, yes. But I don't see the need to push that far, app.state is relatively simple to maintain. I would discourage new users from using it by removing it from the docs and such.

[gnat]

Please, please, avoid deprecating useful features when there is no major benefit to deprecation!

Every deprecation burns out current adopters of Starlette, who took a gamble on Starlette, who then are stuck with:

1. Broken apps; or
2. Non-upgradeable apps.

Just because your current project does not use a specific feature, doesn't mean it should be deleted from Starlette.

I can't believe these deprecation proposals keep happening.

[adriangb]

We aren't planning on deprecating app.state. Unless you're referring to something else?

[Kludex]

As we talked at some point, we are not deprecating app.state, but lifespan state is recommended.