Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 53
FEAT: Pass all connection string params to mssql-py-core for bulk copy#439
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Gaurav Sharma (bewithgaurav)
merged 21 commits into
main
from
bewithgaurav/bulkcopy-connstr-to-pycoreFeb 20, 2026
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
cb370cd
FEAT: Support Access Tokens for Bulk Copy
bewithgaurav d283b8f
Fix test_008_auth to unpack 3-tuple from process_connection_string
bewithgaurav 356392f
Fix comments: replace ODBC/Rust references with DDBC/mssql-py-core
bewithgaurav f10f70d
Add test coverage for get_raw_token and auth_type assertions
bewithgaurav bf134cb
Fix auth_type propagation for bulkcopy on all platforms
bewithgaurav 16e3a0c
Validate auth_type in _acquire_token before credential lookup
bewithgaurav cb2daea
Address review feedback: validate auth_type in _acquire_token, fix fa…
bewithgaurav f0f34fd
Add tests for extract_auth_type and _acquire_token unsupported auth_type
bewithgaurav 4bfd259
Add SQL_COPT_SS_ACCESS_TOKEN constant, replace hardcoded 1256
bewithgaurav abca6eb
Merge branch 'main' of https://github.com/microsoft/mssql-python into…
bewithgaurav d81a22d
Wrap bulk copy token acquisition with context-specific error handling
bewithgaurav d532b8e
Catch ValueError in bulk copy token acquisition
bewithgaurav 621391a
Forward all connection string params to py-core in bulkcopy
bewithgaurav da09a86
Merge main, resolve conflicts (keep connstr_to_pycore_params)
bewithgaurav 7d70539
Improve connstr_to_pycore_params docstring and variable names
bewithgaurav 0e861a9
Remove unsupported params from connstr_to_pycore_params key_map
bewithgaurav 0b1941c
Fix coverage comment shell injection from diff-cover output
bewithgaurav 8c2a696
Move boolean validation to connection.rs — pass trust_server_certific…
bewithgaurav dffadd6
Use first-wins for synonym keys to match ODBC behaviour
bewithgaurav 942ad7d
Fix allowlist test to expect first-wins synonym behavior
bewithgaurav c8851a2
Merge branch 'main' of https://github.com/microsoft/mssql-python into…
bewithgaurav File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -17,7 +17,7 @@ | ||
| import warnings | ||
| from typing import List, Union, Any, Optional, Tuple, Sequence, TYPE_CHECKING, Iterable | ||
| from mssql_python.constants import ConstantsDDBC as ddbc_sql_const, SQLTypes | ||
| from mssql_python.helpers import check_error | ||
| from mssql_python.helpers import check_error, connstr_to_pycore_params | ||
| from mssql_python.logging import logger | ||
| from mssql_python import ddbc_bindings | ||
| from mssql_python.exceptions import ( | ||
| @@ -2498,6 +2498,7 @@ def nextset(self) -> Union[bool, None]: | ||
| ) | ||
| return True | ||
| # ── Mapping from ODBC connection-string keywords (lowercase, as _parse returns) | ||
| def _bulkcopy( | ||
| self, | ||
| table_name: str, | ||
| @@ -2632,38 +2633,10 @@ def _bulkcopy( | ||
| "Specify the target database explicitly to avoid accidentally writing to system databases." | ||
| ) | ||
| # Build connection context for bulk copy library | ||
| # Note: Password is extracted separately to avoid storing it in the main context | ||
| # dict that could be accidentally logged or exposed in error messages. | ||
| trust_cert = params.get("trustservercertificate", "yes").lower() in ("yes", "true") | ||
| # Parse encryption setting from connection string | ||
| encrypt_param = params.get("encrypt") | ||
| if encrypt_param is not None: | ||
| encrypt_value = encrypt_param.strip().lower() | ||
| if encrypt_value in ("yes", "true", "mandatory", "required"): | ||
| encryption = "Required" | ||
| elif encrypt_value in ("no", "false", "optional"): | ||
| encryption = "Optional" | ||
| else: | ||
| # Pass through unrecognized values (e.g., "Strict") to the underlying driver | ||
| encryption = encrypt_param | ||
| else: | ||
| encryption = "Optional" | ||
| context = { | ||
| "server": params.get("server"), | ||
| "database": params.get("database"), | ||
| "trust_server_certificate": trust_cert, | ||
| "encryption": encryption, | ||
| } | ||
| # Build pycore_context with appropriate authentication. | ||
| # For Azure AD: acquire a FRESH token right now instead of reusing | ||
| # the one from connect() time — avoids expired-token errors when | ||
| # bulkcopy() is called long after the original connection. | ||
| pycore_context = dict(context) | ||
| # Translate parsed connection string into the dict py-core expects. | ||
| pycore_context = connstr_to_pycore_params(params) | ||
bewithgaurav marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| # 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 | ||
| @@ -2680,10 +2653,6 @@ def _bulkcopy( | ||
| "Bulk copy: acquired fresh Azure AD token for auth_type=%s", | ||
| self.connection._auth_type, | ||
| ) | ||
| else: | ||
| # SQL Server authentication — use uid/password from connection string | ||
| pycore_context["user_name"] = params.get("uid", "") | ||
| pycore_context["password"] = params.get("pwd", "") | ||
| pycore_connection = None | ||
| pycore_cursor = None | ||
| @@ -2722,9 +2691,8 @@ def _bulkcopy( | ||
| finally: | ||
| # Clear sensitive data to minimize memory exposure | ||
| if pycore_context: | ||
| pycore_context.pop("password", None) | ||
| pycore_context.pop("user_name", None) | ||
| pycore_context.pop("access_token", None) | ||
| for key in ("password", "user_name", "access_token"): | ||
| pycore_context.pop(key, None) | ||
| # Clean up bulk copy resources | ||
| for resource in (pycore_cursor, pycore_connection): | ||
| if resource and hasattr(resource, "close"): | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -250,6 +250,97 @@ def _sanitize_for_logging(input_val: Any, max_length: int = max_log_length) -> s | ||
| return True, None, sanitized_attr, sanitized_val | ||
| def connstr_to_pycore_params(params: dict) -> dict: | ||
| """Translate parsed ODBC connection-string params for py-core's bulk copy path. | ||
| When ``cursor.bulkcopy()`` is called, mssql-python opens a *separate* | ||
| connection through mssql-py-core. | ||
| py-core's ``connection.rs`` expects a Python dict with snake_case keys — | ||
| different from the ODBC-style keys that ``_ConnectionStringParser._parse`` | ||
| returns. | ||
| This function bridges that gap: it maps lowercase ODBC keys (e.g. | ||
| ``"trustservercertificate"``) to py-core keys (``"trust_server_certificate"``) | ||
| and converts numeric strings to ``int`` for timeout/size params. | ||
| Boolean params (TrustServerCertificate, MultiSubnetFailover) are passed as | ||
| strings — ``connection.rs`` validates Yes/No and rejects invalid values. | ||
| Unrecognised keys are silently dropped. | ||
| """ | ||
| # Only keys listed below are forwarded to py-core. | ||
| # Unknown/reserved keys (app, workstationid, language, connect_timeout, | ||
| # mars_connection) are silently dropped here. In the normal connect() | ||
| # path the parser validates keywords first (validate_keywords=True), | ||
| # but bulkcopy parses with validation off, so this mapping is the | ||
| # authoritative filter in that path. | ||
| key_map = { | ||
| # auth / credentials | ||
| "uid": "user_name", | ||
bewithgaurav marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| "pwd": "password", | ||
| "trusted_connection": "trusted_connection", | ||
| "authentication": "authentication", | ||
| # server (accept parser synonyms) | ||
| "server": "server", | ||
| "addr": "server", | ||
| "address": "server", | ||
| # database | ||
| "database": "database", | ||
| "applicationintent": "application_intent", | ||
| # encryption / TLS (include snake_case alias the parser may emit) | ||
| "encrypt": "encryption", | ||
| "trustservercertificate": "trust_server_certificate", | ||
| "trust_server_certificate": "trust_server_certificate", | ||
| "hostnameincertificate": "host_name_in_certificate", | ||
| "servercertificate": "server_certificate", | ||
| # Kerberos | ||
| "serverspn": "server_spn", | ||
| # network | ||
| "multisubnetfailover": "multi_subnet_failover", | ||
| "ipaddresspreference": "ip_address_preference", | ||
| "keepalive": "keep_alive", | ||
| "keepaliveinterval": "keep_alive_interval", | ||
| # sizing / limits ("packet size" with space is a common pyodbc-ism) | ||
| "packetsize": "packet_size", | ||
| "packet size": "packet_size", | ||
| "connectretrycount": "connect_retry_count", | ||
| "connectretryinterval": "connect_retry_interval", | ||
| } | ||
| int_keys = { | ||
| "packet_size", | ||
| "connect_retry_count", | ||
| "connect_retry_interval", | ||
| "keep_alive", | ||
| "keep_alive_interval", | ||
| } | ||
| pycore_params: dict = {} | ||
| for connstr_key, pycore_key in key_map.items(): | ||
| raw_value = params.get(connstr_key) | ||
| if raw_value is None: | ||
| continue | ||
| # First-wins: match ODBC behaviour — first synonym in the | ||
| # connection string takes precedence (e.g. Addr before Server). | ||
| if pycore_key in pycore_params: | ||
| continue | ||
| # ODBC values are always strings; py-core expects native types for int keys. | ||
| # Boolean params (trust_server_certificate, multi_subnet_failover) are passed | ||
| # as strings — all Yes/No validation is in connection.rs for single-location | ||
| # consistency with Encrypt, ApplicationIntent, IPAddressPreference, etc. | ||
| if pycore_key in int_keys: | ||
| # Numeric params (timeouts, packet size, etc.) — skip on bad input | ||
| try: | ||
| pycore_params[pycore_key] = int(raw_value) | ||
| except (ValueError, TypeError): | ||
| pass # let py-core fall back to its compiled-in default | ||
| else: | ||
| # String params (server, database, encryption, etc.) — pass through | ||
| pycore_params[pycore_key] = raw_value | ||
| return pycore_params | ||
| # Settings functionality moved here to avoid circular imports | ||
| # Initialize the locale setting only once at module import time | ||
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
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
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
Uh oh!
There was an error while loading. Please reload this page.