FEAT: Add opt-in/opt-out ODBC provider selection (msodbcsql18 / mssql-odbc) - #730
Open
gargsaumya wants to merge 10 commits into
Open
FEAT: Add opt-in/opt-out ODBC provider selection (msodbcsql18 / mssql-odbc)#730gargsaumya wants to merge 10 commits into
gargsaumya wants to merge 10 commits into
Conversation
Resolve the ODBC provider from MSSQL_PYTHON_ODBC_PROVIDER env var, the mssql_python.odbc_provider module property, then a default (msodbcsql18). Selection resolves once and freezes at first connect; unknown values fail closed. The native loader imports the selected provider package and resolves a provider-specific driver path. Adds get_odbc_provider_info() diagnostics and unit tests.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a process-wide, resolve-once ODBC provider selection mechanism so mssql-python can switch at runtime between the classic ODBC Driver 18 provider (msodbcsql18) and a future Rust provider (mssql-odbc), while keeping the existing public connection API unchanged.
Changes:
- Introduces
ProviderManagerto resolve provider selection (env var → module property → default), freeze it at first resolution, and fail closed on invalid selections. - Wires provider resolution into
Connection.__init__and pushes the selected provider into the native loader viaddbc_bindings.set_odbc_provider. - Adds a new unit test module covering precedence/normalization/freeze behavior and missing-provider fail-closed behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_026_odbc_provider.py | Adds unit tests for provider precedence, normalization, freeze semantics, and fail-closed behavior. |
| mssql_python/pybind/ddbc_bindings.cpp | Adds native-side provider selection plumbing and uses provider-specific package/dist names during driver resolution. |
| mssql_python/odbc_provider.py | Implements the Python-side provider selection engine (ProviderManager). |
| mssql_python/connection.py | Freezes/verifies provider selection and pushes it into the native layer before driver load. |
| mssql_python/init.py | Exposes mssql_python.odbc_provider and get_odbc_provider_info() as public diagnostics/surface area. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
mssql_python/pybind/ddbc_bindings.cppLines 1083-1094 1083 if (e.matches(PyExc_ModuleNotFoundError)) {
1084 // Phase 2: the standalone package is required. Turn the missing
1085 // dependency into a clear, actionable error instead of a fallback.
1086 LOG("GetOdbcLibsBaseDir: required package %s is not installed (%s)",
! 1087 packageName.c_str(), e.what());
1088 ThrowStdException(
! 1089 "The required '" + distName + "' package (which ships the ODBC driver "
! 1090 "binaries) is not installed. Install it with: pip install " + distName);
1091 }
1092 // A different import-time error means the package is installed but
1093 // broken; surface it instead of silently masking the real problem.
1094 LOG("GetOdbcLibsBaseDir: importing %s failed unexpectedly (%s); "📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 61.5%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 75.6%
mssql_python.row.py: 77.6%
mssql_python.pybind.connection.connection_pool.cpp: 81.4%
mssql_python.__init__.py: 83.1%
mssql_python.pybind.connection.connection.cpp: 84.3%
mssql_python.logging.py: 85.5%
mssql_python.helpers.py: 89.3%🔗 Quick Links
|
…ype stubs - Remove the C++ env-var fallback (banned getenv/DevSkim finding); Python is already the sole authoritative resolver and pushes the selection via set_odbc_provider(). PoolingManager.enable() now also resolves+pushes so an explicit pooling() call before any connect still honors the selection. - Fix two GetOdbcLibsBaseDir log messages that hardcoded 'mssql_python_odbc' regardless of the selected provider. - Widen the public odbc_provider setter type hint to Optional[str] to match ProviderManager.set_property(). - Add odbc_provider and get_odbc_provider_info() to mssql_python.pyi (PEP 561 stubs). - Make the missing-provider test deterministic by patching import_module instead of relying on the package being absent.
gargsaumya
changed the title
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.
Linked work item: AB#47445
Summary
Introduces runtime selection between the classic Microsoft ODBC Driver 18 (
msodbcsql18) and the Rust ODBC driver (mssql-odbc), without changing the public API or install command.msodbcsql18remains the default (Phase 1); the Rust driver is opt-in and fails closed until its provider wheel ships.Motivation
Design-review feedback on the Rust ODBC provider integration: hardcode the two providers and switch on an env var + module property, and use customer-friendly provider strings
msodbcsql18/mssql-odbc(not "classic").What's implemented
mssql_python/odbc_provider.py,ProviderManager): precedenceMSSQL_PYTHON_ODBC_PROVIDERenv var ->mssql_python.odbc_providermodule property -> default (msodbcsql18); values normalized and validated; unknown value fails closed (no fallback).RuntimeWarning(connection-pool model).mssql_python.odbc_providerproperty +get_odbc_provider_info()diagnostic.connection.py,pooling.py):Connection.__init__andPoolingManager.enable()both resolve + verify the provider (ensure_available()), then push the selection to native (ddbc_bindings.set_odbc_provider) before the driver loads, so an explicitpooling()call before anyconnect()still honors the selection.pybind/ddbc_bindings.cpp): Python is the sole resolver; the native side only reads the value pushed viaset_odbc_provider(), falling back to the hardcoded classic default if nothing was pushed yet (no environment variable read in C++).GetOdbcLibsBaseDirimports the selected provider's package;GetDriverPathCpphas a Rust branch (mssql-odbc.dll/.so/.dylib, nolibprefix, under the mssql-python-ownedlibs/layout); Windowsmssql-auth.dllis non-fatal for the Rust provider. Classic path is unchanged.Behavior
mssql-odbcselected with its package absent -> clear fail-closed error: "...package 'mssql_python_rust_odbc' is not installed. Install it with: pip install mssql-python-rust-odbc." No dummy wheels required.Testing
tests/test_026_odbc_provider.py— 17 unit tests (precedence, normalization, fail-closed, freeze, post-freeze warning, diagnostics, public surface).mssql-odbcfail-closed behavior end-to-end.Deferred (tracked separately)
mssql-python-rust-odbcwheel pipeline -> PyPI dependency.mssql-odbc.