feat(orm): validate connection config at DatabaseManager construction time#196
Open
tmgbedu wants to merge 1 commit into
Open
feat(orm): validate connection config at DatabaseManager construction time#196tmgbedu wants to merge 1 commit into
tmgbedu wants to merge 1 commit into
Conversation
… time PR #194 hardened ConnectionFactory.build_url()/make() so a bad connection config fails with a friendly DriverNotFound instead of a raw KeyError, but only at the point of first connection/engine-creation (e.g. db:migrate or the first query) since that is when factory code first runs. DatabaseManager now validates every configured connection up front, in __init__, so DatabaseProvider.register() (which builds the DatabaseManager during app boot) surfaces a bad config immediately: - a connection missing its 'driver' key, or with an unsupported driver, raises DriverNotFound, reusing PR #194's exception and driver list - a non-numeric 'port' on a non-sqlite driver raises ValueError - a missing/empty port is left alone (ConnectionFactory already defaults it per-driver) and an explicit 'url' bypasses these checks entirely, mirroring build_url()'s own passthrough Updates test_connection_raises_for_missing_driver to assert the failure now happens at DatabaseManager construction rather than at first .connection() call, and adds a boot-time validation test class covering the happy path, each failure mode, and an end-to-end Application-boot repro.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Summary
Follow-up to backlog task #1223 / PR #194, per the reviewer's recommendation on that PR (task #1231).
PR #194 hardened
ConnectionFactory.build_url()/make()so a bad connection config fails with a friendlyDriverNotFoundinstead of a rawKeyError/ValueError-- but only at the point of first connection/engine-creation (e.g.db:migrate, or the app's first query), since that's when factory code first runs.This PR moves that failure earlier:
DatabaseManagernow validates every configured connection up front, in__init__. SinceDatabaseProvider.register()constructs theDatabaseManagerduring app boot, a misconfigured connection now raises immediately when theApplicationis constructed, not lazily on first DB use.Validated per connection (skipped entirely if the connection supplies an explicit
url, mirroringbuild_url()'s own passthrough):driverkey must be present ->DriverNotFoundif missingdrivervalue must be one ofConnectionFactory.DRIVER_URLS->DriverNotFoundif unsupported (reuses the exception + supported-driver list from PR fix(orm): harden ConnectionFactory against bad port and unknown driver #194, doesn't reinvent)portthat's present but not int-castable (e.g."not-a-port") ->ValueError. A missing/empty port is not an error sinceConnectionFactoryalready defaults it per-driver.Test plan
uv run pytest fastapi_startkit/tests/masoniteorm/config/test_manager.py -v-- newTestDatabaseManagerBootTimeValidationclass: happy path (sqlite/mysql/postgres all construct fine), missing driver key, unsupported driver, non-numeric port, missing port is not an error, expliciturlbypass, and an end-to-endApplicationboot repro -- all passingtest_connection_raises_for_missing_driverintest_model_attributes.pyto assert the failure now happens atDatabaseManager(...)construction rather than at first.connection()calluv run pytest --ignore=tests/masoniteorm/postgresfromfastapi_startkit/-- 1988 passed, 7 skipped, no regressionsdb:migrateend-to-end unchanged; a bad config (unsupported driver) now raisesDriverNotFoundatApplication(...)construction, beforedb:migrateor any query ever runsruff check/ruff format --checkcleanPer the current merge freeze, this PR is intentionally left open/unmerged.