chore: Unset future flag in engines and sessions. - #43273
Merged
Conversation
hy144328
requested review from
betodealmeida,
dpgaspar,
eschutho,
geido,
hainenber,
kgabryje,
mistercrunch,
nytai,
rusackas,
sadpandajoe,
sha174n and
villebro
as code owners
August 17, 2026 22:21
dosubot
Bot
added
change:backend
github-actions
Bot
added
risk:db-migration
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #43273 +/- ##
==========================================
+ Coverage 66.65% 66.75% +0.09%
==========================================
Files 2874 2879 +5
Lines 163787 164412 +625
Branches 37799 37797 -2
==========================================
+ Hits 109168 109746 +578
- Misses 52479 52527 +48
+ Partials 2140 2139 -1
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:
|
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review Agent Run #d015f4
Actionable Suggestions - 2
-
superset/db_engine_specs/gsheets.py - 1
- Connection resource leak · Line 393-393
-
superset/migrations/versions/2024-02-07_17-13_87d38ad83218_migrate_can_view_and_drill_permission.py - 1
- Inconsistent exception handling · Line 76-77
Additional Suggestions - 1
-
superset/migrations/versions/2025-04-13_22-10_378cecfdba9f_merge_x_axis_sort_series_with_x_axis_.py - 1
-
Redundant param removal verified · Line 61-61Removing `future=True` from `db.Session()` aligns with the broader codebase cleanup (commit 9fa26f4). In SQLAlchemy 2.0 (`>=2.0.0, <2.1` per `pyproject.toml`), future-mode is always enabled by default so the parameter is redundant. Ensure Flask-SQLAlchemy 3.1.1's Session wrapper does not reject this kwarg on SQLAlchemy 2.0, as it was a valid opt-in parameter in SQLAlchemy 1.4.
-
Filtered by Review Rules
Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
-
superset/migrations/versions/2023-07-18_15-30_863adcf72773_delete_obsolete_druid_nosql_slice_parameters.py - 1
- SQLAlchemy 2.0 compatibility fix · Line 51-51
-
superset/migrations/versions/2022-03-02_16-41_7293b0ca7944_change_adhoc_filter_b_from_none_to_.py - 2
- Correct SQLAlchemy 2.0 fix · Line 50-50
- Correct SQLAlchemy 2.0 fix · Line 69-69
-
superset/migrations/versions/2017-02-08_14-16_a99f2f7c195a_rewriting_url_from_shortner_with_new_.py - 1
- Correct parameter removal · Line 64-64
-
superset/migrations/versions/2023-07-19_16-48_a23c6f8b1280_cleanup_erroneous_parent_filter_ids.py - 1
- SQLAlchemy future=True removed · Line 51-51
-
superset/migrations/versions/2021-02-18_09-13_c501b7c653a3_add_missing_uuid_column.py - 1
- CWE-20: Removed parameter causes runtime error · Line 65-65
-
superset/migrations/versions/2022-03-02_09-20_b5a422d8e252_fix_query_and_saved_query_null_schema.py - 1
- future=True removal correct · Line 54-54
-
superset/migrations/versions/2020-12-14_13-40_4b84f97828aa_security_converge_logs.py - 2
- SQLAlchemy 2.0+ compatibility fix · Line 58-58
- SQLAlchemy 2.0+ compatibility fix · Line 58-58
-
superset/migrations/versions/2016-09-07_23-50_33d996bcc382_update_slice_model.py - 1
- Missing return type annotation · Line 50-53
Review Details
-
Files reviewed - 160 · Commit Range:
9fa26f4..9fa26f4- scripts/benchmark_migration.py
- superset/cli/test_db.py
- superset/commands/streaming_export/base.py
- superset/db_engine_specs/gsheets.py
- superset/migrations/shared/catalogs.py
- superset/migrations/versions/2016-04-25_08-54_c3a8f8611885_materializing_permission.py
- superset/migrations/versions/2016-06-07_12-33_d8bc074f7aad_add_new_field_is_restricted_to_.py
- superset/migrations/versions/2016-06-27_08-43_27ae655e4247_make_creator_owners.py
- superset/migrations/versions/2016-09-07_23-50_33d996bcc382_update_slice_model.py
- superset/migrations/versions/2016-09-22_11-31_eca4694defa7_sqllab_setting_defaults.py
- superset/migrations/versions/2017-01-24_12-31_db0c65b146bd_update_slice_model_json.py
- superset/migrations/versions/2017-02-08_14-16_a99f2f7c195a_rewriting_url_from_shortner_with_new_.py
- superset/migrations/versions/2017-12-08_08-19_67a6ac9b727b_update_spatial_params.py
- superset/migrations/versions/2017-12-17_11-06_21e88bc06c02_annotation_migration.py
- superset/migrations/versions/2018-02-13_08-07_e866bd2d4976_smaller_grid.py
- superset/migrations/versions/2018-04-03_08-19_130915240929_is_sqllab_viz_flow.py
- superset/migrations/versions/2018-04-10_11-19_bf706ae5eb46_cal_heatmap_metric_to_metrics.py
- superset/migrations/versions/2018-06-04_11-12_c5756bec8b47_time_grain_sqla.py
- superset/migrations/versions/2018-06-07_09-52_afb7730f6a9c_remove_empty_filters.py
- superset/migrations/versions/2018-06-13_10-20_4451805bbaa1_remove_double_percents.py
- superset/migrations/versions/2018-06-13_14-54_bddc498dd179_adhoc_filters.py
- superset/migrations/versions/2018-06-14_14-31_80a67c5192fa_single_pie_chart_metric.py
- superset/migrations/versions/2018-07-05_15-19_3dda56f1c4c6_migrate_num_period_compare_and_period_.py
- superset/migrations/versions/2018-07-19_23-41_c617da68de7d_form_nullable.py
- superset/migrations/versions/2018-07-20_15-31_7f2635b51f5d_update_base_columns.py
- superset/migrations/versions/2018-07-20_15-57_e9df189e5c7e_update_base_metrics.py
- superset/migrations/versions/2018-07-22_11-59_bebcf3fed1fe_convert_dashboard_v1_positions.py
- superset/migrations/versions/2018-08-01_11-47_7fcdcde0761c_.py
- superset/migrations/versions/2018-11-12_13-31_4ce8df208545_migrate_time_range_for_default_filters.py
- superset/migrations/versions/2018-12-11_22-03_fb13d49b72f9_better_filters.py
- superset/migrations/versions/2018-12-15_12-34_3e1b21cd94a4_change_owner_to_m2m_relation_on_.py
- superset/migrations/versions/2019-03-21_10-22_d94d33dbe938_form_strip.py
- superset/migrations/versions/2019-04-09_16-27_80aa3f04bc82_add_parent_ids_in_dashboard_layout.py
- superset/migrations/versions/2019-06-28_13-17_ab8c66efdd01_resample.py
- superset/migrations/versions/2019-07-15_12-00_190188938582_adding_unique_constraint_on_dashboard_slices_tbl.py
- superset/migrations/versions/2019-09-11_21-49_5afa9079866a_serialize_schema_permissions_py.py
- superset/migrations/versions/2019-09-19_13-40_258b5280a45e_form_strip_leading_and_trailing_whitespace.py
- superset/migrations/versions/2019-10-10_13-52_1495eb914ad3_time_range.py
- superset/migrations/versions/2019-11-06_15-23_78ee127d0d1d_reconvert_legacy_filters_into_adhoc.py
- superset/migrations/versions/2020-02-07_14-13_3325d4caccc8_dashboard_scoped_filters.py
- superset/migrations/versions/2020-03-25_10-42_f9a30386bd74_cleanup_time_grainularity.py
- superset/migrations/versions/2020-04-29_09-24_620241d1153f_update_time_grain_sqla.py
- superset/migrations/versions/2020-08-12_00-24_978245563a02_migrate_iframe_to_dash_markdown.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/2020-10-05_18-10_af30ca79208f_collapse_alerting_models_into_a_single_.py
- superset/migrations/versions/2020-10-21_21-09_96e99fb176a0_add_import_mixing_to_saved_query.py
- superset/migrations/versions/2020-11-20_14-24_e38177dbf641_security_converge_saved_queries.py
- superset/migrations/versions/2020-11-30_15-25_40f16acf1ba7_security_converge_reports.py
- superset/migrations/versions/2020-11-30_17-54_8ee129739cf9_security_converge_css_templates.py
- superset/migrations/versions/2020-12-09_14-13_ccb74baaa89b_security_converge_charts.py
- superset/migrations/versions/2020-12-10_15-05_45731db65d9c_security_converge_datasets.py
- superset/migrations/versions/2020-12-11_11-45_1f6dca87d1a2_security_converge_dashboards.py
- superset/migrations/versions/2020-12-11_17-02_c25cb2c78727_security_converge_annotations.py
- superset/migrations/versions/2020-12-14_10-49_42b4c9e01447_security_converge_databases.py
- superset/migrations/versions/2020-12-14_13-40_4b84f97828aa_security_converge_logs.py
- superset/migrations/versions/2020-12-16_12-15_e37912a26567_security_converge_queries.py
- superset/migrations/versions/2021-01-23_16-25_260bf0649a77_migrate_x_dateunit_in_time_range.py
- superset/migrations/versions/2021-02-04_09-34_070c043f2fdb_add_granularity_to_charts_where_missing.py
- superset/migrations/versions/2021-02-10_12-32_41ce8799acc3_rename_pie_label_type.py
- superset/migrations/versions/2021-02-14_11-46_1412ec1e5a7b_legacy_force_directed_to_echart.py
- superset/migrations/versions/2021-02-18_09-13_c501b7c653a3_add_missing_uuid_column.py
- superset/migrations/versions/2021-03-24_09-47_989bbe479899_rename_filter_configuration_in_.py
- superset/migrations/versions/2021-04-07_07-21_134cea61c5e7_remove_dataset_health_check_message.py
- superset/migrations/versions/2021-04-09_16-14_085f06488938_country_map_use_lowercase_country_name.py
- superset/migrations/versions/2021-04-12_12-38_fc3a3a8ff221_migrate_filter_sets_to_new_format.py
- superset/migrations/versions/2021-04-29_15-32_f1410ed7ec95_migrate_native_filters_to_new_schema.py
- superset/migrations/versions/2021-08-02_16-39_e323605f370a_fix_schemas_allowed_for_csv_upload.py
- superset/migrations/versions/2021-08-03_15-36_143b6f2815da_migrate_pivot_table_v2_heatmaps_to_new_.py
- superset/migrations/versions/2021-08-04_17-16_f6196627326f_update_chart_permissions.py
- superset/migrations/versions/2021-08-09_17-32_07071313dd52_change_fetch_values_predicate_to_text.py
- superset/migrations/versions/2021-08-31_11-37_021b81fe4fbb_add_type_to_native_filter_configuration.py
- superset/migrations/versions/2021-09-27_11-31_60dc453f4e2e_migrate_timeseries_limit_metric_to_.py
- superset/migrations/versions/2021-10-12_11-15_32646df09c64_update_time_grain_sqla.py
- superset/migrations/versions/2021-11-11_04-18_0ca9e5f1dacd_rename_to_schemas_allowed_for_file_.py
- superset/migrations/versions/2021-12-10_19-25_bb38f40aa3ff_add_force_screenshot_to_alerts_reports.py
- superset/migrations/versions/2021-12-13_14-06_fe23025b9441_rename_big_viz_total_form_data_fields.py
- superset/migrations/versions/2021-12-17_16-56_31bb738bd1d2_move_pivot_table_v2_legacy_order_by_to_.py
- superset/migrations/versions/2022-03-02_09-20_b5a422d8e252_fix_query_and_saved_query_null_schema.py
- superset/migrations/versions/2022-03-02_16-41_7293b0ca7944_change_adhoc_filter_b_from_none_to_.py
- superset/migrations/versions/2022-04-01_14-38_a9422eeaae74_new_dataset_models_take_2.py
- superset/migrations/versions/2022-04-04_15-04_b0d0249074e4_deprecate_time_range_endpoints_v2.py
- superset/migrations/versions/2022-04-18_11-20_ad07e4fdbaba_rm_time_range_endpoints_from_qc_3.py
- superset/migrations/versions/2022-05-03_19-39_cbe71abde154_fix_report_schedule_and_log.py
- superset/migrations/versions/2022-05-18_16-07_e786798587de_delete_none_permissions.py
- superset/migrations/versions/2022-06-19_16-17_f3afaf1f11f0_add_unique_name_desc_rls.py
- superset/migrations/versions/2022-06-27_14-59_7fb8bca906d2_permalink_rename_filterstate.py
- superset/migrations/versions/2022-07-05_15-48_409c7b420ab0_add_created_by_fk_as_owner.py
- superset/migrations/versions/2022-07-07_13-00_c747c78868b6_migrating_legacy_treemap.py
- superset/migrations/versions/2022-07-07_14-00_06e1e70058c7_migrating_legacy_area.py
- superset/migrations/versions/2022-08-16_15-23_6d3c6f9d665d_fix_table_chart_conditional_formatting_.py
- superset/migrations/versions/2022-11-28_17-51_4ce1d9b25135_remove_filter_bar_orientation.py
- superset/migrations/versions/2023-02-28_14-46_c0a3ea245b61_remove_show_native_filters.py
- superset/migrations/versions/2023-03-05_10-06_d0ac08bb5b83_invert_horizontal_bar_chart_order.py
- superset/migrations/versions/2023-03-17_13-24_b5ea9d343307_bar_chart_stack_options.py
- superset/migrations/versions/2023-03-27_12-30_7e67aecbf3f1_chart_ds_constraint.py
- superset/migrations/versions/2023-05-01_12-03_9c2a5681ddfd_convert_key_value_entries_to_json.py
- superset/migrations/versions/2023-05-11_12-41_4ea966691069_cross_filter_global_scoping.py
- superset/migrations/versions/2023-06-08_09-02_9ba2ce3086e5_migrate_pivot_table_v1_to_v2.py
- superset/migrations/versions/2023-06-08_10-22_4c5da39be729_migrate_treemap_chart.py
- superset/migrations/versions/2023-06-08_11-34_ae58e1e58e5c_migrate_dual_line_to_mixed_chart.py
- superset/migrations/versions/2023-07-18_15-30_863adcf72773_delete_obsolete_druid_nosql_slice_parameters.py
- superset/migrations/versions/2023-07-19_16-48_a23c6f8b1280_cleanup_erroneous_parent_filter_ids.py
- superset/migrations/versions/2023-07-19_17-54_ee179a490af9_deckgl_path_width_units.py
- superset/migrations/versions/2023-08-02_15-23_0769ef90fddd_fix_schema_perm_for_datasets.py
- superset/migrations/versions/2023-08-14_09-38_9f4a086c2676_add_normalize_columns_to_sqla_model.py
- superset/migrations/versions/2023-09-06_13-18_317970b4400c_added_time_secondary_column_to_.py
- superset/migrations/versions/2023-12-15_17-58_06dd9ff00fe8_add_percent_calculation_type_funnel_.py
- superset/migrations/versions/2024-01-18_14-41_a32e0c4d8646_migrate_sunburst_chart.py
- superset/migrations/versions/2024-01-18_15-20_214f580d09c9_migrate_filter_boxes_to_native_filters.py
- superset/migrations/versions/2024-02-07_17-13_87d38ad83218_migrate_can_view_and_drill_permission.py
- superset/migrations/versions/2024-03-01_10-47_be1b217cd8cd_big_number_kpi_single_metric.py
- superset/migrations/versions/2024-04-08_15-43_5ad7321c2169_mig_new_csv_upload_perm.py
- superset/migrations/versions/2024-04-17_14-04_d60591c5515f_mig_new_excel_upload_perm.py
- superset/migrations/versions/2024-04-26_12-36_4a33124c18ad_mig_new_columnar_upload_perm.py
- superset/migrations/versions/2024-05-10_18-02_f84fde59123a_update_charts_with_old_time_comparison.py
- superset/migrations/versions/2025-01-08_09-34_d482d51c15ca_remove_legacy_plugins_5_0.py
- superset/migrations/versions/2025-01-22_14-34_74ad1125881c_converge_upload_permissions.py
- superset/migrations/versions/2025-04-13_22-10_378cecfdba9f_merge_x_axis_sort_series_with_x_axis_.py
- superset/migrations/versions/2025-06-06_00-39_363a9b1e8992_convert_metric_currencies_from_str_to_json.py
- superset/migrations/versions/2025-12-16_12-00_f5b5f88d8526_fix_form_data_string_in_query_context.py
- superset/migrations/versions/2026-03-02_00-00_ce6bd21901ab_migrate_deckgl_and_mapbox.py
- superset/migrations/versions/2026-03-02_12-00_a1b2c3d4e5f6_add_granular_export_permissions.py
- superset/migrations/versions/2026-05-29_00-00_b4a3f2e1d0c9_fix_security_view_menu_case.py
- superset/migrations/versions/2026-06-23_03-23_a7d3f1b9c2e4_cleanup_stale_can_import_pvm.py
- superset/migrations/versions/2026-06-30_00-00_d24e6b0a9c7f_strip_metricsqlexpressions_from_ag_grid_params.py
- superset/migrations/versions/2026-08-05_00-00_1a27941d5352_restore_pivot_table_percent_display.py
- superset/migrations/versions/2026-08-06_00-00_c4a1b8e2d739_databend_secure_to_sslmode.py
- superset/models/core.py
- superset/tags/models.py
- tests/integration_tests/dao/conftest.py
- tests/integration_tests/fixtures/datasource.py
- tests/integration_tests/migrations/composite_pk_association_tables__tests.py
- tests/integration_tests/migrations/composite_pk_round_trip__tests.py
- tests/integration_tests/model_tests.py
- tests/unit_tests/charts/test_filters.py
- tests/unit_tests/conftest.py
- tests/unit_tests/connectors/sqla/models_test.py
- tests/unit_tests/databases/error_provenance_test.py
- tests/unit_tests/databases/filters_test.py
- tests/unit_tests/db_engine_specs/test_gsheets.py
- tests/unit_tests/db_engine_specs/test_sqlite.py
- tests/unit_tests/distributed_lock/distributed_lock_tests.py
- tests/unit_tests/extensions/test_sqlalchemy.py
- tests/unit_tests/migrations/composite_pk_association_tables_test.py
- tests/unit_tests/migrations/test_add_deleted_at_to_dashboards.py
- tests/unit_tests/migrations/test_add_deleted_at_to_slices.py
- tests/unit_tests/migrations/test_add_deleted_at_to_tables.py
- tests/unit_tests/migrations/test_databend_secure_to_sslmode.py
- tests/unit_tests/migrations/test_enforce_oauth2_token_uniqueness.py
- tests/unit_tests/migrations/test_restore_pivot_table_percent_display.py
- tests/unit_tests/migrations/test_strip_metricsqlexpressions_from_ag_grid_params.py
- tests/unit_tests/models/core_test.py
- tests/unit_tests/models/helpers_test.py
- tests/unit_tests/models/owner_association_tables_test.py
- tests/unit_tests/security/exclude_users_filter_test.py
- tests/unit_tests/utils/filters_test.py
- tests/unit_tests/utils/pandas_sqlalchemy_compat_test.py
- tests/unit_tests/versioning/test_listener.py
- tests/unit_tests/views/test_soft_delete_filter.py
-
Files skipped - 0
-
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
AI Code Review powered by
| }, | ||
| future=True, | ||
| ) | ||
| conn = engine.connect() |
Contributor
There was a problem hiding this comment.
Connection resource leak
The connection at line 393 is never explicitly closed and has no finally block or context manager. Multiple early returns (lines 414, 425) bypass any potential cleanup. Compare to the correct pattern at line 525: with engine.connect() as conn:. This causes a resource leak where the database connection remains open after the function returns.
Code Review Run #d015f4
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
Comment on lines
+76
to
77
| session = Session(bind=bind) | ||
|
|
Contributor
There was a problem hiding this comment.
Inconsistent exception handling
The downgrade() function catches SQLAlchemyError but silently ignores it by printing and calling pass, allowing the migration to report success even on failure. The upgrade() function correctly re-raises exceptions. Inconsistent error handling between upgrade/downgrade creates a maintenance hazard and can mask database migration failures.
Code Review Run #d015f4
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
sadpandajoe
added
risk:many-files
sadpandajoe
approved these changes
Aug 20, 2026
Contributor
|
Heads up that this may not be a no-op: the project currently pins sqlalchemy<2 (1.4.x), and on 1.4 future=True opts into 2.0-style connection/transaction/commit semantics. Removing it on the live paths (e.g. streaming_export/base.py, db_engine_specs/gsheets.py, migrations/shared/catalogs.py) reverts them to legacy 1.x behavior, which the test suite won't necessarily surface. Could you clarify the intended behavior here (and whether this is prep for the SQLAlchemy 2.0 work) rather than a chore, and scope it to the paths you actually mean to change? |
Contributor
Author
|
Thanks for the feedback, @sha174n . Not sure that I follow. After #42803 , my understanding is that we are now on SQLAlchemy 2, see Line 114 in a05a099 superset/requirements/development.txt Line 953 in a05a099 |
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 .
SUMMARY
SQLAlchemy is bumped to version 2.0, and it is time to remove the scaffolding.
This PR removes the future flags, that enabled version 2 behavior in SQLAlchemy 1.4.
This PR reverses #42277 and #42365 .
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
I
greped for occurrences offutureandTrue.There should be no effect.
ADDITIONAL INFORMATION