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

Moved open_session call from RequestContext.push to wsgi_app #1538

Closed
wants to merge 6 commits into from
Closed

Moved open_session call from RequestContext.push to wsgi_app #1538

wants to merge 6 commits into from

Conversation

WeirdCarrotMonster
Copy link

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.

@keyan
Copy link
Contributor

keyan commented Aug 2, 2015

@WeirdCarrotMonster I'm concerned about breaking backwards compatibility. Is there anyway to avoid this? Did you update the docs to reflect your proposed change?

@WeirdCarrotMonster
Copy link
Author

@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 test_request_context and session in their code.

I haven't updated the docs yet, since i hope there is better solution.

@vagamens
Copy link

vagamens commented Aug 3, 2015

would it be possible to reimplement those functions using the newer apis?

On 08/03/2015 03:09 AM, Eugene Protozanov wrote:

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 |test_request_context| and |session| in
their code.

I haven't updated the docs yet, since i hope there is better solution.


Reply to this email directly or view it on GitHub
#1538 (comment).

@ThiefMaster
Copy link
Member

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.

@davidism davidism closed this in 13754b6 Apr 22, 2017
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Application/request stack corruption if unhandled exception is raised in session interface
6 participants