Skip to content

chore(backend): Bump SQLAlchemy to version 2.#39735

Draft
hy144328 wants to merge 2 commits into
apache:masterfrom
hy144328:chore/sqlalchemy-2
Draft

chore(backend): Bump SQLAlchemy to version 2.#39735
hy144328 wants to merge 2 commits into
apache:masterfrom
hy144328:chore/sqlalchemy-2

Conversation

@hy144328
Copy link
Copy Markdown

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

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

@hy144328 hy144328 marked this pull request as draft April 28, 2026 22:17
@dosubot dosubot Bot added change:backend Requires changing the backend dependencies:python risk:breaking-change Issues or PRs that will introduce breaking changes labels Apr 28, 2026
Comment thread pyproject.toml
Comment on lines +102 to 103
"sqlalchemy>=2, <3",
"sqlalchemy-utils>=0.38.0, <0.43", # expanding lowerbound to work with pydoris
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 Architect Review — CRITICAL

Bumping SQLAlchemy to >=2 while the codebase still calls Engine.execute (for example db.engine.execute("SELECT 1") in superset/initialization/init.py:40 and engine.execute(...) in multiple db_engine_specs) will cause runtime AttributeError under SQLAlchemy 2, breaking normal app startup and common database operations.

Suggestion: Revert the SQLAlchemy requirement to <2 until all Engine.execute usages are refactored to use Connection.execute/db.session.execute, or include those refactors in this PR and verify core startup and DB engine spec paths under SQLAlchemy 2.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.

**Path:** pyproject.toml
**Line:** 102:103
**Comment:**
	*CRITICAL: Bumping SQLAlchemy to >=2 while the codebase still calls Engine.execute (for example db.engine.execute("SELECT 1") in superset/initialization/__init__.py:40 and engine.execute(...) in multiple db_engine_specs) will cause runtime AttributeError under SQLAlchemy 2, breaking normal app startup and common database operations.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

Comment thread pyproject.toml
Comment on lines +102 to 103
"sqlalchemy>=2, <3",
"sqlalchemy-utils>=0.38.0, <0.43", # expanding lowerbound to work with pydoris
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 Architect Review — HIGH

Legacy Alembic migration scripts still use SQLAlchemy 1.x-only constructs like MetaData(bind=bind), autoload=True, and select([...]) (for example in superset/migrations/versions/2020-04-24_10-46_e557699a813e_add_tables_relation_to_row_level_.py and 2020-01-08_01-17_e96dbf2cfef0_datasource_cluster_fk.py), which are incompatible with SQLAlchemy 2 and will cause db upgrade to fail for fresh installs or long-jump upgrades.

Suggestion: Before enforcing SQLAlchemy>=2, update or patch the affected Alembic revisions (or add compatibility shims in the migration env) to remove MetaData(bind=...), autoload=True, and select([...]) usage, and run a full migration chain test from an empty database under SQLAlchemy 2.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.

**Path:** pyproject.toml
**Line:** 102:103
**Comment:**
	*HIGH: Legacy Alembic migration scripts still use SQLAlchemy 1.x-only constructs like MetaData(bind=bind), autoload=True, and select([...]) (for example in superset/migrations/versions/2020-04-24_10-46_e557699a813e_add_tables_relation_to_row_level_.py and 2020-01-08_01-17_e96dbf2cfef0_datasource_cluster_fk.py), which are incompatible with SQLAlchemy 2 and will cause db upgrade to fail for fresh installs or long-jump upgrades.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

@pull-request-size pull-request-size Bot added size/L and removed size/S labels Apr 28, 2026
@github-actions github-actions Bot added risk:db-migration PRs that require a DB migration and removed dependencies:python labels Apr 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:backend Requires changing the backend review:draft risk:breaking-change Issues or PRs that will introduce breaking changes risk:db-migration PRs that require a DB migration size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants