🪛 [Proposal] - Add new on_startup/on_shutdown events

[tarsil]

• Add new way of handling on_startup/on_shutdown by generating a lifepsan event to match the new
  way and handle legacy versions.

A lot of plugins and addons for Starlette and Starlette related frameworks use the old ways to mabage the events. These are still very useful the way they are.

My initial proposal would be to maintain at least the way it is declared for backwards compatibility and internally convert/generate the new async context manager for the lifespan based on that.

This way we keep the newly lifespan approach and different ways of achieving that while maintaining backwards compatibility for the current packages out there.

This also means this would be only for python 3.8+ since python 3.7 EOL will be in June this year.

@Kludex any thoughts?

[adriangb]

I like this idea in general. I think moving the complexity out of App/Router and into a nicely self-contained backward compatibility layer gives us most of the benefit of the deprecation (not needing to worry about the cognitive overhead or breaking tests stemming from multiple code-paths) while reducing the burden on users.

I would still like to remove the arguments from Router, maybe keep them in App and warn if they are used. And also remove these arguments from the documentation which gives users the benefit of the deprecation (they don't need to be confused about multiple ways to do something and new users won't pick the method with more foot guns). The goal would be that end users that have simple use cases and are migrating to 1.0 would have to make no immediate changes. Other libraries or advanced users that are, for example, passing on_startup=... to Router directly would have to insert the compatibility shim themselves.

Regarding this PR, maybe make it a PR against #2068?

Does that sound like a good compromise @Kludex ?

[tarsil]

I like that idea as well. I just ran that locally and it was working as I was thinking but maybe we can use both ways. I will open another PR with a self contained version as suggested and after seeing what you shared. Thank you

[Kludex]

And also remove these arguments from the documentation which gives users the benefit of the deprecation (they don't need to be confused about multiple ways to do something and new users won't pick the method with more foot guns).

Didn't we remove the on_startup and on_shutdown from the documentation already?

The idea here would be to include this on 1.0 or before that?

[tarsil]

@Kludex the idea would be preparing before hand for the 1.0.0. So I would say this is for the 1.0.0 because so far we still have those parameters. Although, this context manager could even be added now to work internally and generate the lifespan from those

[Kludex]

But then I don't understand this: "Regarding this PR, maybe make it a PR against #2068?"

Because that is supposed to land on 1.0, not before.

[tarsil]

Oh that it was a suggestion to do a different approach against that idea as well. So my question now is, what is the best approach here. What is the route, if any, that this should take?

[adriangb]

Didn't we remove the on_startup and on_shutdown from the documentation already?

If we did great 👍, I loose track of these sorts of details 😅.

We can include this as part of #2068 or before it as a standalone change. I’m open to either. In my mind, the former was easier, so I suggested changing the target branch to that PR. I think, including it without removing the existing machinery may be tricky. But again open to either.

[Kludex]

If we did great +1, I loose track of these sorts of details sweat_smile.

We did.

What I had in mind is to merge #2068 to get in 1.0 (not before), and I don't see why the PR that we have here would be included either on 1.0 (since we are removing the feature), or before on 0.2x.y, well, because we intend to remove the feature on 1.0... 🤔

Am I missing something?

[Kludex]

Do we need a new folder here? 🤔

[tarsil]

@Kludex simply providing a way for old packages or widely used packages out there a way to keep having what they have without breaking the whole system because of the new lifespan way of integrating.

Now that I'm thinking, another way of doing this is by actually isolating this functionality in a self contained package and this could serve as a bridge from Starlette 1.0 and the older packages out there

[tarsil]

Now that I'm thinking, maybe this is exactly what I'm gonna do. This way Starlette remains as is and for those who still want to use the old ways (although internally generates the new lifespan) they can simply use that package that will be simply Starlette with that wrapper

[tarsil]

I will be closing this one. It was better to release a different package, independent and self contained applying what was I trying to do it here. This way, the codebase remains clean.

It is here: https://starlette-bridge.tarsild.io/