FEAT: Azure Interactive support for Mac and Linux #129
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enhances connection string construction in the mssql_python package by enabling return of driver attributes for AAD Interactive Authentication and by merging attributes more robustly.
- Extended
_construct_connection_stringto return either a simple string or a tuple(connection_str, attrs_before)and merged any existingattrs_before. - Updated
add_driver_to_connection_strandadd_driver_name_to_app_parameterto support returning driver attributes (attrs_before) and to fetch AAD tokens on non-Windows platforms. - Adjusted tests to expect the corrected format without duplicate semicolons.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/test_003_connection.py | Removed extra semicolon in expected connection string tests. |
| mssql_python/helpers.py | Modified driver helper to handle tuple returns and AAD tokens; enhanced docstrings. |
| mssql_python/connection.py | Updated constructor and _construct_connection_string to unpack tuple returns and merge attributes. |
Comments suppressed due to low confidence (3)
mssql_python/helpers.py:19
- Typo in docstring: "DDBC" should be corrected to "ODBC" to match the driver being added.
Add the DDBC driver to the connection string if not present.
mssql_python/helpers.py:135
- Missing import for
platform. Addimport platformat the top of the module to avoid a NameError.
if has_aad_interactive and platform.system().lower() != "windows":
mssql_python/connection.py:69
- Variable
attrs_beforeis undefined here; the tuple unpack usedattrs_from_driver. It should beself._attrs_before = attrs_from_driver or {}.
self._attrs_before = attrs_before or {}
…rosoft/mssql-python into jahnvi/interactive_mac_linux
| def __str__(self): | ||
| """Return a string representation of the row""" | ||
| return f"Row({', '.join(map(str, self._values))})" |
There was a problem hiding this comment.
we wouldn't be needing Row(...) in the string representations, for end users the output should look like a tuple (e.g., (1, 2, 3)).
I suggest updating both the methods as follows:
def __str__(self):
return str(tuple(self._values))
def __repr__(self):
return repr(tuple(self._values))
this makes sure that printing or representing a Row instance makes it behave just like a tuple, which is the expected behaviour, also please add tests for this inside test cursor to test fetch functions results.
There was a problem hiding this comment.
I believe these changes should be a separate PR of its own
| if not param: | ||
| continue | ||
|
|
||
| if param.lower().startswith("authentication="): |
There was a problem hiding this comment.
The auth logic is currently spread across multiple helpers and is resulting in cascading function changes and unstable signatures.
I recommend refactoring all authentication-related code into a dedicated mssql_python/auth.py module. This will centralize and simplify authentication logic and help maintain single responsibility in other modules (like helpers and connection).
|
Added a new PR with required changes #135 |
ADO Work Item Reference
Summary
This pull request introduces significant enhancements to the handling of connection strings in the
mssql_pythonpackage, focusing on improved support for Active Directory Interactive Authentication (AAD) and more robust handling of attributes passed to the database driver. The changes ensure better compatibility across platforms and allow the seamless merging of attributes required by the driver.Enhancements to Connection String Handling:
_construct_connection_stringmethod inmssql_python/connection.pyto support returning a tuple containing the connection string and a dictionary of attributes (attrs_before) when needed. This allows for more flexible handling of attributes required by the database driver. [1] [2] [3]add_driver_to_connection_strfunction inmssql_python/helpers.pyto return either a string or a tuple of the connection string andattrs_before. This change ensures that attributes required by the driver are preserved and passed correctly. [1] [2]Support for Active Directory Interactive Authentication:
add_driver_name_to_app_parameterfunction inmssql_python/helpers.pyto handle AAD Interactive Authentication. On non-Windows platforms, it now uses theazure-identitylibrary to fetch a token and includes it in the connection attributes. This ensures compatibility with Azure SQL databases requiring AAD authentication.These updates improve the flexibility and platform compatibility of the
mssql_pythonpackage, making it more robust for a variety of use cases.