fix(db): isolate malformed dialect entry points - #43265
Conversation
Strengthen the regression test so it proves plugin isolation rather than just absence of an exception: a malformed entry point is skipped with a warning while a well-formed dialect in the same scan still registers its driver.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #43265 +/- ##
==========================================
- Coverage 66.65% 66.65% -0.01%
==========================================
Files 2874 2874
Lines 163784 163784
Branches 37798 37798
==========================================
- Hits 109177 109174 -3
- Misses 52469 52471 +2
- Partials 2138 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:
|
Code Review Agent Run #33a30fActionable Suggestions - 0Additional Suggestions - 1
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 |
|
Superseded by #43110, which merged the same fix to |
Why
Third-party SQLAlchemy dialect entry points are discovered during database engine-spec enumeration. An entry point can load successfully while returning an object that does not satisfy the dialect contract, such as an object without
.name. That exception currently escapes the plugin boundary and can break application bootstrap for every user rather than disabling only the malformed connector.What
Validate and normalize the loaded dialect's
nameanddriverinside the existing third-party entry-point exception boundary. Malformed plugins are logged and skipped. A regression test models an entry point that loads an object without.name, alongside a well-formed entry point that must still register its driver.Blast radius
Only database connector discovery is affected. Valid SQLAlchemy dialects follow the same path as before; malformed optional plugins now degrade in isolation.
How to test
pytest tests/unit_tests/db_engine_specs/test_init.py— 3 passed. Reverting onlysuperset/db_engine_specs/__init__.pyto master makestest_malformed_dialect_entry_point_does_not_break_bootstrapfail with the escapingAttributeError, so it is a true regression test.{SqliteEngineSpec: {"pysqlite"}}.PRE_COMMIT_HOME=/tmp/pre-commit-superset uvx pre-commit run --files superset/db_engine_specs/__init__.py tests/unit_tests/db_engine_specs/test_init.py— all applicable hooks pass (ruff, ruff-format, mypy). Thepylinthook cannot run in this sandbox (pylint: command not found); it is covered by thepre-commitCI job.Risk & rollback
Low risk: the change broadens the existing plugin isolation boundary and changes the failed-plugin log from debug to warning. Revert this commit to restore the previous behavior.
Review guidance
Please review the exception boundary in
superset/db_engine_specs/__init__.pyfirst, then the malformed-contract regression fixture.