-
-
Notifications
You must be signed in to change notification settings - Fork 377
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: adds before_shutdown/after_startup lifespan hooks #3598
Conversation
am I thinking of something else or did we have these before? |
@JacobCoffee you're right, see #1663. @cofin I'm not sure if this is a good solution or pattern we should support / promote. This would effectively be the same then as adding something to the front of If the goal is to provide a way to say "run this before all IMO having hooks that depend on other hooks is an anti pattern. If your hooks involve state, you should use the lifespan context manager. And if they don't involve state, then the order they're called in shouldn't matter. If the order they are called in does matter, combine them together in a single hook. (same applies to the startup hooks as well) If we do want to support giving priorities to some hooks, then we should implement a priority configuration |
The specific need was While implementing these changes, I remembered us having this support previously, but I don't remember exactly why we removed though. I do think there's a valid case for having a set of actions that can run after specific startup sequences have loaded, so maybe, we should think about a priority/ordering mechanism? |
Definitely. IMO implementing priorities would be the way to go about this. |
Closing for now. |
Description
Closes: #3597
Closes