Repository navigation
FIX: Restore bulk copy after upgrading from 1.15.0 - #860
Jahnvi Thakkar (jahnvi480) wants to merge 3 commits into
Conversation
Load the nested Rust extension directly when an upgrade removes its package initializer. Cover namespace-only parents and missing native dependencies for bulk copy and async queries. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
PR Performance Report✅ No regression detectedNo consistent slowdowns detected across all 2 environments. 0 IMPROVEMENTS 0 SLOWDOWNS 2/2 ENVIRONMENTS Coverage: 2 of 2 environments completed. Advisory result; does not block merging. Performance diagnosticsPhase times are inclusive diagnostics and must not be added together. They identify where measured time changed, not why it changed. No affected phases or call-count changes were recorded. All database tasks and timingsUnix / SQL Server 2022
Unix / SQL Server 2025
Build and measurement detailsPR head:
A consistent change requires more than 20% median paired movement, at least 1 ms between the median runtimes, and at least 80% of pairs exceeding the relative threshold in the same direction. A slowdown without enough pair agreement is reported as inconsistent. The displayed change is the median of paired before-and-after ratios. It is not recalculated from the two displayed median runtimes. Both revisions use profiling-enabled builds on the same agent and database, with alternating order and discarded warmups. Results are diagnostic and do not represent production-wheel latency. Raw samples and logs are attached to the ADO run as |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Replace localhost with example.test in the two import regression tests to address DevSkim comments without suppressing the rule. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
Reported Windows validation supports the fix, but cross-platform native-extension upgrade behavior remains unverified and warrants final human review.
No open issues 🎉
What changed in this PR
Addresses #855 by loading the Rust extension directly, allowing bulk copy and internal async queries to work when an upgrade from 1.15.0 removes the parent package initializer. Public APIs and package ownership remain unchanged.
Changes:
- Switch bulk-copy and async loaders to
mssql_py_core.mssql_py_core. - Add namespace-package and missing-dependency regression tests, and update existing mocks.
- Document the initializer repair command.
| File | Description |
|---|---|
| tests/test_024_bulkcopy_arrow.py | Update Arrow dependency imports and mocks. |
| tests/test_020_bulkcopy_auth_cleanup.py | Add loader regression tests and update authentication mocks. |
| tests/test_008_auth.py | Target the native submodule in the bulk-copy mock. |
| tests/AsyncTest/test_001_async_query_native.py | Verify direct loading and namespace-package support. |
| mssql_python/cursor.py | Load the native submodule for both bulk-copy methods. |
| mssql_python/async_query/_native.py | Load the native submodule for internal async operations. |
| CHANGELOG.md | Describe the upgrade fix and repair command. |
Review effort: Balanced
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.performance_counter.hpp: 0.7%
mssql_python.pybind.logger_bridge.cpp: 57.9%
mssql_python.pybind.ddbc_bindings.h: 62.6%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.connection.connection_pool.cpp: 82.3%
mssql_python.pybind.connection.connection.cpp: 83.1%
mssql_python.logging.py: 86.2%
mssql_python.pooling.py: 90.1%
mssql_python.pybind.fetch_temporal.hpp: 92.1%
mssql_python.cursor.py: 92.5%🔗 Quick Links
|
Sumit Sarabhai (sumitmsft)
left a comment
Sumit Sarabhai (sumitmsft)
left a comment
There was a problem hiding this comment.
Reviewed the PR for correctness, security, reliability, performance, test coverage, repository conventions, and applicable architecture and design specifications. No actionable issues were identified. The implementation is consistent with repository standards and the applicable approved design requirements.
Recommendation: Approve
Work Item / Issue Reference
Summary
Load
mssql_py_core.mssql_py_coredirectly for both bulk-copy methods and the internal async loader. During a 1.15.0 upgrade, pip installs the new Rust dependency before uninstalling the old driver; the old uninstall removes the initializer shared by both distributions. The native extension remains available, but top-level re-exports disappear.Add regression cases for a namespace-only parent package and missing native dependencies, update existing bulk-copy mocks, and document the repair command in the changelog. Package ownership and public APIs remain unchanged.
Validation on Windows x64 / Python 3.13:
python -m pytest -q tests\test_008_auth.py tests\test_019_bulkcopy.py tests\test_020_bulkcopy_auth_cleanup.py tests\test_024_bulkcopy_arrow.py tests\AsyncTest --tb=short: 511 passed, including live SQL Server coverage.python -m pre_commit run black-check --all-files --hook-stage pre-push: passed.Fixes: #855