From f46c1df2b1cdf1b0fc18d51d0937acf5a995d28d Mon Sep 17 00:00:00 2001 From: Gaurav Sharma <223556219+Copilot@users.noreply.github.com> Date: Mon, 29 Jun 2026 13:28:27 +0530 Subject: [PATCH 1/3] FIX: forward connection timeout to bulkcopy pycore connection 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> --- mssql_python/cursor.py | 9 ++++ tests/test_020_bulkcopy_auth_cleanup.py | 65 +++++++++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index aa0eed00a..e43f1b947 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -2941,6 +2941,15 @@ def bulkcopy( # Translate parsed connection string into the dict py-core expects. pycore_context = connstr_to_pycore_params(params) + # Forward the cursor's query timeout to py-core so the bulkcopy + # connection uses the same limit instead of py-core's compiled-in 15s + # default. _timeout is the snapshot taken at cursor creation (same value + # _set_timeout uses); 0 means "no override", so py-core keeps its default. + # type-is-int guard keeps bool/mocked values from leaking through. + connect_timeout = self._timeout + if type(connect_timeout) is int and connect_timeout > 0: + pycore_context["connect_timeout"] = connect_timeout + # Token acquisition — only thing cursor must handle (needs azure-identity SDK) if self.connection._auth_type: # Fresh token acquisition for mssql-py-core connection diff --git a/tests/test_020_bulkcopy_auth_cleanup.py b/tests/test_020_bulkcopy_auth_cleanup.py index 164438344..8ab4cd18a 100644 --- a/tests/test_020_bulkcopy_auth_cleanup.py +++ b/tests/test_020_bulkcopy_auth_cleanup.py @@ -26,6 +26,7 @@ def _make_cursor(connection_str, auth_type): cursor = Cursor.__new__(Cursor) cursor._connection = mock_conn + cursor._timeout = 0 cursor.closed = False cursor.hstmt = None return cursor @@ -108,3 +109,67 @@ def capture_context(ctx, **kwargs): assert "access_token" not in captured_context assert captured_context.get("user_name") == "sa" assert captured_context.get("password") == "mypwd" + + +def _capture_bulkcopy_context(cursor): + """Run bulkcopy with a mocked pycore module and return the captured context.""" + captured_context = {} + + mock_pycore_cursor = MagicMock() + mock_pycore_cursor.bulkcopy.return_value = { + "rows_copied": 1, + "batch_count": 1, + "elapsed_time": 0.1, + } + mock_pycore_conn = MagicMock() + mock_pycore_conn.cursor.return_value = mock_pycore_cursor + + def capture_context(ctx, **kwargs): + captured_context.update(ctx) + return mock_pycore_conn + + mock_pycore_module = MagicMock() + mock_pycore_module.PyCoreConnection = capture_context + + with patch.dict("sys.modules", {"mssql_py_core": mock_pycore_module}): + cursor.bulkcopy("dbo.test_table", [(1, "row")], timeout=10) + + return captured_context + + +class TestBulkcopyConnectTimeout: + """Verify cursor.bulkcopy forwards the cursor timeout to pycore (issue #626).""" + + @patch("mssql_python.cursor.logger") + 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) + cursor._timeout = 30 + + captured = _capture_bulkcopy_context(cursor) + + assert captured.get("connect_timeout") == 30 + + @patch("mssql_python.cursor.logger") + 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) + cursor._timeout = 0 + + captured = _capture_bulkcopy_context(cursor) + + assert "connect_timeout" not in captured + + @patch("mssql_python.cursor.logger") + 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) + cursor._timeout = 45 + cursor._connection.timeout = 99 # changed after cursor creation, must be ignored + + captured = _capture_bulkcopy_context(cursor) + + assert captured.get("connect_timeout") == 45 From 8f7a03769c7afe2e55a6670dc608b7cb735f1c38 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma <223556219+Copilot@users.noreply.github.com> Date: Tue, 30 Jun 2026 12:18:19 +0530 Subject: [PATCH 2/3] CHORE: set _timeout on bulkcopy mock cursor in test_008 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> --- tests/test_008_auth.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_008_auth.py b/tests/test_008_auth.py index d82ecaea4..add9495c4 100644 --- a/tests/test_008_auth.py +++ b/tests/test_008_auth.py @@ -586,6 +586,7 @@ def test_bulkcopy_path_preserves_user_assigned_msi_client_id(self): cursor = Cursor.__new__(Cursor) cursor._connection = mock_conn + cursor._timeout = 0 cursor.closed = False cursor.hstmt = None From ca19b2a7cd9b6dab9b75ef5973e45deb6c4f08a1 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma <223556219+Copilot@users.noreply.github.com> Date: Wed, 8 Jul 2026 14:21:04 +0530 Subject: [PATCH 3/3] FIX: accept IntEnum timeout for bulkcopy connect_timeout, exclude bool the public Connection.timeout setter validates with isinstance(int), so it accepts IntEnum members. the earlier type(x) is int guard rejected those, so a cursor could apply the query timeout but silently fall back to py-core's 15s for the bulkcopy connect. switch to isinstance(int) and not isinstance(bool), normalise to a plain int, and keep the >0 gate: py-core's TCP attempt path turns connect_timeout=0 into a 0ms timeout that fails instantly, so 0 stays unset and py-core applies its own default. tests cover IntEnum forwarding and bool rejection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- mssql_python/cursor.py | 13 ++++++++---- tests/test_020_bulkcopy_auth_cleanup.py | 28 +++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index e43f1b947..f77a3ba87 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -2944,11 +2944,16 @@ def bulkcopy( # Forward the cursor's query timeout to py-core so the bulkcopy # connection uses the same limit instead of py-core's compiled-in 15s # default. _timeout is the snapshot taken at cursor creation (same value - # _set_timeout uses); 0 means "no override", so py-core keeps its default. - # type-is-int guard keeps bool/mocked values from leaking through. + # _set_timeout uses). Accept any int the public Connection.timeout setter + # accepts (including IntEnum) but exclude bool, then normalise to a plain + # int so py-core's u32 extract is unambiguous. 0 stays a "no override": + # py-core's default connect path treats 0 as no deadline, but its TCP + # attempt path turns 0 into a 0ms timeout that fails instantly, so we + # leave 0 unset and let py-core apply its own 15s default. connect_timeout = self._timeout - if type(connect_timeout) is int and connect_timeout > 0: - pycore_context["connect_timeout"] = connect_timeout + if isinstance(connect_timeout, int) and not isinstance(connect_timeout, bool): + if connect_timeout > 0: + pycore_context["connect_timeout"] = int(connect_timeout) # Token acquisition — only thing cursor must handle (needs azure-identity SDK) if self.connection._auth_type: diff --git a/tests/test_020_bulkcopy_auth_cleanup.py b/tests/test_020_bulkcopy_auth_cleanup.py index 8ab4cd18a..0dbe07d86 100644 --- a/tests/test_020_bulkcopy_auth_cleanup.py +++ b/tests/test_020_bulkcopy_auth_cleanup.py @@ -10,6 +10,7 @@ """ import secrets +from enum import IntEnum from unittest.mock import MagicMock, patch SAMPLE_TOKEN = secrets.token_hex(44) @@ -173,3 +174,30 @@ def test_uses_cursor_snapshot_not_live_connection(self, mock_logger): captured = _capture_bulkcopy_context(cursor) assert captured.get("connect_timeout") == 45 + + @patch("mssql_python.cursor.logger") + def test_intenum_timeout_forwarded_as_plain_int(self, mock_logger): + """IntEnum (accepted by the public setter) is forwarded, normalised to int.""" + mock_logger.is_debug_enabled = False + + class _T(IntEnum): + thirty = 30 + + cursor = _make_cursor("Server=localhost;Database=testdb;UID=sa;PWD=pwd", None) + cursor._timeout = _T.thirty + + captured = _capture_bulkcopy_context(cursor) + + assert captured.get("connect_timeout") == 30 + assert type(captured.get("connect_timeout")) is int + + @patch("mssql_python.cursor.logger") + def test_bool_timeout_not_forwarded(self, mock_logger): + """bool is a subclass of int but must not be treated as a timeout.""" + mock_logger.is_debug_enabled = False + cursor = _make_cursor("Server=localhost;Database=testdb;UID=sa;PWD=pwd", None) + cursor._timeout = True + + captured = _capture_bulkcopy_context(cursor) + + assert "connect_timeout" not in captured