fix(deps): roll back SQLAlchemy 2.0 upgrade

[aminghadersohi]

SUMMARY

Reverts #42803 (8014f782d3c702487c6afaa40e2389210a915f91), restoring the dependency set and compatible behavior from immediately before the SQLAlchemy 2.0 / Flask-SQLAlchemy 3.1.1 upgrade:

• SQLAlchemy 1.4.54
• Flask-SQLAlchemy 2.5.1
• SQLAlchemy 1.4-compatible database dialect bounds
• the pre-upgrade ORM, session-scoping, migration, URL-rendering, and result-row behavior
• SQLAlchemy 2.0 deprecation warnings used to catch incompatible code before a future upgrade

This is a semantic revert of the exact squash commit, rebased onto current master. It preserves the 88 commits merged after #42803 rather than resetting affected files wholesale.

requirements/base.txt, requirements/development.txt, and requirements/translations.txt were regenerated with the repository's ./scripts/uv-pip-compile.sh Docker/Python 3.11 workflow.

The revert applied without textual conflicts. Post-#42803 changes with plausible SQLAlchemy coupling were reviewed and retained:

• [Bug] import_tag reuses a poisoned SQLAlchemy Session after concurrent tag conflict #42912 / fix(import): isolate per-tag import in a SAVEPOINT to avoid poisoned session (#42912) #42920 (4684851336), nested SAVEPOINT handling: supported by SQLAlchemy 1.4 and covered by tests.
• fix(mcp): give concurrent tool calls isolated db sessions #42629 (d0658bacc8), MCP per-call session isolation: explicitly supports Flask-SQLAlchemy 2.5.1 and is covered by tests.
• chore(deps-dev): bump sqlalchemy-bigquery from 1.17.1 to 1.17.2 #43209 (2773bc94eb), sqlalchemy-bigquery 1.17.2: resolves against SQLAlchemy 1.4.54.
• chore(duckdb): remove cursor.description workaround, no longer reproducible #43101 (3fb58900b9), DuckDB cursor workaround removal: real-cursor test passes with SQLAlchemy 1.4.54, duckdb-engine 0.17.0, and duckdb 1.5.5.

No post-#42803 commit required an additional compatibility change.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Not applicable; backend dependency rollback only.

TESTING INSTRUCTIONS

Dependency artifacts:

./scripts/uv-pip-compile.sh

Targeted PR compatibility tests:

pytest -q \
  tests/unit_tests/extensions/test_sqlalchemy.py \
  tests/unit_tests/commands/dataset/test_duplicate.py \
  tests/unit_tests/commands/importers/v1/examples_test.py \
  tests/unit_tests/databases/filters_test.py \
  tests/unit_tests/db_engine_specs/test_snowflake.py \
  tests/unit_tests/db_engine_specs/test_trino.py \
  tests/unit_tests/sql_lab_test.py
# 175 passed

Post-merge session/SAVEPOINT compatibility:

pytest -q \
  tests/unit_tests/charts/commands/importers/v1/import_test.py \
  tests/unit_tests/mcp_service/test_session_scope.py \
  tests/unit_tests/mcp_service/test_auth_user_resolution.py
# 52 passed, 4 skipped

DuckDB compatibility:

pytest -q tests/unit_tests/db_engine_specs/test_duckdb.py
# 9 passed

Pre-commit checks:

pre-commit run --from-ref origin/master --to-ref HEAD
# passed

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[bito-code-review]

Code Review Agent Run #8b0ed2

Actionable Suggestions - 0

Additional Suggestions - 9

• superset/views/core.py - 1

  • CWE-252: Missing persistent state commit · Line 465-469
    Removing `db.session.add(dash)` breaks the new-dashboard creation path. The `Dashboard.slices` relationship uses default `cascade_backrefs=True` which does not reliably persist transient Dashboard objects when `dash.slices.append(slc)` is called. This will silently fail to save newly created dashboards — users will see no error but the dashboard won't exist after the request. Restore the explicit `db.session.add(dash)` call.
• superset/utils/core.py - 2

  • Missing rollback after successful ping · Line 819-819
    `pessimistic_connection_handling` is an `engine_connect` event handler that pings connections on checkout. When `connection.scalar(select(1))` succeeds (line 818), the SELECT runs inside an implicit transaction. In SQLAlchemy 1.4 with `autocommit=False`, that transaction persists until explicitly rolled back or committed — meaning the connection returns to the pool with an open transaction rather than a clean idle state. This can cause subsequent operations to see 'current transaction is aborted' errors or unexpected lock contention. The `rollback()` call was introduced in PR feat: bump SQLAlchemy to 2.0 and flask-sqlalchemy to 3.1.1 #42803 (SQLAlchemy 2.0 bump) and its removal by the revert may be unintended, since the revert only claims to roll back the version bump, not the behavioral fix.
  • Missing rollback after re-validation ping · Line 832-832
    On the `err.connection_invalidated` re-validation path (line 830), `connection.scalar(select(1))` re-validates the connection. After a successful re-validation, the connection still carries an open transaction. If the exception is re-raised on the `else` branch of a surrounding try/except at a higher stack level, the connection will be returned to the pool with an unresolved transaction. This creates the same 'current transaction is aborted' failure mode for the next caller of this connection.
