FIX: forward connection timeout to bulkcopy pycore connection#650
Draft
bewithgaurav wants to merge 2 commits into
Draft
FIX: forward connection timeout to bulkcopy pycore connection#650bewithgaurav wants to merge 2 commits into
bewithgaurav wants to merge 2 commits into
Conversation
cursor.bulkcopy() opens a separate connection through mssql-py-core, which defaulted to a hardcoded 15s connect timeout with no way to override it. forward the cursor's query timeout (connect(timeout=X)) into pycore's connect_timeout when set, so the same limit applies to the bulk copy connection. 0 stays a no-override, leaving pycore on its default. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| def test_positive_timeout_forwarded(self, mock_logger): | ||
| """cursor._timeout > 0 ⇒ connect_timeout reaches pycore, overriding 15s.""" | ||
| mock_logger.is_debug_enabled = False | ||
| cursor = _make_cursor("Server=localhost;Database=testdb;UID=sa;PWD=pwd", None) |
| def test_zero_timeout_not_forwarded(self, mock_logger): | ||
| """cursor._timeout == 0 ⇒ no override, pycore keeps its default.""" | ||
| mock_logger.is_debug_enabled = False | ||
| cursor = _make_cursor("Server=localhost;Database=testdb;UID=sa;PWD=pwd", None) |
| def test_uses_cursor_snapshot_not_live_connection(self, mock_logger): | ||
| """timeout is the cursor snapshot; later connection changes don't apply.""" | ||
| mock_logger.is_debug_enabled = False | ||
| cursor = _make_cursor("Server=localhost;Database=testdb;UID=sa;PWD=pwd", None) |
the msi-client-id bulkcopy regression test builds a Cursor via __new__ without _timeout. _bulkcopy now reads self._timeout to forward connect_timeout (issue #626), so the bare mock raised AttributeError. set _timeout=0 to match a real cursor, same as the other bulkcopy mocks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changesNo lines with coverage information in this diff. 📋 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: 59.9%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.connection.connection.cpp: 76.2%
mssql_python.pybind.ddbc_bindings.cpp: 76.4%
mssql_python.row.py: 76.9%
mssql_python.__init__.py: 77.3%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.logging.py: 85.5%
mssql_python.connection.py: 85.6%🔗 Quick Links
|
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.
Work Item / Issue Reference
Summary
cursor.bulkcopy() opens a separate connection through mssql-py-core, which defaulted to a hardcoded 15s connect timeout with no way to override it from Python. I forward the cursor's query timeout (set via connect(timeout=X)) into pycore's connect_timeout when it's set, so the bulk copy connection honors the same limit. timeout=0 stays a no-override, leaving pycore on its 15s default. added tests covering the positive forward, the zero case, and that the cursor snapshot (not a later live connection change) is what's used.