Flask-Admin w/ SQLAlchemy holds a single session for the entire app lifetime?

[samuelhwilliams]

Passing db.session to the admin model views will create a session and then hold that for the lifetime of the app. This feels wrong - it should probably be using request-scoped sessions each time?

TBD further investigation to narrow down if this is a generic sqlalchemy flask-admin issue, or flask-sqlalchemy vs flask-sqlalchemy-lite.

Also to add a reproduction case.

[ElLorans]

My understanding is that db.session is a scoped_session proxy in Flask‑SQLAlchemy, so the ModelView holds a proxy that returns a request‑scoped Session under the hood and Flask‑SQLAlchemy removes it at teardown. https://flask-sqlalchemy.readthedocs.io/en/stable/api/#flask_sqlalchemy.SQLAlchemy.session So I guess it's fine?

[samuelhwilliams]

I believe that's the intent, but not the reality I was seeing earlier. Will try to put up a reproduction over the weekend.

[samuelhwilliams]

alright i'm just curious what copilot would make of this - i'll do it myself if it fails.

Copilot - investigate and produce a reproduction case that either proves or disproves that passing db.session to flask-admin model views is either fine, and will result in request-scoped sessions, or is not fine, and holds a single session for the lifetime or the app.

[samuelhwilliams]

Yeah it looks fine with flask-sqlalchemy, so I reckon it's broken with flask-sqlalchemy-lite specifically. Which is fine since flask-admin doesn't 'officially' support that I guess - but I do actually use FSL so I'd like us to fix this 😅 links in with #2668

[ElLorans]

And yet from the docs, I would expect it to be fine: https://flask-sqlalchemy-lite.readthedocs.io/en/latest/session/#session-management

[samuelhwilliams]

For FSL, db.session is a property that immediately returns an orm.Session instance rather than a proxy to the session that FS gives you, which is a scoped_session wrapper.

FSL docs aren't lying, they just don't clarify that accessing db.session resolves a session immediately, whereas FS provides you an instance that defers session resolution.

I'm unsure if the best place to fix this is Flask-Admin or FSL - if FSL returned a proxy instead of the session directly, it would just work for us. For us to 'fix' it, we'd need to change our documentation to tell users to pass db directly, and then we either call db.session as needed or wrap the session in a proxy ourselves. The latter is probably more straight-forward. We should maybe consider raising a UserWarning if Flask-Admin receives a raw Session object.

[davidism]

I stopped using scoped_session in Flask-SQAlchemy-Lite deliberately. Their docs https://docs.sqlalchemy.org/en/20/orm/contextual.html#unitofwork-contextual make it pretty clear that it has downsides and is only one approach to session lifetime. Since the extension has access to Flask's context lifecycle, that's used directly.

Perhaps you can have code that does "if this is a db object, access db.session, if this is a session object use it directly."

[samuelhwilliams]

Thanks for joining / commenting 🙇 Yeah, I think it makes sense to avoid scoped_session in Flask-SQLAlchemy-Lite based on those SQLAlchemy docs. However, I think Flask-Admin accepting an orm.Session instance directly is likely to be problematic more often than not.

I do thik Flask-Admin should move to recommend passing the (Flask-SQLAlchemy or Flask-SQLAlchemy-Lite) db object directly and then we hit db.session as needed, which FS/FSL will resolve to the correct session of the moment.

I feel like it's a footgun for Flask-Admin to ask users to pass in db.session directly when (gut feeling) most users of Flask will expect request-scoped/context scoped sessions, especially if they're migrating from FS to FSL. I'm not certain there's a valid common use-case for passing an orm.Session instance directly to Flask-Admin, but if we think there's any chance it's needed then we could allow it and just emit a warning to flag the possible problems from doing it.

(At least, I certainly shot myself in the foot by passing FSL's db.session through and it could have very much caused an incident for one of the services I use it with)

[ElLorans]

Related: #2585