Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
jahnvi480 merged 26 commits intomainfrombewithgaurav/enhance_logging
Jul 17, 2025
Merged

Conversation

bewithgaurav
Copy link
Collaborator

@bewithgauravbewithgaurav commentedJul 14, 2025
edited
Loading

ADO Work Item Reference

AB#37336


Summary

This pull request refactors the logging system in themssql_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:

  • IntroducedLoggingManager 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)
  • Added a universallog 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:

  • Replaced conditionalENABLE_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]
  • Removed redundantlogger initialization from several modules, as thelog function now handles logger retrieval. (mssql_python/auth.py,[1];mssql_python/cursor.py,[2]

New Utilities:

  • Addedsanitize_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:

  • Improved cursor tracking by explicitly adding cursors to a connection's cursor set and ensuring proper cleanup during resource deallocation. (mssql_python/connection.py,mssql_python/connection.pyR186)
  • Refactored cursor cleanup logic to avoid unnecessary operations and ensure weak references are handled correctly. (mssql_python/cursor.py,mssql_python/cursor.pyL419-R432)

Minor Enhancements:

  • Updated__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]

@CopilotCopilotAI review requested due to automatic review settingsJuly 14, 2025 13:30
@github-actionsgithub-actionsbot added the pr-size: mediumModerate update size labelJul 14, 2025
Copy link
Contributor

@CopilotCopilotAI 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

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.

  • IntroduceLoggingManager singleton to manage logging configuration
  • ReplaceENABLE_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
FileDescription
tests/test_006_logging.pyUpdated tests to useLoggingManager and removed global flag
mssql_python/logging_config.pyAddedLoggingManager class, removedENABLE_LOGGING, wrapped exports
mssql_python/helpers.pyRemoved global flag import, addedsanitize_connection_string
mssql_python/exceptions.pyReplaced global flag checks withlogger existence
mssql_python/cursor.pyReplaced global flag checks withlogger existence
mssql_python/connection.pyInitialized logger at import, usesanitize_connection_string
mssql_python/pybind/ddbc_bindings.cppEnhancedLOG templated function and updated Windows driver path logic
Comments suppressed due to low confidence (3)

mssql_python/helpers.py:189

  • The newsanitize_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

  • Thelogger 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

  • Thelogger variable is not defined in this module. Definelogger = get_logger() at the top or retrieve the logger in scope before using it.
            if logger:

@github-actionsgithub-actionsbot added pr-size: mediumModerate update size and removed pr-size: mediumModerate update size labelsJul 14, 2025
@github-actionsgithub-actionsbot added pr-size: mediumModerate update size and removed pr-size: mediumModerate update size labelsJul 16, 2025
@github-actionsgithub-actionsbot added pr-size: largeSubstantial code update and removed pr-size: mediumModerate update size labelsJul 16, 2025
@github-actionsgithub-actionsbot added pr-size: largeSubstantial code update and removed pr-size: largeSubstantial code update labelsJul 16, 2025
@github-actionsgithub-actionsbot added pr-size: largeSubstantial code update and removed pr-size: largeSubstantial code update labelsJul 16, 2025
@github-actionsgithub-actionsbot added pr-size: largeSubstantial code update and removed pr-size: largeSubstantial code update labelsJul 16, 2025
@github-actionsgithub-actionsbot added pr-size: largeSubstantial code update and removed pr-size: largeSubstantial code update labelsJul 16, 2025
@github-actionsgithub-actionsbot added pr-size: largeSubstantial code update and removed pr-size: largeSubstantial code update labelsJul 16, 2025
@github-actionsgithub-actionsbot added pr-size: largeSubstantial code update and removed pr-size: mediumModerate update size labelsJul 17, 2025
@github-actionsgithub-actionsbot added pr-size: largeSubstantial code update and removed pr-size: largeSubstantial code update labelsJul 17, 2025
@github-actionsgithub-actionsbot added pr-size: largeSubstantial code update and removed pr-size: largeSubstantial code update labelsJul 17, 2025
gargsaumya
gargsaumya previously approved these changesJul 17, 2025
jahnvi480
jahnvi480 previously approved these changesJul 17, 2025
@github-actionsgithub-actionsbot added pr-size: largeSubstantial code update and removed pr-size: largeSubstantial code update labelsJul 17, 2025
@github-actionsgithub-actionsbot added pr-size: largeSubstantial code update and removed pr-size: largeSubstantial code update labelsJul 17, 2025
@github-actionsgithub-actionsbot added pr-size: largeSubstantial code update and removed pr-size: largeSubstantial code update labelsJul 17, 2025
@jahnvi480jahnvi480 merged commita4da13a intomainJul 17, 2025
16 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

Copilot code reviewCopilotCopilot left review comments

@jahnvi480jahnvi480jahnvi480 approved these changes

@gargsaumyagargsaumyagargsaumya approved these changes

Assignees
No one assigned
Labels
pr-size: largeSubstantial code update
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@bewithgaurav@jahnvi480@gargsaumya

[8]ページ先頭

©2009-2025 Movatter.jp