Skip to content

Conversation

@bewithgaurav
Copy link
Collaborator

@bewithgaurav bewithgaurav commented Feb 9, 2026

Work Item / Issue Reference

AB#42282
AB#42283


Summary

This pull request introduces significant improvements to Azure AD authentication handling for bulk copy operations, ensuring fresh token acquisition to prevent expired-token errors, and refactors related code for clarity and robustness. It also updates the connection and authentication APIs to propagate and utilize authentication type information more reliably.

Azure AD authentication enhancements:

  • Added AADAuth.get_raw_token and refactored token acquisition logic to ensure a fresh Azure AD token is acquired each time bulkcopy is called, preventing expired-token errors. The new method avoids credential/token caching and is used specifically for bulk copy operations. (mssql_python/auth.py, mssql_python/auth.pyL33-R51)
  • Updated bulk copy logic to use the new get_raw_token method, storing the auth type on the connection and acquiring a fresh token at bulk copy time. Sensitive data is now removed from memory after use for improved security. (mssql_python/cursor.py, [1] [2]

Connection and authentication API changes:

  • Refactored process_connection_string to return the authentication type as a third value, and added extract_auth_type to reliably extract auth type from the connection string when not propagated (e.g., Windows Interactive). Connection objects now store the auth type for later use. (mssql_python/auth.py, [1] [2] [3]; mssql_python/connection.py, [4] [5] [6]

Testing improvements:

  • Expanded test coverage to verify raw token acquisition, connection string processing, and correct storage of authentication type on connection objects. Tests ensure that the new APIs and behaviors work as expected. (tests/test_008_auth.py, [1] [2] [3]

Error handling and logging:

  • Improved error handling and logging in token acquisition, providing clearer messages for unsupported authentication types and unexpected errors. (mssql_python/auth.py, [1] [2]

Documentation and naming consistency:

  • Updated docstrings and comments throughout the authentication code for clarity and consistency, reflecting the new behaviors and APIs. (mssql_python/auth.py, mssql_python/auth.pyL183-R197)

@github-actions github-actions bot added the pr-size: small Minimal code update label Feb 10, 2026
@bewithgaurav bewithgaurav changed the title FEAT: Support Access Tokens for Bulk Copy FEAT: EntraID Access Token Support for BulkCopy Feb 10, 2026
@github-actions
Copy link

github-actions bot commented Feb 10, 2026

📊 Code Coverage Report

🔥 Diff Coverage

100%


🎯 Overall Coverage

76%


📈 Total Lines Covered: 5496 out of 7157
📁 Project: mssql-python


Diff Coverage

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

  • mssql_python/auth.py (100%)
  • mssql_python/connection.py (100%)

Summary

  • Total: 26 lines
  • Missing: 0 lines
  • Coverage: 100%

📋 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.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 79.6%
mssql_python.cursor.py: 84.7%
mssql_python.__init__.py: 84.9%

🔗 Quick Links

⚙️ Build Summary 📋 Coverage Details

View Azure DevOps Build

Browse Full Coverage Report

@bewithgaurav bewithgaurav force-pushed the bewithgaurav/bulkcopy-accesstoken-support branch from 97bfb90 to f10f70d Compare February 10, 2026 08:18
@github-actions github-actions bot added pr-size: medium Moderate update size and removed pr-size: small Minimal code update labels Feb 10, 2026
@bewithgaurav bewithgaurav marked this pull request as ready for review February 10, 2026 08:19
Copilot AI review requested due to automatic review settings February 10, 2026 08:19
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds Entra ID (Azure AD) access-token handling to support bulkcopy() by storing the detected AAD auth type on the Connection and acquiring a fresh token at bulk-copy time, alongside related auth API adjustments and test updates.

Changes:

  • Added AADAuth.get_raw_token() and internalized token acquisition via AADAuth._acquire_token().
  • Extended process_connection_string() to return auth_type and persisted it on Connection for later bulkcopy token acquisition.
  • Updated Cursor._bulkcopy() to use a fresh AAD token (or uid/pwd) and to remove sensitive keys from the bulk-copy context after use.

Reviewed changes

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

File Description
mssql_python/auth.py Introduces raw token acquisition and returns auth_type from process_connection_string() for later bulkcopy token refresh.
mssql_python/connection.py Stores _auth_type on the connection so bulkcopy can reacquire AAD tokens later.
mssql_python/cursor.py Acquires a fresh AAD token at bulkcopy time and removes sensitive fields from the pycore context in finally.
tests/test_008_auth.py Updates tests for the new process_connection_string() return signature and adds coverage for get_raw_token() / _auth_type storage.

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

- Add extract_auth_type() fallback for Windows Interactive where
  process_connection_string returns auth_type=None (DDBC handles
  auth natively but bulkcopy needs it for Azure Identity)
- Preserve auth_type in fallthrough return so bulkcopy can attempt
  fresh token acquisition even when connect-time token fails
- Fix get_raw_token docstring to match actual credential behavior
- Use 'is None' instead of '== None' in test assertion
Raises ValueError with supported types instead of KeyError when an
unsupported auth_type is passed to _acquire_token.
)

try:
logger.debug(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed unnecessary log statements, there is an info log above which captures these

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

Labels

pr-size: medium Moderate update size

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant