refactor: update SQLAlchemy select() syntax to 2.0#40276
Open
hy144328 wants to merge 5 commits into
Open
Conversation
hy144328
requested review from
betodealmeida,
eschutho,
geido,
kgabryje,
michael-s-molina,
mistercrunch,
rusackas,
sadpandajoe and
villebro
as code owners
dosubot
Bot
added
change:backend
Contributor
Code Review Agent Run #ff1d28Actionable 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 AI Code Review powered by |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the codebase to use SQLAlchemy’s 1.4/2.0-style select() calling convention (positional columns instead of legacy list-based usage), and adjusts pytest warning configuration to surface/deprecate specific SQLAlchemy 2.0 warnings during local runs.
Changes:
- Refactor legacy
select([ ... ])/select(list_of_cols)calls toselect(col1, col2, ...)orselect(*cols). - Update core query construction sites (engine specs, helpers, metadb, import utils) and unit tests accordingly.
- Replace
-p no:warningswithfilterwarningsrules and error on the legacyselect()calling style.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit_tests/db_engine_specs/test_mssql.py | Updates select() usage in MSSQL engine spec unit test. |
| tests/unit_tests/db_engine_specs/test_datastore.py | Updates select(*fields) usage in Datastore engine spec unit test. |
| tests/unit_tests/db_engine_specs/test_bigquery.py | Updates select(*fields) usage in BigQuery engine spec unit test. |
| superset/utils/core.py | Updates connection ping to select(1) in pessimistic connection handling. |
| superset/models/helpers.py | Refactors multiple query builders to SQLAlchemy 2.0-style select() calls. |
| superset/migrations/versions/2024-01-17_13-09_96164e3017c6_tagged_object_unique_constraint.py | Refactors select() usage in migration cleanup subquery and delete query. |
| superset/migrations/versions/2022-04-01_14-38_a9422eeaae74_new_dataset_models_take_2.py | Refactors many select() calls used for insert_from_select migration logic. |
| superset/migrations/versions/2020-04-24_10-46_e557699a813e_add_tables_relation_to_row_level_.py | Refactors select() usage in upgrade/downgrade migration logic. |
| superset/migrations/versions/2020-01-08_01-17_e96dbf2cfef0_datasource_cluster_fk.py | Refactors select() usage in datasource/cluster FK migration. |
| superset/extensions/metadb.py | Refactors table/count selects to SQLAlchemy 2.0-style select(). |
| superset/db_engine_specs/base.py | Refactors select_star() query construction to avoid legacy select(list) usage. |
| superset/connectors/sqla/models.py | Refactors ad-hoc type probe query select() calls. |
| superset/common/tags.py | Refactors tag backfill queries to SQLAlchemy 2.0-style select() calls. |
| superset/commands/importers/v1/utils.py | Refactors relationship lookup query to SQLAlchemy 2.0-style select(). |
| pytest.ini | Switches from -p no:warnings to filterwarnings and errors on legacy select() calling style. |
Comment on lines
51
to
55
| statement = datasources.update().values( | ||
| cluster_id=sa.select([clusters.c.id]) | ||
| cluster_id=sa.select(clusters.c.id) | ||
| .where(datasources.c.cluster_name == clusters.c.cluster_name) | ||
| .as_scalar() | ||
| ) |
Comment on lines
93
to
97
| statement = datasources.update().values( | ||
| cluster_name=sa.select([clusters.c.cluster_name]) | ||
| cluster_name=sa.select(clusters.c.cluster_name) | ||
| .where(datasources.c.cluster_id == clusters.c.id) | ||
| .as_scalar() | ||
| ) |
| #addopts = -p no:warnings | ||
| asyncio_mode = auto | ||
|
|
||
| # `ignore` virtually reproduces to `-p no:warnings`. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #40276 +/- ##
==========================================
- Coverage 64.16% 64.14% -0.02%
==========================================
Files 2591 2591
Lines 138162 138223 +61
Branches 32048 32059 +11
==========================================
+ Hits 88647 88669 +22
- Misses 47986 48024 +38
- Partials 1529 1530 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See #40273 .
Depends on #40274 .
SUMMARY
Enable errors for deprecated
sqlalchemy.select()syntax, and fix them all.In addition to fixing all broken tests, I manually went through all results of
git grep select(.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
No errors:
ADDITIONAL INFORMATION