FEAT: Pass all connection string params to mssql-py-core for bulk copy

[Gaurav Sharma (bewithgaurav)]

Work Item / Issue Reference

AB#42663

GitHub Issue: #<ISSUE_NUMBER>

---

Summary

This pull request refactors and simplifies how ODBC connection string parameters are translated for use with the bulk copy feature in mssql_python. The key change is the introduction of a new helper function, connstr_to_pycore_params, which centralizes and standardizes the parameter mapping logic. This reduces code duplication, improves maintainability, and ensures sensitive data is handled more securely.

Bulk copy connection context refactor:

• Added a new helper function connstr_to_pycore_params in mssql_python/helpers.py to translate ODBC connection string parameters into the format expected by the py-core bulk copy path, including type conversion and key normalization.
• Updated mssql_python/cursor.py to use connstr_to_pycore_params for building the pycore_context dictionary, replacing the previous inline mapping and logic. [1][2]

Security and code quality improvements:

• Simplified sensitive data cleanup in the bulk copy context by iterating over a tuple of sensitive keys instead of repeating pop calls.
• Removed redundant manual extraction and assignment of authentication parameters in favor of the new helper function, reducing the risk of accidentally logging or exposing credentials.

Documentation and maintainability:

• Added detailed documentation to the new helper function explaining its purpose, supported keys, and conversion logic.
• Added a clarifying comment in cursor.py indicating the mapping from ODBC connection-string keywords.

[github-actions]

📊 Code Coverage Report

🔥 Diff Coverage 20%

🎯 Overall Coverage 76%

📈 Total Lines Covered:5502 out of 7178 📁 Project:mssql-python

---

Diff Coverage

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

• mssql_python/connection_string_parser.py (100%)
• mssql_python/cursor.py (100%)
• mssql_python/helpers.py (5.9%): Missing lines 275,307,315,317-320,324-325,331,333-336,339,341

Summary

• Total: 20 lines
• Missing: 16 lines
• Coverage: 20%

mssql_python/helpers.py

Lines 271-279

271# mars_connection) are silently dropped here. In the normal connect()272# path the parser validates keywords first (validate_keywords=True),273# but bulkcopy parses with validation off, so this mapping is the274# authoritative filter in that path.
! 275key_map= {
276# auth / credentials277"uid": "user_name",
278"pwd": "password",
279"trusted_connection": "trusted_connection",

---

Lines 303-311

303"packet size": "packet_size",
304"connectretrycount": "connect_retry_count",
305"connectretryinterval": "connect_retry_interval",
306 }
! 307int_keys= {
308"packet_size",
309"connect_retry_count",
310"connect_retry_interval",
311"keep_alive",

---

Lines 311-329

311"keep_alive",
312"keep_alive_interval",
313 }
314 ! 315pycore_params: dict= {}
316 ! 317forconnstr_key, pycore_keyinkey_map.items():
! 318raw_value=params.get(connstr_key)
! 319ifraw_valueisNone:
! 320continue321322# First-wins: match ODBC behaviour — first synonym in the323# connection string takes precedence (e.g. Addr before Server).
! 324ifpycore_keyinpycore_params:
! 325continue326327# ODBC values are always strings; py-core expects native types for int keys.328# Boolean params (trust_server_certificate, multi_subnet_failover) are passed329# as strings — all Yes/No validation is in connection.rs for single-location

---

Lines 327-345

327# ODBC values are always strings; py-core expects native types for int keys.328# Boolean params (trust_server_certificate, multi_subnet_failover) are passed329# as strings — all Yes/No validation is in connection.rs for single-location330# consistency with Encrypt, ApplicationIntent, IPAddressPreference, etc.
! 331ifpycore_keyinint_keys:
332# Numeric params (timeouts, packet size, etc.) — skip on bad input
! 333try:
! 334pycore_params[pycore_key] =int(raw_value)
! 335except (ValueError, TypeError):
! 336pass# let py-core fall back to its compiled-in default337else:
338# String params (server, database, encryption, etc.) — pass through
! 339pycore_params[pycore_key] =raw_value340 ! 341returnpycore_params342343344# Settings functionality moved here to avoid circular imports

