Dependency on Flask-SQLAlchemy

[connorbrinton]

Hi Jean,

As far as I can tell, I don't see any part of the code here that actually requires Flask-SQLAlchemy. I'm currently using it in a project that doesn't use Flask-SQLAlchemy without any problems (Thanks! 😄). However, because the project depends on the Flask-SQLAlchemy package, it gets installed along with my package even though it's not necessary.

I was also hesitant to use the project at all, since I initially thought it would be Flask-SQLAlchemy-specific.

Would it be possible to remove the dependency on Flask-SQLAlchemy, or could you point out the part of the code that depends on it?

Thanks!

[connorbrinton]

I've determined that the exact spots that depend on Flask-SQLAlchemy objects are:

connection = _db.engine.connect()

And:

session = _db.create_scoped_session(options=options)

This conflicts with the documentation, which states that "the _db fixture should expose access to a valid SQLAlchemy Session object that can interact with your database", implying that any valid SQLAlchemy Session object will do. In fact, only those which have an engine attribute and a create_scoped_session function will work, which is likely to be only Flask-SQLAlchemy objects.

The documentation should be updated to reflect the fact that this project does not support arbitrary SQLAlchemy Session objects. Additionally, the dependency on Flask-SQLAlchemy should be removed, since the module is never imported.

[jeancochrane]

Great points! Removing the flask-sqlalchemy dependency and updating the docs should be straightforward.

Do you have a sense of how hard it would be to create equivalent connection and session objects with the vanilla SQLAlchemy API? If making the module work for SQLAlchemy more generally is a two-line change, I'd be happy to move forward on that immediately.

[connorbrinton]

I don't think it would be too difficult.

It looks like there's some extra locking logic around connection creation in flask-sqlalchemy, but it shouldn't be relevant for most applications. I think a similar connection object could be created by just calling engine.connect().

And the session object from flask-sqlalchemy is created more or less as:

from sqlalchemy.orm import scoped_session, sessionmaker
session = scoped_session(sessionmaker(bind=engine))

Along with some flask-sqlachemy-specific event handling logic.

Theoretically, adapting pytest-flask-sqlalchemy to work with vanilla SQLAlchemy objects would be as easy as allowing users to return an engine from the _db fixture and then creating the connection and session objects according to the returned object's shape.

P.S. Sorry for the slow response, I've been visiting family for the holidays 🙂

[jpmckinney]

@connorbrinton Shouldn't it be:

session = scoped_session(sessionmaker(bind=connection))

As for _db.engine.connect(), if _db returned an engine, instead of a SQLAlchemy instance, we could just replace that with _db.connect(), I think?

If so, to preserve backwards compatibility, you can perhaps test whether _db is a SQLAlchemy instance, and if not, then, just use it as-is.

[connorbrinton]

@jpmckinney Great question 🙂 I have to admit that I don't totally understand everything that's going on with relation to bind=connection vs bind=engine 😅 I think bind=connection is nice because it makes it explicit that we're only using a single, transactionally isolated connection. However, I think it might be necessary to mock connection.engine before passing it to sessionmaker 🤔

I originally wrote bind=engine based on the behavior of Flask-SQLAlchemy (see this line). Normally using bind=engine would cause problems, since the engine could be used to create a new connection outside of the transactional scope. However, pytest-flask-sqlalchemy overrides FlaskSQLAlchemy's default bind argument here, so you're totally right that bind=connection would be better for an implementation independent of FlaskSQLAlchemy 😄

Interestingly, it seems like that original bind=connection gets replaced with bind=mocked_engine in this fixture. The mocked engine redirects all new connection requests to the single existing connection. I'm not totally sure why it's necessary to create a new mocked engine though, if the session is already bound to a single connection. Maybe SQLAlchemy tries to access the original engine through the connection otherwise?

[jpmckinney]

I ended up switching to Django (for other reasons) for my project, but I think you're right that there's more work to do around the engine.