chore(sqla2): bump SQLAlchemy-dependent drivers to dual-compat versions - #42542
Conversation
Code Review Agent Run #4fe897Actionable Suggestions - 0Additional Suggestions - 1
Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
|
Another incremental step, @hy144328 :) |
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Prepares for the SQLAlchemy 2.0 migration by bumping SQLAlchemy-dependent packages to dual-compatible versions and surfacing dialects/drivers that are known to be incompatible with SQLAlchemy 2.0 in generated DB docs.
Changes:
- Add
known_incompatibilitiesmetadata to engine specs (and compatible DB entries) and populate it for Solr, Kusto, D1, and Aurora Data API. - Pin
flask-sqlalchemyexplicitly to the last dual-compatible release line and bump/select dialect driver versions accordingly. - Update compiled requirement pins to reflect the new dependency constraints.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| superset/db_engine_specs/base.py | Introduces KnownIncompatibility + adds known_incompatibilities to metadata TypedDicts |
| superset/db_engine_specs/solr.py | Flags Solr SQLAlchemy 2.0 incompatibility metadata |
| superset/db_engine_specs/kusto.py | Flags Kusto SQLAlchemy 2.0 incompatibility metadata |
| superset/db_engine_specs/d1.py | Flags D1 SQLAlchemy 2.0 uncertainty/incompatibility metadata |
| superset/db_engine_specs/postgres.py | Flags Aurora Data API incompatibility on Postgres-compatible entry |
| superset/db_engine_specs/mysql.py | Flags Aurora Data API incompatibility on MySQL-compatible entry |
| requirements/base.txt | Updates compiled pin for flask-sqlalchemy |
| requirements/development.txt | Updates compiled pins for flask-sqlalchemy / sqlalchemy-bigquery |
| pyproject.toml | Adds explicit flask-sqlalchemy pin and adjusts optional dialect dependency constraints |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #42542 +/- ##
=======================================
Coverage 65.57% 65.57%
=======================================
Files 2818 2818
Lines 160038 160046 +8
Branches 36557 36557
=======================================
+ Hits 104938 104946 +8
Misses 53054 53054
Partials 2046 2046
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Code Review Agent Run #4f1864Actionable Suggestions - 0Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
Code Review Agent Run #d22c6cActionable Suggestions - 0Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Code Review Agent Run #4f8b99Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
Prep work for the SQLAlchemy 1.4 -> 2.0 core bump (discussion #40273, step 6). Every SQLAlchemy-dependent package Superset ships or offers as an optional extra gets bumped to the newest release that supports both SQLAlchemy 1.4 and 2.0, so each bump can be merged and tested independently under today's SQLAlchemy 1.4 pin before the core bump lands as its own final PR. - flask-sqlalchemy: pin explicitly (was purely transitive via flask-appbuilder, resolving to 2.5.1, 1.4-only) at >=3.0.5,<3.1, the last release before 3.1.0 drops 1.4 support. flask-appbuilder and marshmallow-sqlalchemy are already dual-compatible as pinned, no change needed. - sqlalchemy-bigquery: 1.17.0 -> 1.17.1 (patch; the project was archived 2026-05-16, likely its final release). - sqlalchemy-cratedb: 0.41.0 -> 0.43.1. - sqlalchemy-drill: drop an artificial <2 SQLAlchemy-version ceiling upstream never had (1.1.10 already supports 2.0 via import_dbapi). - firebolt-sqlalchemy: 1.0.0 -> 1.1.2. - sqlalchemy-ocient: 1.0.0 -> 3.0.0 (lower confidence: closed-source vendor package, no public changelog to verify against). - snowflake-sqlalchemy floor was already bumped independently by dependabot (#42418) to the same target this research landed on. Five extras (risingwave, exasol, firebird, redshift, dremio) have NO release supporting both SQLAlchemy versions -- version numbers cut straight from 1.4-only to 2.0-only with no overlap. Pinned each to its last 1.4-only version with a comment noting the 2.0 target to bump to once the core sqlalchemy bump lands, not before. Four extras have no SQLAlchemy 2.0 support anywhere in their ecosystem today and are flagged via a new `known_incompatibilities` metadata field (surfaced through the generated database docs): kusto, aurora-data-api (including our own dormant preset-io fork), solr, and d1. sqlalchemy-vertica-python was initially suspected to be a fifth, but turned out to already be dual-compatible (v0.6.3 already ships `import_dbapi`/`text()` wrapping) once the actual merged PR was checked instead of relying on PyPI metadata alone. Verified locally: full dependency resolution succeeds, the app boots and creates all tables cleanly under SQLAlchemy 1.4.54 + Flask-SQLAlchemy 3.0.5, 483/487 relevant unit tests pass (the 4 failures are a pre-existing local mysqlclient linkage issue, unrelated to this change), and the new metadata flows through generate_yaml_docs() end-to-end for all four flagged engines.
Flask-SQLAlchemy 3.0 stopped defaulting SQLite engines to NullPool when pool_size isn't set (changelog: "SQLite engines do not use NullPool if pool_size is 0"). Superset never configures pool_size, so under 2.5.1 this was silently applying; under 3.0.5 it falls back to QueuePool instead, which holds connections open. With a file-based SQLite db shared between the test process and the Celery worker, and ENABLE_VERSIONING_CAPTURE adding a version_transaction write to every commit, held-open pooled connections caused a wave of "database is locked" errors in test-sqlite CI (19 failed, 47 errors) that don't reproduce on master.
Real CI (test-sqlite, test-mysql) surfaced a structural incompatibility between Flask-SQLAlchemy 3.0.5's session/app-context scoping and how Superset currently crosses context boundaries (Celery tasks, background jobs): widespread "NoneType has no attribute X" failures across unrelated test files (dashboards, embedded views, security, tasks) plus MySQL "Lock wait timeout exceeded" errors. Tried restoring NullPool for SQLite (Flask-SQLAlchemy 3.0 stopped defaulting SQLite to NullPool when pool_size is unset) since that matched the initial "database is locked" symptom -- it had zero effect (identical failure count with and without it), confirming the locking was a downstream symptom of the deeper session-scoping issue, not a connection-pool problem. Reverting that fix along with the bump. Without an explicit upper bound, uv's resolver naturally re-picks 3.0.5 on any lockfile regen (flask-appbuilder allows <4), so this pins <3.0 explicitly rather than just removing the forcing pin. This needs dedicated investigation into Superset's session/app-context handling, not a bump slipped into driver-compat prep. Revisit alongside the actual SQLAlchemy 2.0 core bump (discussion #40273, step 6) -- Flask-SQLAlchemy 3.1+ requires SQLAlchemy 2.0 anyway, so this was always going to need to happen around the same time.
Extract the duplicated Aurora Data API known_incompatibilities block from mysql.py/postgres.py into a shared base.py constant, and drop the stray space in the sqlalchemy-solr version pin for PEP 508 consistency. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… fallback, correct d1 SQLAlchemy 2.0 claim - Add the TypeScript contract for `known_incompatibilities` and render it on the generated database docs pages (both top-level and within each compatible-database entry), so the field added for the Aurora Data API / D1 SQLAlchemy 2.0 warnings is actually visible instead of just reaching the JSON unused. - Teach the AST-based docs fallback extractor to resolve module-level literal constants (e.g. AURORA_DATA_API_KNOWN_INCOMPATIBILITIES) so a metadata dict that references a shared constant by name no longer serializes as the bare identifier string when Flask context isn't available. - Correct the d1.py known_incompatibilities reason: sqlalchemy-d1's only release (0.1.0) pins `sqlalchemy<2,>=1.4`, which explicitly excludes SQLAlchemy 2.0 rather than merely lacking confirmation either way. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
c9b1c94 to
7045efc
Compare
Code Review Agent Run #c5cf87Actionable Suggestions - 0Additional Suggestions - 2
Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
… MERGE) Investigation-only, cherry-picked from mikebridge's test/verify-sqlalchemy-continuum branch (backend-relevant hunks only, skipping unrelated frontend formatting drift). Bumps sqlalchemy to 2.0.51 and flask-sqlalchemy to 3.1.1 directly, rather than the FSA 3.0.5 intermediate step attempted in PR #42542. Notable fix beyond the version bumps: pessimistic_connection_handling (superset/utils/core.py) now calls connection.rollback() after its pool-checkout health-check SELECT. Under SQLAlchemy 2.0's autobegin behavior that SELECT implicitly opens a transaction it previously never closed, which is a plausible mechanism for the MySQL lock-wait-timeout symptom from the original #42542 break. Also updates two migration scripts (2018-07-26 add_implicit_tags, 2022-04-01 new_dataset_models_take_2) to Mapped[] typed relationship annotations -- these define their own standalone declarative models and were missed by discussion #40273's earlier unit-test-driven deprecation-warning sweep, since migration scripts aren't exercised by that suite. Removes the now-obsolete SQLALCHEMY_WARN_20 pytest.ini filterwarnings error lines, since sqlalchemy.exc.RemovedIn20Warning doesn't fire (or exist in the same form) once SQLAlchemy 2.0 is actually installed. Pushing to get a real test-sqlite/test-mysql CI signal, since local sqlite runs don't reliably reproduce the original break either way.
Root cause of the embedded-dashboard/task-framework test failures: Flask-SQLAlchemy 3.x scopes db.session by the identity of the active Flask app-context object (id(app_ctx)) instead of by thread/greenlet identity like 2.x. Superset's codebase and test fixtures widely (and often only implicitly, without an explicit commit()) assume a single shared session per thread across nested app.app_context() blocks - true under 2.x, false under 3.x once any code pushes a second context on the same thread (e.g. the AppContextTask Celery wrapper below, or test fixtures that each do their own `with app.app_context():`). Verified locally (real flask-sqlalchemy==3.1.1 + sqlalchemy==2.0.51, sqlite backend) against the exact tests failing in CI: restoring the 2.x scopefunc fixes tests/integration_tests/embedded/test_view.py (the original symptom from the first #42542 break), tasks/test_event_ handlers.py, tasks/test_timeout.py, and one of two tasks/test_ throttling.py failures. Also guard the Celery AppContextTask wrapper's app_context() push with has_app_context(), so eager-mode task execution (e.g. .apply() called from an existing request/test context) reuses the caller's session instead of unconditionally splitting into a second, blind one - the most impactful single instance of the pattern, since every task run goes through it. Two failures remain locally, both pre-existing and unrelated to this scoping issue (confirmed present in CI before this fix too): dashboards/soft_delete_tests.py's db.session.bind is None under FSA 3.x/SQLA 2.0, and a sqlite-specific check_same_thread cross-thread connection reuse in test_throttling.py's timer-thread path.
…sn't Re-read PR #8's actual body instead of relying on a summary -- Devin had already found the flask-sqlalchemy pin comment, traced it to apache/superset#42542, and explicitly flagged that the MySQL/Celery CI matrix (unavailable on this fork) was the real verification gate. The orchestrator's old poll logic just saw "a PR exists" and labeled it devin-pr-open regardless, discarding that caveat. Corrected the README and Loom outline to reflect that the gap was in the harness collapsing a hedged verdict into a binary label, not in Devin's investigation -- a more accurate (and sharper) story for why structured_output_schema matters. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… MERGE) Investigation-only, cherry-picked from mikebridge's test/verify-sqlalchemy-continuum branch (backend-relevant hunks only, skipping unrelated frontend formatting drift). Bumps sqlalchemy to 2.0.51 and flask-sqlalchemy to 3.1.1 directly, rather than the FSA 3.0.5 intermediate step attempted in PR #42542. Notable fix beyond the version bumps: pessimistic_connection_handling (superset/utils/core.py) now calls connection.rollback() after its pool-checkout health-check SELECT. Under SQLAlchemy 2.0's autobegin behavior that SELECT implicitly opens a transaction it previously never closed, which is a plausible mechanism for the MySQL lock-wait-timeout symptom from the original #42542 break. Also updates two migration scripts (2018-07-26 add_implicit_tags, 2022-04-01 new_dataset_models_take_2) to Mapped[] typed relationship annotations -- these define their own standalone declarative models and were missed by discussion #40273's earlier unit-test-driven deprecation-warning sweep, since migration scripts aren't exercised by that suite. Removes the now-obsolete SQLALCHEMY_WARN_20 pytest.ini filterwarnings error lines, since sqlalchemy.exc.RemovedIn20Warning doesn't fire (or exist in the same form) once SQLAlchemy 2.0 is actually installed. Pushing to get a real test-sqlite/test-mysql CI signal, since local sqlite runs don't reliably reproduce the original break either way.
Root cause of the embedded-dashboard/task-framework test failures: Flask-SQLAlchemy 3.x scopes db.session by the identity of the active Flask app-context object (id(app_ctx)) instead of by thread/greenlet identity like 2.x. Superset's codebase and test fixtures widely (and often only implicitly, without an explicit commit()) assume a single shared session per thread across nested app.app_context() blocks - true under 2.x, false under 3.x once any code pushes a second context on the same thread (e.g. the AppContextTask Celery wrapper below, or test fixtures that each do their own `with app.app_context():`). Verified locally (real flask-sqlalchemy==3.1.1 + sqlalchemy==2.0.51, sqlite backend) against the exact tests failing in CI: restoring the 2.x scopefunc fixes tests/integration_tests/embedded/test_view.py (the original symptom from the first #42542 break), tasks/test_event_ handlers.py, tasks/test_timeout.py, and one of two tasks/test_ throttling.py failures. Also guard the Celery AppContextTask wrapper's app_context() push with has_app_context(), so eager-mode task execution (e.g. .apply() called from an existing request/test context) reuses the caller's session instead of unconditionally splitting into a second, blind one - the most impactful single instance of the pattern, since every task run goes through it. Two failures remain locally, both pre-existing and unrelated to this scoping issue (confirmed present in CI before this fix too): dashboards/soft_delete_tests.py's db.session.bind is None under FSA 3.x/SQLA 2.0, and a sqlite-specific check_same_thread cross-thread connection reuse in test_throttling.py's timer-thread path.
SUMMARY
See #40273 (SQLAlchemy 2.0 migration battleplan). Steps 1-5 of the battleplan are done as of this week (#42277, #42365 merged,
__allow_unmapped__landed in #41179). This PR is prep for step 6, the actual coresqlalchemybump: it bumps every SQLAlchemy-dependent driver/framework package Superset ships (default install + every optional dialect extra) to the newest version that supports both SQLAlchemy 1.4 and 2.0, so each driver bump can be merged and tested independently under today's SQLAlchemy 1.4 pin, before the core bump happens as its own final PR.flask-sqlalchemyis explicitly NOT bumped here, on purpose — see below. It's the one package in this whole sweep that looked simple on paper but turned out not to be.Small, safe bumps:
sqlalchemy-bigquery1.17.0 → 1.17.1 (patch;googleapis/python-bigquery-sqlalchemywas archived 2026-05-16, likely its final release)sqlalchemy-cratedb0.41.0 → 0.43.1sqlalchemy-drill: dropped an artificial<2SQLAlchemy-version ceiling upstream never had (1.1.10 already shipsimport_dbapi)firebolt-sqlalchemy1.0.0 → 1.1.2sqlalchemy-ocient1.0.0 → 3.0.0 (lower confidence — closed-source vendor package, no public changelog to verify against)snowflake-sqlalchemy's floor was already independently bumped by dependabot (chore(deps-dev): update snowflake-sqlalchemy requirement from <2,>=1.10.2 to >=1.11.0,<2 #42418) to the same target this research landed on — nothing to do there.alembicandflask-migrateare both unbounded on SQLAlchemy already — no changes needed.Five extras skipped for now — no dual-compat version exists:
sqlalchemy-risingwave,sqlalchemy-exasol,sqlalchemy-firebird,sqlalchemy-redshift, andsqlalchemy-dremio's newest release all cut hard from SQLAlchemy-1.4-only straight to 2.0-only with zero overlapping version. Each is pinned to its last 1.4-only release with a comment explaining why, and the SQLAlchemy-2.0 target version to bump to once the core sqlalchemy bump lands, not before.Four extras flagged as currently unsupported under SQLAlchemy 2.0:
sqlalchemy-kusto,sqlalchemy-aurora-data-api(including our own dormantpreset-iofork),sqlalchemy-solr, andsqlalchemy-d1have no SQLAlchemy 2.0 support anywhere in their ecosystem today. Rather than just noting this in the PR description, added a newknown_incompatibilitiesfield toDBEngineSpecMetadata(superset/db_engine_specs/base.py) that flows through to the auto-generated database docs, so this is visible to anyone looking at those DBs' docs pages, not just buried in a merged PR. Checked each project's repo (not just PyPI metadata) for a path forward:setup.pyonmainhard-pinssqlalchemy==1.4.*— worth filing an issue asking their plans.preset-io/sqlalchemy-aurora-data-api) is dead since 2021. The more active community fork (cloud-utils/sqlalchemy-aurora-data-api) has an open, unresolved SQLAlchemy 2.0 issue (#43) — since we already own a fork here, this is the one case where contributing a patch is realistically in our own hands rather than waiting on a stranger.One correction worth flagging:
sqlalchemy-vertica-pythonwas initially flagged as a fifth unsupported case based on PyPI metadata alone, but turned out to already be dual-compatible — the actual merged upstream PR (bluelabsio/sqlalchemy-vertica-python#18) landed in the current pin (0.6.3) and addsimport_dbapiwhile keeping the 1.4-styledbapialias. No change needed there; removed from the flagged list once verified against the actual source.Why
flask-sqlalchemyisn't bumped hereIt looked like the one bump that mattered: purely transitive via
flask-appbuilder(which allows<4), souv's resolver naturally re-picks the newest allowed release —3.0.5, the last one before 3.1.0 drops SQLAlchemy 1.4 support — on any lockfile regen.flask-appbuilderandmarshmallow-sqlalchemyare already dual-compatible as pinned. Local verification (clean venv, app boot,db.create_all(), 483/487 relevant unit tests) all looked fine.Real CI told a different story.
test-sqliteandtest-mysqlboth failed with a pattern that has nothing to do with driver compatibility: widespreadAttributeError: 'NoneType' object has no attribute 'embedded'/'schema'/'dialect'across unrelated test files (dashboards, embedded views, security, tasks), plus MySQLLock wait timeout exceedederrors. That's consistent with Flask-SQLAlchemy 3.0's changelog — "the session is scoped to the current app context instead of the thread" — interacting badly with how Superset currently crosses context boundaries (Celery tasks, background jobs).The
test-sqlitefailures initially looked like a narrower, fixable issue: FSA 3.0 also stopped defaulting SQLite engines toNullPoolwhenpool_sizeis unset, and Superset'stest-sqlitejob runs a separate Celery worker process against the same file-based SQLite db, which is exactly the scenarioNullPoolprotects. Tried restoring it explicitly — it had zero effect (identical failure count, 19 failed / 47 errors, with and without it), confirming the locking was a downstream symptom of the deeper session-scoping issue, not a connection-pool problem on its own.So:
flask-sqlalchemyis pinned explicitly at>=2.5.1, <3.0(not just left unpinned — without the explicit cap,uvre-resolves to 3.0.5 on its own). This needs dedicated investigation into Superset's session/app-context handling across Celery boundaries, not a bump slipped into driver-compat prep. It'll need to happen anyway around the same time as step 6, since Flask-SQLAlchemy 3.1+ requires SQLAlchemy 2.0.TESTING INSTRUCTIONS
requirements/base.txtandrequirements/development.txtregenerated via./scripts/uv-pip-compile.sh(the official Docker-based compile process) — full dependency resolution succeeds with no conflicts.known_incompatibilitiesmetadata flows throughgenerate_yaml_docs()end-to-end for all four flagged engines (Solr, D1, Kusto, and the Aurora entries nested under MySQL/Postgrescompatible_databases).pre-commit(mypy, ruff, pylint, thedb-engine-spec-metadatavalidator) all pass.flask-sqlalchemyissue described above (see run history on this PR).FOLLOW-UP (not in this PR)
dodopizza/sqlalchemy-kustoasking about SQLAlchemy 2.0 plans.preset-io/sqlalchemy-aurora-data-apifork directly, or contribute tocloud-utils/sqlalchemy-aurora-data-apiissue Supporting arbitrary expressions #43 instead.sqlalchemy-solrandsqlalchemy-d1are lower priority — watch for upstream activity.next-version leg), so step 6 gets continuous signal before anyone commits to the actual core bump.risingwave/exasol/firebird/redshift/dremioceilings documented here need to flip to their 2.0-only versions in lockstep with the coresqlalchemybump itself (step 6), not before.ADDITIONAL INFORMATION