merge app and request contexts into a single context
Opened this issue · 3 comments
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.