• superset/daos/dashboard.py - 1

  • Inconsistent ID population by path · Line 535-535
    Removing the explicit `flush()` makes `dash.id` availability dependent on implicit autoflush triggers. The deleted comments explicitly warned that whether the ID was usable was 'an accident of whatever query the caller happened to run afterward.' The `duplicate_slices=True` path internally flushes per-cloned-slice (line 519) and can trigger autoflush when assigning `new_slice.dashboards.append(dash)`, while the `duplicate_slices=False` path relies solely on autoflush. Callers using the non-flush path may observe `dash.id == None` if they don't trigger autoflush before accessing it.
• superset/tasks/context.py - 2

  • Lost conditional context logic · Line 256-256
    Removing the `has_app_context()` check changes behavior when called from a context where app context already exists. The old code avoided pushing a nested context; the new code always pushes one. While Flask handles nested contexts, this change is not semantically equivalent.
  • Unconditional nested app context · Line 477-479
    The `_trigger_cleanup_handlers` method now unconditionally pushes a nested app context. If this method is called from a path that already has an app context (e.g., from within `app_context()` in line 260), this creates nested contexts. Verify this is the intended behavior.
• superset/db_engine_specs/databend.py - 1

  • Forward-compatibility with SQLAlchemy 2.0 · Line 285-297
    Removing `.render_as_string(hide_password=False)` makes the code fragile for SQLAlchemy 2.0 re-upgrade. In 1.4, `str(URL)` hides passwords by default, so this change is functionally equivalent now—but if Superset ever re-upgrades to SQLAlchemy 2.0, the Databend connection will silently fail since `str(URL)` will hide the password. The `sqlalchemy_uri_decrypted` property (core.py:1392) already uses the explicit form for comparison. Consider keeping the explicit call or at minimum the comment explaining the version-specific behavior.
• tests/unit_tests/extensions/test_sqlalchemy.py - 1

  • Duplicated URL constant · Line 285-290
    The URL `https://sqlalche.me/e/14/f405` appears identically in two separate test assertions (lines 289 and 381). Consider extracting this to a module-level constant for maintainability.
• superset/db_engine_specs/clickhouse.py - 1

  • Code clarity improvement · Line 490-500
    The removed comment explained SQLAlchemy 2.0's password-obfuscation change. Consider adding a brief note that str() is used intentionally (identical to render_as_string in 1.4.x) for consistency with the base class pattern at base.py:2921.

Filtered by Review Rules

Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.

• requirements/base.txt - 1

  • SQLAlchemy version conflict with pandas · Line 384-384
• superset/migrations/versions/2020-04-24_10-46_e557699a813e_add_tables_relation_to_row_level_.py - 2

  • SQLAlchemy MetaData.bind deprecated · Line 38-38
  • SQLAlchemy MetaData.bind deprecated · Line 71-71
• superset/db_engine_specs/couchbase.py - 1

  • CWE-755: Missing Password in URI · Line 179-179
• tests/unit_tests/db_engine_specs/test_trino.py - 1

  • CWE-561: Dead Code in Test · Line 847-847
• tests/unit_tests/db_engine_specs/test_snowflake.py - 1

  • Test assertion will fail · Line 219-219
• superset/daos/dataset.py - 1

  • Deprecated Query.get() API · Line 633-633

Review Details

• Files reviewed - 56 · Commit Range: ec621b3..ec621b3

  • pyproject.toml
  • pytest.ini
  • requirements/base.txt
  • requirements/development.txt
  • superset-core/pyproject.toml
  • superset/cli/export_example.py
  • superset/commands/dataset/duplicate.py
  • superset/commands/importers/v1/examples.py
  • superset/connectors/sqla/models.py
  • superset/daos/dashboard.py
  • superset/daos/dataset.py
  • superset/databases/utils.py
  • superset/db_engine_specs/base.py
  • superset/db_engine_specs/clickhouse.py
  • superset/db_engine_specs/couchbase.py
  • superset/db_engine_specs/databend.py
  • superset/db_engine_specs/databricks.py
  • superset/db_engine_specs/duckdb.py
  • superset/db_engine_specs/snowflake.py
  • superset/extensions/__init__.py
  • superset/initialization/__init__.py
  • superset/mcp_service/chart/preview_utils.py
  • superset/migrations/versions/2018-07-26_11-10_c82ee8a39623_add_implicit_tags.py
  • superset/migrations/versions/2020-01-08_01-17_e96dbf2cfef0_datasource_cluster_fk.py
  • superset/migrations/versions/2020-04-24_10-46_e557699a813e_add_tables_relation_to_row_level_.py
  • superset/migrations/versions/2020-09-15_18-22_e5ef6828ac4e_add_rls_filter_type_and_grouping_key.py
  • superset/migrations/versions/2020-09-24_12-04_3fbbc6e8d654_fix_data_access_permissions_for_virtual_.py
  • superset/migrations/versions/2020-09-28_17-57_b56500de1855_add_uuid_column_to_import_mixin.py
  • superset/migrations/versions/2021-02-04_09-34_070c043f2fdb_add_granularity_to_charts_where_missing.py
  • superset/migrations/versions/2021-02-18_09-13_c501b7c653a3_add_missing_uuid_column.py
  • superset/migrations/versions/2022-04-01_14-38_a9422eeaae74_new_dataset_models_take_2.py
  • superset/models/core.py
  • superset/models/sql_types/presto_sql_types.py
  • superset/security/manager.py
  • superset/tasks/context.py
  • superset/tasks/manager.py
  • superset/utils/core.py
  • superset/views/core.py
  • tests/integration_tests/charts/api_tests.py
  • tests/integration_tests/dashboards/api_tests.py
  • tests/integration_tests/dashboards/soft_delete_tests.py
  • tests/integration_tests/datasource_tests.py
  • tests/integration_tests/db_engine_specs/hive_tests.py
  • tests/integration_tests/db_engine_specs/presto_tests.py
  • tests/integration_tests/model_tests.py
  • tests/integration_tests/superset_test_config.py
  • tests/integration_tests/versioning/id_reuse_tests.py
  • tests/unit_tests/commands/dataset/test_duplicate.py
  • tests/unit_tests/commands/importers/v1/examples_test.py
  • tests/unit_tests/databases/filters_test.py
  • tests/unit_tests/db_engine_specs/test_snowflake.py
  • tests/unit_tests/db_engine_specs/test_trino.py
  • tests/unit_tests/extensions/test_sqlalchemy.py
  • tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.py
  • tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py
  • tests/unit_tests/sql_lab_test.py
