john-bodley commented on issue #25108: URL: https://github.com/apache/superset/issues/25108#issuecomment-1703318814
Thanks @craig-rueda for your feedback. In answer to your questions: - I didn't consider an external SQLAlchemy manager outside of Flask-SQLAlchemy which is the defacto session manager for Superset, Flask-AppBuilder, and Flask-Migrate (as defined [here](https://github.com/apache/superset/blob/8b2a408dea5516cc5163446280ad10c165da0f92/requirements/base.txt#L117-L120)). In terms of proxying/postponing the commit we could handle this on a per dependency basis as monkey patching this globally could be difficult. There's a section above which mentions how to deal with Flask-AppBuilder, and (outside of testing and non-Flask templates) it seems like Flask-Migrate [does not](https://github.com/search?q=repo%3Amiguelgrinberg%2FFlask-Migrate%20commit&type=code) explicitly commit. Not doubt there are pros/cons with having an external SQLAlchemy manger, but one significant pro of the Flask-SQLAlchemy approach is nested transactions can be readily adopted by Flask-AppBuilder etc. and seamlessly integrated with Superset given the both share the the same instance of the of the preconfigured scoped session (`db.session`) as described in [SIP-99A](https://github.com/apache/superset/issues/25107). - I did share a [pattern](https://stackoverflow.com/a/38626139) which we could use/augment for testing. There are likely instances where we do need to commit the transaction (due to a subsequent API call) whereas other times having changes persisted to the SQLAlchemy session and simply rolling back is suffice. - Sadly I don't believe that SQLAlchemy supports a "read-only" transaction given that the “virtual” transaction is created automatically when needed, though I like the idea from a readability etc. standpoint. Per a Google search on this topic, one could define a read-only user or configure this at the engine level ([example](https://stackoverflow.com/questions/25904020/how-to-use-read-only-transaction-mode-in-sqlalchemy)). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