---

---

📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)

mssql_python.pybind.logger_bridge.hpp: 58.8%
mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.row.py: 66.2%
mssql_python.pybind.ddbc_bindings.cpp: 69.3%
mssql_python.pybind.ddbc_bindings.h: 69.7%
mssql_python.pybind.connection.connection.cpp: 75.3%
mssql_python.helpers.py: 77.4%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 79.6%
mssql_python.cursor.py: 84.7%

---

🔗 Quick Links

⚙️ Build Summary	📋 Coverage Details
View Azure DevOps Build	Browse Full Coverage Report

[Gaurav Sharma (bewithgaurav)]

code coverage isn't reflecting correct numbers since the tests are a part of mssql-py-core as of now
will get auto-fixed once we migrate them here

[Copilot]

Pull request overview

Refactors the bulk copy (“py-core”) connection context construction so that parsed ODBC connection-string parameters are centrally translated into the parameter format expected by mssql_py_core, reducing inline mapping logic in cursor.bulkcopy().

Changes:

• Added connstr_to_pycore_params() helper to map/convert parsed connection-string params into py-core snake_case keys (with basic int coercion).
• Updated Cursor._bulkcopy() to build pycore_context via the new helper and simplified sensitive-key cleanup.
• Adjusted PR code coverage workflow to pass multiline env values into jq via shell variables.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
mssql_python/helpers.py	Introduces connstr_to_pycore_params() for bulkcopy py-core parameter translation.
mssql_python/cursor.py	Uses the new helper to build py-core bulkcopy connection context and streamlines sensitive key cleanup.
.github/workflows/pr-code-coverage.yml	Tweaks jq --arg inputs to use shell vars for better handling of multiline env values.

Comments suppressed due to low confidence (1)

mssql_python/cursor.py:2651

• In the Azure AD token flow, pycore_context is built from connstr_to_pycore_params(params) before adding access_token, which means user_name/password from the original connection string may also be included (and retained until finally). Previously, SQL credentials were only added in the non-AAD path. Consider explicitly removing user_name/password when _auth_type is set (or when setting access_token) to minimize sensitive-data exposure and avoid ambiguous/dual auth inputs being passed to mssql-py-core.

 # Translate parsed connection string into the dict py-core expects.
pycore_context = connstr_to_pycore_params(params)
# Token acquisition — only thing cursor must handle (needs azure-identity SDK)
if self.connection._auth_type:
# Fresh token acquisition for mssql-py-core connection
from mssql_python.auth import AADAuth
try:
raw_token = AADAuth.get_raw_token(self.connection._auth_type)
except (RuntimeError, ValueError) as e:
raise RuntimeError(
f"Bulk copy failed: unable to acquire Azure AD token "
f"for auth_type '{self.connection._auth_type}': {e}"
) from e
pycore_context["access_token"] = raw_token

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Sumit Sarabhai (sumitmsft)]

I am approving this with one question. Although I know the answer but just want to confirm:

Previously bulkcopy effectively defaulted TrustServerCertificate to “yes” when not specified (it used params.get("trustservercertificate", "yes") ... and passed a boolean to py-core).
Now we only forward trust_server_certificate if the keyword is present, which means bulkcopy depends on mssql-py-core’s default when it’s absent.
Since ODBC 18.5 is “secure by default” (Encrypt on; TrustServerCertificate off), can you confirm:

• What does mssql-py-core do when trust_server_certificate is not provided?
• Does this match ODBC 18.5’s effective defaults and our non-bulkcopy connection behavior?

[Gaurav Sharma (bewithgaurav)]

Sumit Sarabhai (@sumitmsft) - thanks for the review
previous impl deviated from ODBC defaults, and hence we're making changes to all default values and validation logic here, below are the answers as per the changed (expected) behaviour:

• when trust_server_certificate is not in the dict, py-core defaults to false (don't trust = validate certificate).
• does this match ODBC 18.5? : yes. ODBC 18.5's "secure by default" means:
  - Encrypt = Mandatory (on)
  - TrustServerCertificate = No (false)