• Files skipped - 1

  • UPDATING.md - Reason: Filter setting
• Tools

  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful
  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers a full AI review.

• /pause - Pauses automatic reviews on this pull request.

• /resume - Resumes automatic reviews.

• /resolve - Marks all Bito-posted review comments as resolved.

• /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[codecov]

Codecov Report

❌ Patch coverage is 58.33333% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.65%. Comparing base (cfd40bd) to head (ec621b3).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
superset/tasks/manager.py	0.00%	1 Missing and 2 partials ⚠️
superset/db_engine_specs/databricks.py	0.00%	2 Missing ⚠️
superset/cli/export_example.py	0.00%	1 Missing ⚠️
superset/commands/importers/v1/examples.py	0.00%	1 Missing ⚠️
superset/db_engine_specs/clickhouse.py	0.00%	1 Missing ⚠️
superset/db_engine_specs/couchbase.py	0.00%	1 Missing ⚠️
superset/db_engine_specs/databend.py	0.00%	1 Missing ⚠️
superset/db_engine_specs/duckdb.py	0.00%	1 Missing ⚠️
superset/db_engine_specs/snowflake.py	0.00%	1 Missing ⚠️
superset/mcp_service/chart/preview_utils.py	0.00%	1 Missing ⚠️
... and 2 more

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #43260   +/-   ##
=======================================
  Coverage   66.65%   66.65%           
=======================================
  Files        2874     2874           
  Lines      163781   163750   -31     
  Branches    37798    37796    -2     
=======================================
- Hits       109168   109153   -15     
+ Misses      52475    52458   -17     
- Partials     2138     2139    +1     

Flag	Coverage Δ
hive	38.15% <27.77%> (+0.01%)	⬆️
mysql	57.84% <58.33%> (+<0.01%)	⬆️
postgres	57.88% <58.33%> (+<0.01%)	⬆️
presto	40.10% <27.77%> (+0.01%)	⬆️
python	59.25% <58.33%> (-0.01%)	⬇️
sqlite	57.51% <52.77%> (+<0.01%)	⬆️
unit	100.00% <ø> (ø)

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

☔ View full report in Codecov by Harness.
📢 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.

[villebro]

@aminghadersohi what is the purpose of this revert? Staying on SQLA 1.x is not sustainable, hence we should fix forward with 2.x rather than reverting to a version that's no longer actively maintained. Do note that we're still in the breaking window, so breakage due to major changes is expected.

CC: @rusackas

[aminghadersohi]

@aminghadersohi what is the purpose of this revert? Staying on SQLA 1.x is not sustainable, hence we should fix forward with 2.x rather than reverting to a version that's no longer actively maintained. Do note that we're still in the breaking window, so breakage due to major changes is expected.

CC: @rusackas

Hi @villebro we had some discussions about revert, vs a dual compat pr, vs fix forward so i put up the prs quick. but the vote seems to be 'fix forward' so switching this to draft in case something blocks that plan, but will close this later (probably sooner). thanks for the comment/vote. appreciated as always.

[aminghadersohi]

Closing this in favor of fixing forward on SQLAlchemy 2.0. We are keeping the SQLAlchemy 2 upgrade, updating and qualifying the downstream driver fleet, adding a machine-readable driver inventory/drift gate, and introducing real runtime coverage for materially used connectors rather than retaining the rollback path. The first Shell implementation is preset-io/superset-shell#4845, with follow-up work planned for Athena qualification, DataFusion restoration, customer-usage prioritization, and permanent connector canaries.