-
-
Notifications
You must be signed in to change notification settings - Fork 16.2k
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
Moved open_session call from RequestContext.push to wsgi_app #1538
Conversation
Since open_session call should not happen in RequestContext.push, it needs to be called directly in order to setup session.
@WeirdCarrotMonster I'm concerned about breaking backwards compatibility. Is there anyway to avoid this? Did you update the docs to reflect your proposed change? |
@keyan It's not as compatible as i'd like it to be. The problem is: current approach on managing RequestContext and sessions is wrong. I'm not sure if it can be fixed without breaking compatibility for some people, especially if they use I haven't updated the docs yet, since i hope there is better solution. |
would it be possible to reimplement those functions using the newer apis? On 08/03/2015 03:09 AM, Eugene Protozanov wrote:
|
Also, please clean up the PR a bit. There shouldn't be merge commits in a PR at all (you always rebase feature branches to sync them with upstream) and the various test-related commits could be squashed into a single one. |
|
||
def open_session(self, *args, **kwargs): | ||
if self.fail: | ||
setattr(flask.g, "test_g_attr", 1) |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Fixes #1528
New approach has only one drawback: in any custom tests that require sessions, you need to call
ctx_open_session
directly. I fixed existing tests in flask, but any user test may fail.