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

bug: get_scopefunc returns incorrect greenlet instance for newer flask versions #25

Open
naripok opened this issue Aug 4, 2023 · 0 comments

Comments

@naripok
Copy link

naripok commented Aug 4, 2023

Hey guys, my team may have found a bug in get_scopefunc and we need confirmation.

For context, we're trying to get our test environment to mimic prod as close as possible.
However, there is an unexpected behaviour when using db.session inside a with app.app_context() block.
Naively, we'd expect that a new session would have been pushed inside the new context, as the session is allegedly nested inside and belongs to the app context, so a new app context should mean a new session. However, this is not what we found, and even after context switch, the same session is still in use.

I've pushed a minimal reproducible example here with instructions.

We think the problem lies in the get_scopefunc implementation for newer flask, where instead of having to use the flask-sqlalchemy's original_scopefunc and return an id: int, it actually returns a greenlet instance:

def get_scopefunc(original_scopefunc=None):
    """Returns :func:`.SessionScope`-aware `scopefunc` that has to be used
    during testing.
    """

    if original_scopefunc is None:
        assert flask_sqlalchemy, 'Is Flask-SQLAlchemy installed?'

        try:
            # for flask_sqlalchemy older than 2.2 where the connection_stack
            # was either the app stack or the request stack
            original_scopefunc = flask_sqlalchemy.connection_stack.__ident_func__
        except AttributeError:
            try:
                # when flask_sqlalchemy 2.2 or newer, which supports only flask 0.10
                # or newer, we use app stack
                from flask import _app_ctx_stack
                original_scopefunc = _app_ctx_stack.__ident_func__
            except AttributeError:
                # newer flask does not expose an __ident_func__, use greenlet directly
                import greenlet
                # Should this be `original_scopefunc = lambda: id(greenlet.getcurrent)` or something alike?
                original_scopefunc = greenlet.getcurrent

    def scopefunc():
        rv = original_scopefunc()
        sqlalchemy_scope = _session_scope_stack.top
        if sqlalchemy_scope:
            rv = (rv, id(sqlalchemy_scope))
        return rv

    return scopefunc

We're not 100% sure, mainly because when passing the flask_sqlalchemy's implementation for scopefunc to original_scopefunc param, the tests run as expected (check tests/test_flask_sqlalchemy_get_scopefunc.py in the MRE repo), but when passing what we think would be the correct implementation to the greenlet path, the test still fails (check tests/test_greenlet_get_scopefunc.py in the MRE repo).

It would be great to have at least confirmation that this is the real problem, if possible.
I'm aware this explanation is not top notch, but I'm available for helping with whatever is needed here. Hopefully there is enough context for you guys to understand it. 😅

Thanks for the great module, BTW. 😄

Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant