Skip to content
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

Enhancement: Allow multiple lifecycle hooks of each type to be run on any given request #3752

Open
charles-dyfis-net opened this issue Sep 21, 2024 · 2 comments
Labels
Enhancement This is a new feature or request

Comments

@charles-dyfis-net
Copy link
Contributor

charles-dyfis-net commented Sep 21, 2024

Summary

Only having one lifecycle hook per request makes it impossible to use multiple plugins which both set the same hook, or makes it easy to inadvertently break plugins by overriding a hook that they install and depend on (whether at the root application or a lower layer).

It also places in application code the responsibility of being aware of hooks installed by outer layers, and ensuring that any hooks at inner layers take responsibility for those outer hooks' functionality.

The simplest approach to addressing this is to allow each layer to define a list of hooks, and run all of them for each request (halting when any hook reaches a terminal state, for those hooks where this applies).

Basic Example

Just as guards are defined as a list (where this list is merged between layers to assemble a final list of guards), under this proposal the same would be true of lifecycle hooks.

Drawbacks and Impact

This is a breaking change from an API perspective: Documentation and test cases would need to be updated, plugins or other code assigning lifecycle hooks would need to be modified.

Unresolved questions

Compared to the proposal embodied in #3748, this provides less flexibility -- hooks following that proposal can decide to invoke their parents either before or after themselves, or can modify results returned by parent hooks if they so choose.

However, with this reduction in flexibility there is also a substantive reduction in responsibility: using that proposal, a hook could inadvertently prevent other hooks from running, most concerningly by way of not simply implementing the newer interface.


Note

While we are open for sponsoring on GitHub Sponsors and
OpenCollective, we also utilize Polar.sh to engage in pledge-based sponsorship.

Check out all issues funded or available for funding on our Polar.sh dashboard

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
  • This, along with engagement in the community, helps us know which features are a priority to our users.
Fund with Polar
@charles-dyfis-net charles-dyfis-net added the Enhancement This is a new feature or request label Sep 21, 2024
@provinzkraut
Copy link
Member

How about this:

  • before_request: (request: Request, response: Response | None) -> Response | None. Callables will be reduced. If one hook creates a Response, the following hooks will be able to modify it
  • after_request: (response: Response) -> Response: Same semantics as before_request: Callables will be reduced
  • after_response: (response: Response) -> None: Callables will be mapped and cannot influence each other.

@charles-dyfis-net
Copy link
Contributor Author

How about this:

  • before_request: (request: Request, response: Response | None) -> Response | None. Callables will be reduced. If one hook creates a Response, the following hooks will be able to modify it
  • after_request: (response: Response) -> Response: Same semantics as before_request: Callables will be reduced
  • after_response: (response: Response) -> None: Callables will be mapped and cannot influence each other.

This is all reasonable. If I were designing this myself I might have called the first before_request returning a response terminal rather than invoking the others (even though that would have made behavior exercised in the test suite for #3748 impossible), but that's a place where there are multiple reasonable options -- it's a choice between flexibility (but more room for error) vs deliberate inflexibility (to provide an interface that's hard to misuse), and I think either direction is both workable and defensible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement This is a new feature or request
Projects
None yet
Development

No branches or pull requests

2 participants