Skip to content

chore: support sqlalchemy 2.x and flask-sqlalchemy 3 (breaking) #2241

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

Open
wants to merge 102 commits into
base: master
Choose a base branch
from

Conversation

dpgaspar
Copy link
Owner

@dpgaspar dpgaspar commented May 16, 2024

Description

  • Added config option to not create FAB's tables FAB_CREATE_DB
  • Flask-SQLAlchemy singleton is now created by FAB, import from flask_appbuilder.extensions import db. This means init now is simpler: appbuilder.init_app(app)
  • appbuilder.sm.get_session was renamed to appbuilder.sm.session
  • Doing queries now require an app_context (breaking from flask-sqlalchemy)
  • SQLAInterface will not swallow exceptions, and accepts an optional parameter to commit or not commit
  • appbuilder.get_app does not exist anymore, use current_app instead, you can still use appbuilder.app but it's deprecated

ADDITIONAL INFORMATION

  • Has associated issue:
  • Is CRUD MVC related.
  • Is Auth, RBAC security related.
  • Changes the security db schema.
  • Introduces new feature
  • Removes existing feature

@hasansezertasan
Copy link

Related: #2240

@ddorian
Copy link

ddorian commented May 21, 2024

Also related #2038 #2162 #2108 #1940

@dpgaspar dpgaspar marked this pull request as ready for review June 4, 2024 06:16
@dpgaspar dpgaspar changed the title chore: support sqlalchemy 2.x and flask-sqlalchemy 3 chore: support sqlalchemy 2.x and flask-sqlalchemy 3 (breaking) Jun 4, 2024
Copy link

codecov bot commented Apr 13, 2025

Codecov Report

Attention: Patch coverage is 85.53371% with 103 lines in your changes missing coverage. Please review.

Project coverage is 74.21%. Comparing base (c65e067) to head (68be010).
Report is 39 commits behind head on master.

Files with missing lines Patch % Lines
flask_appbuilder/security/sqla/manager.py 73.07% 28 Missing ⚠️
flask_appbuilder/models/sqla/interface.py 84.68% 17 Missing ⚠️
flask_appbuilder/models/sqla/base_legacy.py 72.72% 15 Missing ⚠️
flask_appbuilder/views.py 58.33% 10 Missing ⚠️
flask_appbuilder/api/__init__.py 91.17% 6 Missing ⚠️
flask_appbuilder/base.py 91.48% 4 Missing ⚠️
flask_appbuilder/security/manager.py 95.50% 4 Missing ⚠️
flask_appbuilder/utils/legacy.py 78.94% 4 Missing ⚠️
flask_appbuilder/cli.py 87.50% 3 Missing ⚠️
flask_appbuilder/models/sqla/base.py 78.57% 3 Missing ⚠️
... and 6 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2241      +/-   ##
==========================================
+ Coverage   73.81%   74.21%   +0.39%     
==========================================
  Files          72       76       +4     
  Lines        8754     9206     +452     
==========================================
+ Hits         6462     6832     +370     
- Misses       2292     2374      +82     
Flag Coverage Δ
python 74.21% <85.53%> (+0.39%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ecederstrand
Copy link
Contributor

Just noting here that this ends up limiting us to using older versions of Pandas because pandas.read_sql() now requires SQLAlchemy >= 2.0. We're using Pandas in Airflow DAGs, and apache-airlfow has a dependency on apache-airflow-providers-fab which again has a dependency on Flask-AppBuilder.

def init_app(self, app: Flask) -> None:
session_options = app.config.get("SQLALCHEMY_SESSION_OPTIONS", {})
if session_options:
self.session = self._make_scoped_session(session_options)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @dpgaspar - we are testing the alpha version of FAB 5 for Airlow and we think this one gives us a hard time running our tests.

Basically in our tests we are creating a number of FabAppBuilders (and we override some parts of it) - but we have an issue that when we access a session in pytest (and we run init_app multiple times in the same interpreter - in pytest - every time with a different App, the session that gets created here stays connected - we have > 90 connections to PostgresDB while running tests and we have very hard time on tracking where and how they should be closed.

Does it ring a bell?

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

Successfully merging this pull request may close these issues.

10 participants