- Notifications
You must be signed in to change notification settings - Fork10
REFACTOR: Enhance & Fix Logs#137
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
This PR refactors the logging system by replacing the globalENABLE_LOGGING
flag with a singletonLoggingManager
, centralizes logger setup with file rotation and timestamped filenames, and sanitizes connection strings. It also updates logging calls across modules to use the new manager, enhances the C++ binding logging function with formatting and error handling, and adjusts driver path logic for Windows compatibility.
- Introduce
LoggingManager
singleton to manage logging configuration - Replace
ENABLE_LOGGING
checks withlogger
existence and use newsanitize_connection_string
- Update C++
LOG
function and Windows driver path logic
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
tests/test_006_logging.py | Updated tests to useLoggingManager and removed global flag |
mssql_python/logging_config.py | AddedLoggingManager class, removedENABLE_LOGGING , wrapped exports |
mssql_python/helpers.py | Removed global flag import, addedsanitize_connection_string |
mssql_python/exceptions.py | Replaced global flag checks withlogger existence |
mssql_python/cursor.py | Replaced global flag checks withlogger existence |
mssql_python/connection.py | Initialized logger at import, usesanitize_connection_string |
mssql_python/pybind/ddbc_bindings.cpp | EnhancedLOG templated function and updated Windows driver path logic |
Comments suppressed due to low confidence (3)
mssql_python/helpers.py:189
- The new
sanitize_connection_string
function lacks unit tests. Add tests to verify that sensitive fields likePwd
are correctly masked.
def sanitize_connection_string(conn_str: str) -> str:
mssql_python/helpers.py:76
- The
logger
variable is not defined in this module, resulting in a NameError. You should calllogger = get_logger()
at the top or fetch a logger inside the function.
if logger:
mssql_python/cursor.py:434
- The
logger
variable is not defined in this module. Definelogger = get_logger()
at the top or retrieve the logger in scope before using it.
if logger:
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
…rosoft/mssql-python into bewithgaurav/enhance_logging
a4da13a
intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
ADO Work Item Reference
Summary
This pull request refactors the logging system in the
mssql_python
package, replacing the previous global logging mechanism with a centralizedLoggingManager
class. It also introduces a universal logging helper function (log
) and a utility to sanitize sensitive information in connection strings. Additionally, it simplifies the codebase by removing redundant logging checks and improving resource management for cursors and connections.Logging System Refactor:
LoggingManager
as a singleton class to manage logging configuration, replacing the globalENABLE_LOGGING
variable. The class centralizes logging setup and provides backward compatibility for checking if logging is enabled. (mssql_python/logging_config.py
,mssql_python/logging_config.pyR11-R52)log
function inhelpers.py
to streamline logging calls across the codebase. This function dynamically retrieves a logger instance and supports multiple log levels. (mssql_python/helpers.py
,mssql_python/helpers.pyR187-R214)Code Simplification:
ENABLE_LOGGING
checks with calls to the newlog
function, simplifying logging logic inconnection.py
,cursor.py
, andexceptions.py
. (mssql_python/connection.py
,[1];mssql_python/cursor.py
,[2];mssql_python/exceptions.py
,[3]logger
initialization from several modules, as thelog
function now handles logger retrieval. (mssql_python/auth.py
,[1];mssql_python/cursor.py
,[2]New Utilities:
sanitize_connection_string
inhelpers.py
to mask sensitive information (e.g., passwords) in connection strings before logging. (mssql_python/helpers.py
,mssql_python/helpers.pyR187-R214)Resource Management Improvements:
mssql_python/connection.py
,mssql_python/connection.pyR186)mssql_python/cursor.py
,mssql_python/cursor.pyL419-R432)Minor Enhancements:
__del__
methods inconnection.py
andcursor.py
to handle exceptions gracefully during cleanup, avoiding issues during garbage collection. (mssql_python/connection.py
,[1];mssql_python/cursor.py
,[2]