Skip to content

FIX: accept timeout=0 in bulkcopy as no timeout - #698

Draft
bewithgaurav wants to merge 1 commit into
mainfrom
bewithgaurav/fix-697-bulkcopy-timeout-zero
Draft

FIX: accept timeout=0 in bulkcopy as no timeout#698
bewithgaurav wants to merge 1 commit into
mainfrom
bewithgaurav/fix-697-bulkcopy-timeout-zero

Conversation

@bewithgaurav

Copy link
Copy Markdown
Collaborator

Work Item / Issue Reference

GitHub Issue: #697


Summary

bulkcopy() rejected timeout=0 with "timeout must be positive", but the BCP API spec documents 0 as no timeout and mssql-py-core implements it that way (BulkCopyTimeoutState::from_seconds maps 0 to an infinite deadline). the guard has been there since the feature shipped, so the documented value never worked.

relaxes the bound to < 0, matching what batch_size already does. bool is excluded explicitly since it is an int subclass and False would otherwise slip through as 0 and silently disable the timeout. negatives stay rejected since py-core takes an unsigned value.

three tests: a live copy with timeout=0, a mocked assertion that 0 reaches py-core unchanged rather than being swapped for the 30s default, and rejection cases for negatives, floats and bools.

the BCP API spec documents timeout 0 as no timeout, and mssql-py-core implements it that way (BulkCopyTimeoutState::from_seconds maps 0 to an infinite deadline). the python layer rejected it with a 'timeout must be positive' guard that has been in place since the feature shipped, so the documented value never worked.

relaxes the bound to '< 0', matching what batch_size already does. bool is excluded explicitly since it is an int subclass and False would otherwise slip through as 0 and silently disable the timeout. negatives stay rejected, py-core takes an unsigned value.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions github-actions Bot added the pr-size: small Minimal code update label Aug 4, 2026
from mssql_python.cursor import Cursor

mock_conn = MagicMock()
mock_conn.connection_str = "Server=localhost;Database=testdb;UID=sa;PWD=mypwd"
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

📊 Code Coverage Report

🔥 Diff Coverage

100%


🎯 Overall Coverage

81%


📈 Total Lines Covered: 7079 out of 8689
📁 Project: mssql-python


Diff Coverage

Diff: main...HEAD, staged and unstaged changes

No 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.ddbc_bindings.cpp: 76.3%
mssql_python.__init__.py: 77.3%
mssql_python.row.py: 77.6%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 81.4%
mssql_python.pybind.connection.connection.cpp: 83.7%
mssql_python.connection.py: 84.7%

🔗 Quick Links

⚙️ Build Summary 📋 Coverage Details

View Azure DevOps Build

Browse Full Coverage Report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-size: small Minimal code update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants