pallets
merge app and request contexts into a single context
Right now we have two separate contexts managed separately, the app and request contexts. This makes the implementation pretty complicated, as we need to maintain two context var stacks, and do a bunch of checks in the request context to make sure we're managing the correct app context. It makes an already confusing topic more complicated to explain: app context is active for requests and cli commands, don't push an app context before making a request, etc.
I think merging the two contexts could be possible. The single context (ExecutionContext?) would have the g, request, and session attributes, but accessing request or session when not in a request would raise an error.
I was never clear on why, but the contexts support being pushed multiple times. If the same context is pushed multiple times, the teardown functions are only run once it's fully popped. I don't think I've ever seen this used, and I can't think of a use case. There is a test, but it just demonstrates that the system works, it doesn't demonstrate any intended use of it. There's no docs about it. Perhaps it's a holdover from how things were tracked as a stack on top of thread locals, before things were refactored?
Having to keep track of a stack of how many times a context has been pushed complicates the implementation, especially when the request context also has to track whether it had to push an app context as well or if one was already present. It would be much easier (and probably faster and less memory) to error if the context is currently pushed.
This is not the same as being able to push different contexts on top of each other.
Tentatively marking this for 3.2. I think I can do the merge and have uses of the old contexts issue deprecation warnings and redirect to the new context. Then it can be fully removed in a later release, maybe 4.0.
So far I've still been using the "app context" name for the new merged context. Couldn't think of a better term.
teardown_appcontext and teardown_request need to be combined, but they currently run at different times. teardown_appcontext runs in a finally block after teardown_request was attempted, and after request is no longer bound. The signals appcontext_tearing_down and request_tearing_down share the same timing.
I'd prefer to move away from the squished word "appcontext" to teardown_context and context_tearing_down. It's not clear whether teardown_appcontext or teardown_request is more commonly used right now, so it would probably be equally disruptive either way, since one of them is also getting removed.
Perhaps steal some ideas from dishka
For example request context could only be entered from a active app context
Also a siebling to request context for usage in background workers might be nice to have
To draw a pytest analogy app context is a bit like session scope and request context is a bit like function scope
I might not have the full picture, however I don't see much value in merging them from user's perspective. I fully acknowledge that it might be a bit of a hell from developer's perspective :)
Logically, application and request contexts separate makes sense, maybe out of habit if nothing else.
I was never clear on why, but the contexts support being pushed multiple times.
Far enough along now that I've run into why this was done. As far as I've found this is the only scenario where it's needed, only during testing. When using stream_with_context to re-push the context once reading the streaming response starts, along with with context to preserve the context after the request is over. These two both try to push the same context, first to keep the preserved context active, then to keep the context active during the stream.
Using a single reference to the previous context is indeed much simpler than two contexts each with a stack of references. With only one context, using a stack would be more straightforward than currently, but seems like unnecessary (but very minor) runtime overhead just to support this test scenario. Need to experiment more.
Keeping a simple integer count of how many times the context has been pushed/popped, and returning early if this is not the first push or last pop, fixes the issue.