-
Notifications
You must be signed in to change notification settings - Fork 17.7k
chore(backend): Bump SQLAlchemy to version 2. #39735
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -99,7 +99,7 @@ dependencies = [ | |
| "sshtunnel>=0.4.0, <0.5", | ||
| "simplejson>=3.15.0", | ||
| "slack_sdk>=3.19.0, <4", | ||
| "sqlalchemy>=1.4, <2", | ||
| "sqlalchemy>=2, <3", | ||
| "sqlalchemy-utils>=0.38.0, <0.43", # expanding lowerbound to work with pydoris | ||
|
Comment on lines
+102
to
103
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| "sqlglot>=28.10.0, <29", | ||
| # newer pandas needs 0.9+ | ||
|
|
||
There was a problem hiding this comment.
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 🤖