From 57cfc1310e44bfadcb779d2e7620f6868b4803a1 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Mon, 14 Jul 2025 18:59:01 +0530 Subject: [PATCH 01/19] REFACTOR: Enhance & Fix Logs --- mssql_python/connection.py | 28 +++-- mssql_python/cursor.py | 10 +- mssql_python/exceptions.py | 6 +- mssql_python/helpers.py | 20 +++- mssql_python/logging_config.py | 157 ++++++++++++++++++++------ mssql_python/pybind/ddbc_bindings.cpp | 33 ++++-- tests/test_006_logging.py | 29 +++-- 7 files changed, 208 insertions(+), 75 deletions(-) diff --git a/mssql_python/connection.py b/mssql_python/connection.py index 2336ae365..72b1ea12d 100644 --- a/mssql_python/connection.py +++ b/mssql_python/connection.py @@ -11,15 +11,21 @@ - Cursors are also cleaned up automatically when no longer referenced, to prevent memory leaks. """ import weakref +import logging from mssql_python.cursor import Cursor -from mssql_python.logging_config import get_logger, ENABLE_LOGGING +from mssql_python.logging_config import get_logger, LoggingManager, setup_logging from mssql_python.constants import ConstantsDDBC as ddbc_sql_const -from mssql_python.helpers import add_driver_to_connection_str, check_error +from mssql_python.helpers import add_driver_to_connection_str, check_error, sanitize_connection_string from mssql_python import ddbc_bindings from mssql_python.pooling import PoolingManager from mssql_python.exceptions import DatabaseError, InterfaceError +# Ensure we're getting the most up-to-date logger logger = get_logger() +# If no logger is available yet, set up a default one +if logger is None: + setup_logging(log_level=logging.INFO) + logger = get_logger() class Connection: @@ -113,8 +119,8 @@ def _construct_connection_string(self, connection_str: str = "", **kwargs) -> st continue conn_str += f"{key}={value};" - if ENABLE_LOGGING: - logger.info("Final connection string: %s", conn_str) + if logger: + logger.info("Final connection string: %s", sanitize_connection_string(conn_str)) return conn_str @@ -137,7 +143,7 @@ def autocommit(self, value: bool) -> None: None """ self.setautocommit(value) - if ENABLE_LOGGING: + if logger: logger.info("Autocommit mode set to %s.", value) def setautocommit(self, value: bool = True) -> None: @@ -193,7 +199,7 @@ def commit(self) -> None: """ # Commit the current transaction self._conn.commit() - if ENABLE_LOGGING: + if logger: logger.info("Transaction committed successfully.") def rollback(self) -> None: @@ -209,7 +215,7 @@ def rollback(self) -> None: """ # Roll back the current transaction self._conn.rollback() - if ENABLE_LOGGING: + if logger: logger.info("Transaction rolled back successfully.") def close(self) -> None: @@ -242,11 +248,11 @@ def close(self) -> None: except Exception as e: # Collect errors but continue closing other cursors close_errors.append(f"Error closing cursor: {e}") - if ENABLE_LOGGING: + if logger: logger.warning(f"Error closing cursor: {e}") # If there were errors closing cursors, log them but continue - if close_errors and ENABLE_LOGGING: + if close_errors and logger: logger.warning(f"Encountered {len(close_errors)} errors while closing cursors") # Clear the cursor set explicitly to release any internal references @@ -258,7 +264,7 @@ def close(self) -> None: self._conn.close() self._conn = None except Exception as e: - if ENABLE_LOGGING: + if logger: logger.error(f"Error closing database connection: {e}") # Re-raise the connection close error as it's more critical raise @@ -266,5 +272,5 @@ def close(self) -> None: # Always mark as closed, even if there were errors self._closed = True - if ENABLE_LOGGING: + if logger: logger.info("Connection closed successfully.") diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index 6e2efc9e7..0f1911e36 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -15,7 +15,7 @@ from typing import List, Union from mssql_python.constants import ConstantsDDBC as ddbc_sql_const from mssql_python.helpers import check_error -from mssql_python.logging_config import get_logger, ENABLE_LOGGING +from mssql_python.logging_config import get_logger, LoggingManager from mssql_python import ddbc_bindings from .row import Row @@ -431,7 +431,7 @@ def _reset_cursor(self) -> None: if self.hstmt: self.hstmt.free() self.hstmt = None - if ENABLE_LOGGING: + if logger: logger.debug("SQLFreeHandle succeeded") # Reinitialize the statement handle self._initialize_cursor() @@ -449,7 +449,7 @@ def close(self) -> None: if self.hstmt: self.hstmt.free() self.hstmt = None - if ENABLE_LOGGING: + if logger: logger.debug("SQLFreeHandle succeeded") self.closed = True @@ -584,7 +584,7 @@ def execute( # Executing a new statement. Reset is_stmt_prepared to false self.is_stmt_prepared = [False] - if ENABLE_LOGGING: + if logger: logger.debug("Executing query: %s", operation) for i, param in enumerate(parameters): logger.debug( @@ -637,7 +637,7 @@ def executemany(self, operation: str, seq_of_parameters: list) -> None: total_rowcount = 0 for parameters in seq_of_parameters: parameters = list(parameters) - if ENABLE_LOGGING: + if logger: logger.info("Executing query with parameters: %s", parameters) prepare_stmt = first_execution first_execution = False diff --git a/mssql_python/exceptions.py b/mssql_python/exceptions.py index c2307a5f5..efbbda09f 100644 --- a/mssql_python/exceptions.py +++ b/mssql_python/exceptions.py @@ -4,7 +4,7 @@ This module contains custom exception classes for the mssql_python package. These classes are used to raise exceptions when an error occurs while executing a query. """ -from mssql_python.logging_config import get_logger, ENABLE_LOGGING +from mssql_python.logging_config import get_logger, LoggingManager logger = get_logger() @@ -621,7 +621,7 @@ def truncate_error_message(error_message: str) -> str: string_third = string_second[string_second.index("]") + 1 :] return string_first + string_third except Exception as e: - if ENABLE_LOGGING: + if logger: logger.error("Error while truncating error message: %s",e) return error_message @@ -641,7 +641,7 @@ def raise_exception(sqlstate: str, ddbc_error: str) -> None: """ exception_class = sqlstate_to_exception(sqlstate, ddbc_error) if exception_class: - if ENABLE_LOGGING: + if logger: logger.error(exception_class) raise exception_class raise DatabaseError( diff --git a/mssql_python/helpers.py b/mssql_python/helpers.py index cffb06467..880f9e425 100644 --- a/mssql_python/helpers.py +++ b/mssql_python/helpers.py @@ -6,7 +6,7 @@ from mssql_python import ddbc_bindings from mssql_python.exceptions import raise_exception -from mssql_python.logging_config import get_logger, ENABLE_LOGGING +from mssql_python.logging_config import get_logger import platform from pathlib import Path from mssql_python.ddbc_bindings import normalize_architecture @@ -73,7 +73,7 @@ def check_error(handle_type, handle, ret): """ if ret < 0: error_info = ddbc_bindings.DDBCSQLCheckError(handle_type, handle, ret) - if ENABLE_LOGGING: + if logger: logger.error("Error: %s", error_info.ddbcErrorMsg) raise_exception(error_info.sqlState, error_info.ddbcErrorMsg) @@ -184,3 +184,19 @@ def get_driver_path(module_dir, architecture): raise RuntimeError(f"ODBC driver not found at: {driver_path_str}") return driver_path_str + + +def sanitize_connection_string(conn_str: str) -> str: + """ + Sanitize the connection string by removing sensitive information. + + Args: + conn_str (str): The connection string to sanitize. + + Returns: + str: The sanitized connection string. + """ + # Remove sensitive information from the connection string, Pwd section + # Replace Pwd=...; or Pwd=... (end of string) with Pwd=***; + import re + return re.sub(r"(Pwd\s*=\s*)[^;]*", r"\1***", conn_str, flags=re.IGNORECASE) diff --git a/mssql_python/logging_config.py b/mssql_python/logging_config.py index d0952724f..5b8054260 100644 --- a/mssql_python/logging_config.py +++ b/mssql_python/logging_config.py @@ -8,58 +8,141 @@ from logging.handlers import RotatingFileHandler import os import sys +import datetime -ENABLE_LOGGING = False +class LoggingManager: + """ + Singleton class to manage logging configuration for the mssql_python package. + This class provides a centralized way to manage logging configuration and replaces + the previous approach using global variables. + """ + _instance = None + _initialized = False + _logger = None + _log_file = None + + def __new__(cls): + if cls._instance is None: + cls._instance = super(LoggingManager, cls).__new__(cls) + return cls._instance + + def __init__(self): + if not self._initialized: + self._initialized = True + self._enabled = False + + @classmethod + def is_logging_enabled(cls): + """Class method to check if logging is enabled for backward compatibility""" + if cls._instance is None: + return False + return cls._instance._enabled + + @property + def enabled(self): + """Check if logging is enabled""" + return self._enabled + + @property + def log_file(self): + """Get the current log file path""" + return self._log_file + + def setup(self, mode="file", log_level=logging.DEBUG): + """ + Set up logging configuration. + + This method configures the logging settings for the application. + It sets the log level, format, and log file location. + + Args: + mode (str): The logging mode ('file' or 'stdout'). + log_level (int): The logging level (default: logging.DEBUG). + """ + # Enable logging + self._enabled = True + + # Create a logger for mssql_python module + # Use a consistent logger name to ensure we're using the same logger throughout + self._logger = logging.getLogger("mssql_python") + self._logger.setLevel(log_level) + + # Configure the root logger to ensure all messages are captured + root_logger = logging.getLogger() + root_logger.setLevel(log_level) + + # Make sure the logger propagates to the root logger + self._logger.propagate = True + + # Clear any existing handlers to avoid duplicates during re-initialization + if self._logger.handlers: + self._logger.handlers.clear() + + # Construct the path to the log file + # Directory for log files - currentdir/logs + current_dir = os.path.dirname(os.path.abspath(__file__)) + log_dir = os.path.join(current_dir, 'logs') + # exist_ok=True allows the directory to be created if it doesn't exist + os.makedirs(log_dir, exist_ok=True) + + # Generate timestamp-based filename for better sorting and organization + timestamp = datetime.datetime.now().strftime("%Y%m%d_%H%M%S") + self._log_file = os.path.join(log_dir, f'mssql_python_trace_{timestamp}_{os.getpid()}.log') + + # Create a log handler to log to driver specific file + # By default we only want to log to a file, max size 500MB, and keep 5 backups + file_handler = RotatingFileHandler(self._log_file, maxBytes=512*1024*1024, backupCount=5) + file_handler.setLevel(log_level) + formatter = logging.Formatter('%(asctime)s - %(levelname)s - %(filename)s - %(message)s') + file_handler.setFormatter(formatter) + self._logger.addHandler(file_handler) + + if mode == 'stdout': + # If the mode is stdout, then we want to log to the console as well + stdout_handler = logging.StreamHandler(sys.stdout) + stdout_handler.setLevel(log_level) + stdout_handler.setFormatter(formatter) + self._logger.addHandler(stdout_handler) + elif mode != 'file': + raise ValueError(f'Invalid logging mode: {mode}') + + return self._logger + + def get_logger(self): + """ + Get the logger instance. + + Returns: + logging.Logger: The logger instance, or None if logging is not enabled. + """ + if not self.enabled: + return None + return self._logger + + +# Create a singleton instance +_manager = LoggingManager() def setup_logging(mode="file", log_level=logging.DEBUG): """ Set up logging configuration. - - This method configures the logging settings for the application. - It sets the log level, format, and log file location. - + + This is a wrapper around the LoggingManager.setup method for backward compatibility. + Args: mode (str): The logging mode ('file' or 'stdout'). log_level (int): The logging level (default: logging.DEBUG). """ - global ENABLE_LOGGING - ENABLE_LOGGING = True - - # Create a logger for mssql_python module - logger = logging.getLogger(__name__) - logger.setLevel(log_level) - - # Construct the path to the log file - # TODO: Use a different dir to dump log file - current_dir = os.path.dirname(os.path.abspath(__file__)) - log_file = os.path.join(current_dir, f'mssql_python_trace_{os.getpid()}.log') - - # Create a log handler to log to driver specific file - # By default we only want to log to a file, max size 500MB, and keep 5 backups - # TODO: Rotate files based on time too? Ex: everyday - file_handler = RotatingFileHandler(log_file, maxBytes=512*1024*1024, backupCount=5) - file_handler.setLevel(log_level) - formatter = logging.Formatter('%(asctime)s - %(levelname)s - %(filename)s - %(message)s') - file_handler.setFormatter(formatter) - logger.addHandler(file_handler) - - if mode == 'stdout': - # If the mode is stdout, then we want to log to the console as well - stdout_handler = logging.StreamHandler(sys.stdout) - stdout_handler.setLevel(log_level) - stdout_handler.setFormatter(formatter) - logger.addHandler(stdout_handler) - elif mode != 'file': - raise ValueError(f'Invalid logging mode: {mode}') + return _manager.setup(mode, log_level) def get_logger(): """ Get the logger instance. + + This is a wrapper around the LoggingManager.get_logger method for backward compatibility. Returns: logging.Logger: The logger instance. """ - if not ENABLE_LOGGING: - return None - return logging.getLogger(__name__) + return _manager.get_logger() diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index f030fdb06..afe99a004 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -530,15 +530,29 @@ void HandleZeroColumnSizeAtFetch(SQLULEN& columnSize) { // TODO: Revisit GIL considerations if we're using python's logger template void LOG(const std::string& formatString, Args&&... args) { - // TODO: Try to do this string concatenation at compile time - std::string ddbcFormatString = "[DDBC Bindings log] " + formatString; - static py::object logging = py::module_::import("mssql_python.logging_config") - .attr("get_logger")(); + // Get the logger each time to ensure we have the most up-to-date logger state + py::object logging = py::module_::import("mssql_python.logging_config").attr("get_logger")(); if (py::isinstance(logging)) { return; } - py::str message = py::str(ddbcFormatString).format(std::forward(args)...); - logging.attr("debug")(message); + + try { + // Add prefix to all logs + std::string ddbcFormatString = "[DDBC Bindings log] " + formatString; + + // Handle both formatted and non-formatted cases + if constexpr (sizeof...(args) == 0) { + // No formatting needed, just use the string directly + logging.attr("debug")(py::str(ddbcFormatString)); + } else { + // Apply formatting + py::str message = py::str(ddbcFormatString).format(std::forward(args)...); + logging.attr("debug")(message); + } + } catch (const std::exception& e) { + // Fallback in case of Python error - don't let logging errors crash the application + std::cerr << "Logging error: " << e.what() << std::endl; + } } // TODO: Add more nuanced exception classes @@ -659,7 +673,7 @@ DriverHandle LoadDriverOrThrowException() { (archStr == "arm64") ? "arm64" : "x86"; - fs::path dllDir = fs::path(moduleDir) / "libs" / archDir; + fs::path dllDir = fs::path(moduleDir) / "libs" / "windows" / archDir; fs::path authDllPath = dllDir / "mssql-auth.dll"; if (fs::exists(authDllPath)) { HMODULE hAuth = LoadLibraryW(std::wstring(authDllPath.native().begin(), authDllPath.native().end()).c_str()); @@ -779,9 +793,8 @@ void SqlHandle::free() { } SQLFreeHandle_ptr(_type, _handle); _handle = nullptr; - std::stringstream ss; - ss << "Freed SQL Handle of type: " << type_str; - LOG(ss.str()); + // Log the handle freeing directly with string concatenation instead of using stringstream + LOG("Freed SQL Handle of type: {}", type_str); } } diff --git a/tests/test_006_logging.py b/tests/test_006_logging.py index e78c29eb5..9c4565bfc 100644 --- a/tests/test_006_logging.py +++ b/tests/test_006_logging.py @@ -1,25 +1,38 @@ import logging import os import pytest -from mssql_python.logging_config import setup_logging, get_logger, ENABLE_LOGGING +from mssql_python.logging_config import setup_logging, get_logger, LoggingManager def get_log_file_path(): - repo_root_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) - pid = os.getpid() - log_file = os.path.join(repo_root_dir, "mssql_python", f"mssql_python_trace_{pid}.log") - return log_file + # Get the LoggingManager singleton instance + manager = LoggingManager() + # If logging is not enabled yet, return the default path pattern + if not manager.enabled: + repo_root_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + pid = os.getpid() + log_dir = os.path.join(repo_root_dir, "mssql_python", "logs") + os.makedirs(log_dir, exist_ok=True) + return os.path.join(log_dir, f"mssql_python_trace_{pid}.log") + # Otherwise return the actual log file path + return manager.log_file @pytest.fixture def cleanup_logger(): """Cleanup logger & log files before and after each test""" def cleanup(): + # Get the LoggingManager singleton instance + manager = LoggingManager() logger = get_logger() if logger is not None: logger.handlers.clear() log_file_path = get_log_file_path() if os.path.exists(log_file_path): os.remove(log_file_path) - ENABLE_LOGGING = False + # Reset the LoggingManager instance + manager._enabled = False + manager._initialized = False + manager._logger = None + manager._log_file = None # Perform cleanup before the test cleanup() yield @@ -29,9 +42,11 @@ def cleanup(): def test_no_logging(cleanup_logger): """Test that logging is off by default""" try: + # Get the LoggingManager singleton instance + manager = LoggingManager() logger = get_logger() assert logger is None - assert ENABLE_LOGGING == False + assert manager.enabled == False except Exception as e: pytest.fail(f"Logging not off by default. Error: {e}") From f95309e1e053fb90ac8dcd3b962150e0cf83bb2c Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Mon, 14 Jul 2025 19:23:24 +0530 Subject: [PATCH 02/19] tests and minor fix --- mssql_python/logging_config.py | 17 +++- tests/test_006_logging.py | 155 ++++++++++++++++++++++++++++++--- 2 files changed, 157 insertions(+), 15 deletions(-) diff --git a/mssql_python/logging_config.py b/mssql_python/logging_config.py index 5b8054260..78303d1a8 100644 --- a/mssql_python/logging_config.py +++ b/mssql_python/logging_config.py @@ -94,7 +94,21 @@ def setup(self, mode="file", log_level=logging.DEBUG): # By default we only want to log to a file, max size 500MB, and keep 5 backups file_handler = RotatingFileHandler(self._log_file, maxBytes=512*1024*1024, backupCount=5) file_handler.setLevel(log_level) - formatter = logging.Formatter('%(asctime)s - %(levelname)s - %(filename)s - %(message)s') + + # Create a custom formatter that adds [Python Layer log] prefix only to non-DDBC messages + class PythonLayerFormatter(logging.Formatter): + def format(self, record): + message = record.getMessage() + # Don't add [Python Layer log] prefix if the message already has [DDBC Bindings log] or [Python Layer log] + if "[DDBC Bindings log]" not in message and "[Python Layer log]" not in message: + # Create a copy of the record to avoid modifying the original + new_record = logging.makeLogRecord(record.__dict__) + new_record.msg = f"[Python Layer log] {record.msg}" + return super().format(new_record) + return super().format(record) + + # Use our custom formatter + formatter = PythonLayerFormatter('%(asctime)s - %(levelname)s - %(filename)s - %(message)s') file_handler.setFormatter(formatter) self._logger.addHandler(file_handler) @@ -102,6 +116,7 @@ def setup(self, mode="file", log_level=logging.DEBUG): # If the mode is stdout, then we want to log to the console as well stdout_handler = logging.StreamHandler(sys.stdout) stdout_handler.setLevel(log_level) + # Use the same smart formatter stdout_handler.setFormatter(formatter) self._logger.addHandler(stdout_handler) elif mode != 'file': diff --git a/tests/test_006_logging.py b/tests/test_006_logging.py index 9c4565bfc..fc9907acf 100644 --- a/tests/test_006_logging.py +++ b/tests/test_006_logging.py @@ -1,20 +1,29 @@ import logging import os import pytest +import glob from mssql_python.logging_config import setup_logging, get_logger, LoggingManager def get_log_file_path(): # Get the LoggingManager singleton instance manager = LoggingManager() - # If logging is not enabled yet, return the default path pattern - if not manager.enabled: - repo_root_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) - pid = os.getpid() - log_dir = os.path.join(repo_root_dir, "mssql_python", "logs") - os.makedirs(log_dir, exist_ok=True) - return os.path.join(log_dir, f"mssql_python_trace_{pid}.log") - # Otherwise return the actual log file path - return manager.log_file + # If logging is enabled, return the actual log file path + if manager.enabled and manager.log_file: + return manager.log_file + # For fallback/cleanup, try to find existing log files in the logs directory + repo_root_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + log_dir = os.path.join(repo_root_dir, "mssql_python", "logs") + os.makedirs(log_dir, exist_ok=True) + + # Try to find existing log files + log_files = glob.glob(os.path.join(log_dir, "mssql_python_trace_*.log")) + if log_files: + # Return the most recently created log file + return max(log_files, key=os.path.getctime) + + # Fallback to default pattern + pid = os.getpid() + return os.path.join(log_dir, f"mssql_python_trace_{pid}.log") @pytest.fixture def cleanup_logger(): @@ -25,9 +34,15 @@ def cleanup(): logger = get_logger() if logger is not None: logger.handlers.clear() - log_file_path = get_log_file_path() - if os.path.exists(log_file_path): - os.remove(log_file_path) + + # Try to remove the actual log file if it exists + try: + log_file_path = get_log_file_path() + if os.path.exists(log_file_path): + os.remove(log_file_path) + except: + pass # Ignore errors during cleanup + # Reset the LoggingManager instance manager._enabled = False manager._initialized = False @@ -56,7 +71,8 @@ def test_setup_logging(cleanup_logger): setup_logging() # This must enable logging logger = get_logger() assert logger is not None - assert logger == logging.getLogger('mssql_python.logging_config') + # Fix: Check for the correct logger name + assert logger == logging.getLogger('mssql_python') assert logger.level == logging.DEBUG # DEBUG level except Exception as e: pytest.fail(f"Logging setup failed: {e}") @@ -99,4 +115,115 @@ def test_logging_in_stdout_mode(cleanup_logger, capsys): captured_stdout = capsys.readouterr().out assert test_message in captured_stdout, "Log message not found in stdout" except Exception as e: - pytest.fail(f"Logging in stdout mode failed: {e}") \ No newline at end of file + pytest.fail(f"Logging in stdout mode failed: {e}") + +def test_python_layer_prefix(cleanup_logger): + """Test that Python layer logs have the correct prefix""" + try: + setup_logging() + logger = get_logger() + assert logger is not None + + # Log a test message + test_message = "This is a Python layer test message" + logger.info(test_message) + + # Check if the log file contains the message with [Python Layer log] prefix + log_file_path = get_log_file_path() + with open(log_file_path, 'r') as f: + log_content = f.read() + + # The logged message should have the Python Layer prefix + assert "[Python Layer log]" in log_content, "Python Layer log prefix not found" + assert test_message in log_content, "Test message not found in log file" + except Exception as e: + pytest.fail(f"Python layer prefix test failed: {e}") + +def test_different_log_levels(cleanup_logger): + """Test that different log levels work correctly""" + try: + setup_logging() + logger = get_logger() + assert logger is not None + + # Log messages at different levels + debug_msg = "This is a DEBUG message" + info_msg = "This is an INFO message" + warning_msg = "This is a WARNING message" + error_msg = "This is an ERROR message" + + logger.debug(debug_msg) + logger.info(info_msg) + logger.warning(warning_msg) + logger.error(error_msg) + + # Check if the log file contains all messages + log_file_path = get_log_file_path() + with open(log_file_path, 'r') as f: + log_content = f.read() + + assert debug_msg in log_content, "DEBUG message not found in log file" + assert info_msg in log_content, "INFO message not found in log file" + assert warning_msg in log_content, "WARNING message not found in log file" + assert error_msg in log_content, "ERROR message not found in log file" + + # Also check for level indicators in the log + assert "DEBUG" in log_content, "DEBUG level not found in log file" + assert "INFO" in log_content, "INFO level not found in log file" + assert "WARNING" in log_content, "WARNING level not found in log file" + assert "ERROR" in log_content, "ERROR level not found in log file" + except Exception as e: + pytest.fail(f"Log levels test failed: {e}") + +def test_singleton_behavior(cleanup_logger): + """Test that LoggingManager behaves as a singleton""" + try: + # Create multiple instances of LoggingManager + manager1 = LoggingManager() + manager2 = LoggingManager() + + # They should be the same instance + assert manager1 is manager2, "LoggingManager instances are not the same" + + # Enable logging through one instance + manager1._enabled = True + + # The other instance should reflect this change + assert manager2.enabled == True, "Singleton state not shared between instances" + + # Reset for cleanup + manager1._enabled = False + except Exception as e: + pytest.fail(f"Singleton behavior test failed: {e}") + +def test_timestamp_in_log_filename(cleanup_logger): + """Test that log filenames include timestamps""" + try: + setup_logging() + + # Get the log file path + log_file_path = get_log_file_path() + filename = os.path.basename(log_file_path) + + # Extract parts of the filename + parts = filename.split('_') + + # The filename should follow the pattern: mssql_python_trace_YYYYMMDD_HHMMSS_PID.log + # Fix: Account for the fact that "mssql_python" contains an underscore + assert parts[0] == "mssql", "Incorrect filename prefix part 1" + assert parts[1] == "python", "Incorrect filename prefix part 2" + assert parts[2] == "trace", "Incorrect filename part" + + # Check date format (YYYYMMDD) + date_part = parts[3] + assert len(date_part) == 8 and date_part.isdigit(), "Date format incorrect in filename" + + # Check time format (HHMMSS) + time_part = parts[4] + assert len(time_part) == 6 and time_part.isdigit(), "Time format incorrect in filename" + + # Process ID should be the last part before .log + pid_part = parts[5].split('.')[0] + assert pid_part.isdigit(), "Process ID not found in filename" + except Exception as e: + pytest.fail(f"Timestamp in filename test failed: {e}") \ No newline at end of file From 9534896ae8a83a3a93250ff70abf483c4c32967a Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 16 Jul 2025 12:43:00 +0530 Subject: [PATCH 03/19] restoring branch --- mssql_python/auth.py | 168 +++++++++++++++++++++++++ mssql_python/connection.py | 41 +++--- mssql_python/constants.py | 6 + mssql_python/cursor.py | 34 +---- mssql_python/exceptions.py | 2 +- mssql_python/helpers.py | 16 --- mssql_python/logging_config.py | 172 ++++++------------------- mssql_python/pybind/ddbc_bindings.cpp | 33 ++--- tests/test_003_connection.py | 172 ++++++++++++++++++++++++- tests/test_005_exceptions.py | 139 ++++++++++++++++++++ tests/test_006_logging.py | 86 +++++++++++++ tests/test_007_auth.py | 175 ++++++++++++++++++++++++++ 12 files changed, 816 insertions(+), 228 deletions(-) create mode 100644 mssql_python/auth.py create mode 100644 tests/test_005_exceptions.py create mode 100644 tests/test_006_logging.py create mode 100644 tests/test_007_auth.py diff --git a/mssql_python/auth.py b/mssql_python/auth.py new file mode 100644 index 000000000..d67bc5efc --- /dev/null +++ b/mssql_python/auth.py @@ -0,0 +1,168 @@ +""" +Copyright (c) Microsoft Corporation. +Licensed under the MIT license. +This module handles authentication for the mssql_python package. +""" + +import platform +import struct +from typing import Tuple, Dict, Optional, Union +from mssql_python.logging_config import get_logger +from mssql_python.constants import AuthType + +logger = get_logger() + +class AADAuth: + """Handles Azure Active Directory authentication""" + + @staticmethod + def get_token_struct(token: str) -> bytes: + """Convert token to SQL Server compatible format""" + token_bytes = token.encode("UTF-16-LE") + return struct.pack(f" bytes: + """Get token using DefaultAzureCredential""" + from azure.identity import DefaultAzureCredential + + try: + # DefaultAzureCredential will automatically use the best available method + # based on the environment (e.g., managed identity, environment variables) + credential = DefaultAzureCredential() + token = credential.get_token("https://database.windows.net/.default").token + return AADAuth.get_token_struct(token) + except Exception as e: + raise RuntimeError(f"Failed to create DefaultAzureCredential: {e}") + + @staticmethod + def get_device_code_token() -> bytes: + """Get token using DeviceCodeCredential""" + from azure.identity import DeviceCodeCredential + + try: + credential = DeviceCodeCredential() + token = credential.get_token("https://database.windows.net/.default").token + return AADAuth.get_token_struct(token) + except Exception as e: + raise RuntimeError(f"Failed to create DeviceCodeCredential: {e}") + + @staticmethod + def get_interactive_token() -> bytes: + """Get token using InteractiveBrowserCredential""" + from azure.identity import InteractiveBrowserCredential + + try: + credential = InteractiveBrowserCredential() + token = credential.get_token("https://database.windows.net/.default").token + return AADAuth.get_token_struct(token) + except Exception as e: + raise RuntimeError(f"Failed to create InteractiveBrowserCredential: {e}") + +def process_auth_parameters(parameters: list) -> Tuple[list, Optional[str]]: + """ + Process connection parameters and extract authentication type. + + Args: + parameters: List of connection string parameters + + Returns: + Tuple[list, Optional[str]]: Modified parameters and authentication type + + Raises: + ValueError: If an invalid authentication type is provided + """ + modified_parameters = [] + auth_type = None + + for param in parameters: + param = param.strip() + if not param: + continue + + if "=" not in param: + modified_parameters.append(param) + continue + + key, value = param.split("=", 1) + key_lower = key.lower() + value_lower = value.lower() + + if key_lower == "authentication": + # Check for supported authentication types and set auth_type accordingly + if value_lower == AuthType.INTERACTIVE.value: + # Interactive authentication (browser-based); only append parameter for non-Windows + if platform.system().lower() == "windows": + continue # Skip adding this parameter for Windows + auth_type = "interactive" + elif value_lower == AuthType.DEVICE_CODE.value: + # Device code authentication (for devices without browser) + auth_type = "devicecode" + elif value_lower == AuthType.DEFAULT.value: + # Default authentication (uses DefaultAzureCredential) + auth_type = "default" + modified_parameters.append(param) + + return modified_parameters, auth_type + +def remove_sensitive_params(parameters: list) -> list: + """Remove sensitive parameters from connection string""" + exclude_keys = [ + "uid=", "pwd=", "encrypt=", "trustservercertificate=", "authentication=" + ] + return [ + param for param in parameters + if not any(param.lower().startswith(exclude) for exclude in exclude_keys) + ] + +def get_auth_token(auth_type: str) -> Optional[bytes]: + """Get authentication token based on auth type""" + if not auth_type: + return None + + if auth_type == "default": + return AADAuth.get_default_token() + elif auth_type == "devicecode": + return AADAuth.get_device_code_token() + # If interactive authentication is requested, use InteractiveBrowserCredential + # but only if not on Windows, since in Windows: AADInteractive is supported. + elif auth_type == "interactive" and platform.system().lower() != "windows": + return AADAuth.get_interactive_token() + return None + +def process_connection_string(connection_string: str) -> Tuple[str, Optional[Dict]]: + """ + Process connection string and handle authentication. + + Args: + connection_string: The connection string to process + + Returns: + Tuple[str, Optional[Dict]]: Processed connection string and attrs_before dict if needed + + Raises: + ValueError: If the connection string is invalid or empty + """ + # Check type first + if not isinstance(connection_string, str): + raise ValueError("Connection string must be a string") + + # Then check if empty + if not connection_string: + raise ValueError("Connection string cannot be empty") + + parameters = connection_string.split(";") + + # Validate that there's at least one valid parameter + if not any('=' in param for param in parameters): + raise ValueError("Invalid connection string format") + + modified_parameters, auth_type = process_auth_parameters(parameters) + + if auth_type: + modified_parameters = remove_sensitive_params(modified_parameters) + token_struct = get_auth_token(auth_type) + if token_struct: + return ";".join(modified_parameters) + ";", {1256: token_struct} + + return ";".join(modified_parameters) + ";", None \ No newline at end of file diff --git a/mssql_python/connection.py b/mssql_python/connection.py index 2ff05e289..71e8d209d 100644 --- a/mssql_python/connection.py +++ b/mssql_python/connection.py @@ -11,21 +11,17 @@ - Cursors are also cleaned up automatically when no longer referenced, to prevent memory leaks. """ import weakref -import logging +import re from mssql_python.cursor import Cursor -from mssql_python.logging_config import get_logger, LoggingManager, setup_logging +from mssql_python.logging_config import get_logger from mssql_python.constants import ConstantsDDBC as ddbc_sql_const -from mssql_python.helpers import add_driver_to_connection_str, check_error, sanitize_connection_string +from mssql_python.helpers import add_driver_to_connection_str, check_error from mssql_python import ddbc_bindings from mssql_python.pooling import PoolingManager from mssql_python.exceptions import DatabaseError, InterfaceError +from mssql_python.auth import process_connection_string -# Ensure we're getting the most up-to-date logger logger = get_logger() -# If no logger is available yet, set up a default one -if logger is None: - setup_logging(log_level=logging.INFO) - logger = get_logger() class Connection: @@ -70,6 +66,17 @@ def __init__(self, connection_str: str = "", autocommit: bool = False, attrs_bef connection_str, **kwargs ) self._attrs_before = attrs_before or {} + + # Check if the connection string contains authentication parameters + # This is important for processing the connection string correctly. + # If authentication is specified, it will be processed to handle + # different authentication types like interactive, device code, etc. + if re.search(r"authentication", self.connection_str, re.IGNORECASE): + connection_result = process_connection_string(self.connection_str) + self.connection_str = connection_result[0] + if connection_result[1]: + self._attrs_before.update(connection_result[1]) + self._closed = False # Using WeakSet which automatically removes cursors when they are no longer in use @@ -120,7 +127,7 @@ def _construct_connection_string(self, connection_str: str = "", **kwargs) -> st conn_str += f"{key}={value};" if logger: - logger.info("Final connection string: %s", sanitize_connection_string(conn_str)) + logger.info("Final connection string: %s", conn_str) return conn_str @@ -182,6 +189,7 @@ def cursor(self) -> Cursor: ) cursor = Cursor(self) + self._cursors.add(cursor) # Track the cursor return cursor def commit(self) -> None: @@ -239,7 +247,7 @@ def close(self) -> None: # Convert to list to avoid modification during iteration cursors_to_close = list(self._cursors) close_errors = [] - + for cursor in cursors_to_close: try: if not cursor.closed: @@ -273,16 +281,3 @@ def close(self) -> None: if logger: logger.info("Connection closed successfully.") - - def __del__(self): - """ - Destructor to ensure the connection is closed when the connection object is no longer needed. - This is a safety net to ensure resources are cleaned up - even if close() was not called explicitly. - """ - if not self._closed: - try: - self.close() - except Exception as e: - if ENABLE_LOGGING: - logger.error(f"Error during connection cleanup in __del__: {e}") diff --git a/mssql_python/constants.py b/mssql_python/constants.py index aade503c7..81e60d37e 100644 --- a/mssql_python/constants.py +++ b/mssql_python/constants.py @@ -116,3 +116,9 @@ class ConstantsDDBC(Enum): SQL_C_WCHAR = -8 SQL_NULLABLE = 1 SQL_MAX_NUMERIC_LEN = 16 + +class AuthType(Enum): + """Constants for authentication types""" + INTERACTIVE = "activedirectoryinteractive" + DEVICE_CODE = "activedirectorydevicecode" + DEFAULT = "activedirectorydefault" \ No newline at end of file diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index c254378c3..d30efcdbe 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -15,7 +15,7 @@ from typing import List, Union from mssql_python.constants import ConstantsDDBC as ddbc_sql_const from mssql_python.helpers import check_error -from mssql_python.logging_config import get_logger, LoggingManager +from mssql_python.logging_config import get_logger from mssql_python import ddbc_bindings from .row import Row @@ -416,10 +416,7 @@ def _initialize_cursor(self) -> None: """ Initialize the DDBC statement handle. """ - # Allocate the DDBC statement handle self._allocate_statement_handle() - # Add the cursor to the connection's cursor set - self.connection._cursors.add(self) def _allocate_statement_handle(self): """ @@ -427,25 +424,15 @@ def _allocate_statement_handle(self): """ self.hstmt = self.connection._conn.alloc_statement_handle() - def _free_cursor(self) -> None: + def _reset_cursor(self) -> None: """ - Free the DDBC statement handle and remove the cursor from the connection's cursor set. + Reset the DDBC statement handle. """ if self.hstmt: self.hstmt.free() self.hstmt = None if logger: logger.debug("SQLFreeHandle succeeded") - # We don't need to remove the cursor from the connection's cursor set here, - # as it is a weak reference and will be automatically removed - # when the cursor is garbage collected. - - def _reset_cursor(self) -> None: - """ - Reset the DDBC statement handle. - """ - # Free the current cursor if it exists - self._free_cursor() # Reinitialize the statement handle self._initialize_cursor() @@ -719,6 +706,7 @@ def fetchall(self) -> List[Row]: # Fetch raw data rows_data = [] ret = ddbc_bindings.DDBCSQLFetchAll(self.hstmt, rows_data) + # Convert raw data to Row objects return [Row(row_data, self.description) for row_data in rows_data] @@ -739,16 +727,4 @@ def nextset(self) -> Union[bool, None]: check_error(ddbc_sql_const.SQL_HANDLE_STMT.value, self.hstmt, ret) if ret == ddbc_sql_const.SQL_NO_DATA.value: return False - return True - - def __del__(self): - """ - Destructor to ensure the cursor is closed when it is no longer needed. - This is a safety net to ensure resources are cleaned up - even if close() was not called explicitly. - """ - if not self.closed: - try: - self.close() - except Exception as e: - logger.error(f"Error closing cursor: {e}") + return True \ No newline at end of file diff --git a/mssql_python/exceptions.py b/mssql_python/exceptions.py index efbbda09f..308a85690 100644 --- a/mssql_python/exceptions.py +++ b/mssql_python/exceptions.py @@ -4,7 +4,7 @@ This module contains custom exception classes for the mssql_python package. These classes are used to raise exceptions when an error occurs while executing a query. """ -from mssql_python.logging_config import get_logger, LoggingManager +from mssql_python.logging_config import get_logger logger = get_logger() diff --git a/mssql_python/helpers.py b/mssql_python/helpers.py index 880f9e425..cecfc39dc 100644 --- a/mssql_python/helpers.py +++ b/mssql_python/helpers.py @@ -184,19 +184,3 @@ def get_driver_path(module_dir, architecture): raise RuntimeError(f"ODBC driver not found at: {driver_path_str}") return driver_path_str - - -def sanitize_connection_string(conn_str: str) -> str: - """ - Sanitize the connection string by removing sensitive information. - - Args: - conn_str (str): The connection string to sanitize. - - Returns: - str: The sanitized connection string. - """ - # Remove sensitive information from the connection string, Pwd section - # Replace Pwd=...; or Pwd=... (end of string) with Pwd=***; - import re - return re.sub(r"(Pwd\s*=\s*)[^;]*", r"\1***", conn_str, flags=re.IGNORECASE) diff --git a/mssql_python/logging_config.py b/mssql_python/logging_config.py index 78303d1a8..d8d45cbca 100644 --- a/mssql_python/logging_config.py +++ b/mssql_python/logging_config.py @@ -8,156 +8,58 @@ from logging.handlers import RotatingFileHandler import os import sys -import datetime +logger = False -class LoggingManager: - """ - Singleton class to manage logging configuration for the mssql_python package. - This class provides a centralized way to manage logging configuration and replaces - the previous approach using global variables. - """ - _instance = None - _initialized = False - _logger = None - _log_file = None - - def __new__(cls): - if cls._instance is None: - cls._instance = super(LoggingManager, cls).__new__(cls) - return cls._instance - - def __init__(self): - if not self._initialized: - self._initialized = True - self._enabled = False - - @classmethod - def is_logging_enabled(cls): - """Class method to check if logging is enabled for backward compatibility""" - if cls._instance is None: - return False - return cls._instance._enabled - - @property - def enabled(self): - """Check if logging is enabled""" - return self._enabled - - @property - def log_file(self): - """Get the current log file path""" - return self._log_file - - def setup(self, mode="file", log_level=logging.DEBUG): - """ - Set up logging configuration. - - This method configures the logging settings for the application. - It sets the log level, format, and log file location. - - Args: - mode (str): The logging mode ('file' or 'stdout'). - log_level (int): The logging level (default: logging.DEBUG). - """ - # Enable logging - self._enabled = True - - # Create a logger for mssql_python module - # Use a consistent logger name to ensure we're using the same logger throughout - self._logger = logging.getLogger("mssql_python") - self._logger.setLevel(log_level) - - # Configure the root logger to ensure all messages are captured - root_logger = logging.getLogger() - root_logger.setLevel(log_level) - - # Make sure the logger propagates to the root logger - self._logger.propagate = True - - # Clear any existing handlers to avoid duplicates during re-initialization - if self._logger.handlers: - self._logger.handlers.clear() - - # Construct the path to the log file - # Directory for log files - currentdir/logs - current_dir = os.path.dirname(os.path.abspath(__file__)) - log_dir = os.path.join(current_dir, 'logs') - # exist_ok=True allows the directory to be created if it doesn't exist - os.makedirs(log_dir, exist_ok=True) - - # Generate timestamp-based filename for better sorting and organization - timestamp = datetime.datetime.now().strftime("%Y%m%d_%H%M%S") - self._log_file = os.path.join(log_dir, f'mssql_python_trace_{timestamp}_{os.getpid()}.log') - - # Create a log handler to log to driver specific file - # By default we only want to log to a file, max size 500MB, and keep 5 backups - file_handler = RotatingFileHandler(self._log_file, maxBytes=512*1024*1024, backupCount=5) - file_handler.setLevel(log_level) - - # Create a custom formatter that adds [Python Layer log] prefix only to non-DDBC messages - class PythonLayerFormatter(logging.Formatter): - def format(self, record): - message = record.getMessage() - # Don't add [Python Layer log] prefix if the message already has [DDBC Bindings log] or [Python Layer log] - if "[DDBC Bindings log]" not in message and "[Python Layer log]" not in message: - # Create a copy of the record to avoid modifying the original - new_record = logging.makeLogRecord(record.__dict__) - new_record.msg = f"[Python Layer log] {record.msg}" - return super().format(new_record) - return super().format(record) - - # Use our custom formatter - formatter = PythonLayerFormatter('%(asctime)s - %(levelname)s - %(filename)s - %(message)s') - file_handler.setFormatter(formatter) - self._logger.addHandler(file_handler) - - if mode == 'stdout': - # If the mode is stdout, then we want to log to the console as well - stdout_handler = logging.StreamHandler(sys.stdout) - stdout_handler.setLevel(log_level) - # Use the same smart formatter - stdout_handler.setFormatter(formatter) - self._logger.addHandler(stdout_handler) - elif mode != 'file': - raise ValueError(f'Invalid logging mode: {mode}') - - return self._logger - - def get_logger(self): - """ - Get the logger instance. - - Returns: - logging.Logger: The logger instance, or None if logging is not enabled. - """ - if not self.enabled: - return None - return self._logger - - -# Create a singleton instance -_manager = LoggingManager() def setup_logging(mode="file", log_level=logging.DEBUG): """ Set up logging configuration. - - This is a wrapper around the LoggingManager.setup method for backward compatibility. - + + This method configures the logging settings for the application. + It sets the log level, format, and log file location. + Args: mode (str): The logging mode ('file' or 'stdout'). log_level (int): The logging level (default: logging.DEBUG). """ - return _manager.setup(mode, log_level) + global logger + logger = True + + # Create a logger for mssql_python module + logger = logging.getLogger(__name__) + logger.setLevel(log_level) + + # Construct the path to the log file + # TODO: Use a different dir to dump log file + current_dir = os.path.dirname(os.path.abspath(__file__)) + log_file = os.path.join(current_dir, f'mssql_python_trace_{os.getpid()}.log') + + # Create a log handler to log to driver specific file + # By default we only want to log to a file, max size 500MB, and keep 5 backups + # TODO: Rotate files based on time too? Ex: everyday + file_handler = RotatingFileHandler(log_file, maxBytes=512*1024*1024, backupCount=5) + file_handler.setLevel(log_level) + formatter = logging.Formatter('%(asctime)s - %(levelname)s - %(filename)s - %(message)s') + file_handler.setFormatter(formatter) + logger.addHandler(file_handler) + + if mode == 'stdout': + # If the mode is stdout, then we want to log to the console as well + stdout_handler = logging.StreamHandler(sys.stdout) + stdout_handler.setLevel(log_level) + stdout_handler.setFormatter(formatter) + logger.addHandler(stdout_handler) + elif mode != 'file': + raise ValueError(f'Invalid logging mode: {mode}') def get_logger(): """ Get the logger instance. - - This is a wrapper around the LoggingManager.get_logger method for backward compatibility. Returns: logging.Logger: The logger instance. """ - return _manager.get_logger() + if not logger: + return None + return logging.getLogger(__name__) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index afe99a004..f030fdb06 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -530,29 +530,15 @@ void HandleZeroColumnSizeAtFetch(SQLULEN& columnSize) { // TODO: Revisit GIL considerations if we're using python's logger template void LOG(const std::string& formatString, Args&&... args) { - // Get the logger each time to ensure we have the most up-to-date logger state - py::object logging = py::module_::import("mssql_python.logging_config").attr("get_logger")(); + // TODO: Try to do this string concatenation at compile time + std::string ddbcFormatString = "[DDBC Bindings log] " + formatString; + static py::object logging = py::module_::import("mssql_python.logging_config") + .attr("get_logger")(); if (py::isinstance(logging)) { return; } - - try { - // Add prefix to all logs - std::string ddbcFormatString = "[DDBC Bindings log] " + formatString; - - // Handle both formatted and non-formatted cases - if constexpr (sizeof...(args) == 0) { - // No formatting needed, just use the string directly - logging.attr("debug")(py::str(ddbcFormatString)); - } else { - // Apply formatting - py::str message = py::str(ddbcFormatString).format(std::forward(args)...); - logging.attr("debug")(message); - } - } catch (const std::exception& e) { - // Fallback in case of Python error - don't let logging errors crash the application - std::cerr << "Logging error: " << e.what() << std::endl; - } + py::str message = py::str(ddbcFormatString).format(std::forward(args)...); + logging.attr("debug")(message); } // TODO: Add more nuanced exception classes @@ -673,7 +659,7 @@ DriverHandle LoadDriverOrThrowException() { (archStr == "arm64") ? "arm64" : "x86"; - fs::path dllDir = fs::path(moduleDir) / "libs" / "windows" / archDir; + fs::path dllDir = fs::path(moduleDir) / "libs" / archDir; fs::path authDllPath = dllDir / "mssql-auth.dll"; if (fs::exists(authDllPath)) { HMODULE hAuth = LoadLibraryW(std::wstring(authDllPath.native().begin(), authDllPath.native().end()).c_str()); @@ -793,8 +779,9 @@ void SqlHandle::free() { } SQLFreeHandle_ptr(_type, _handle); _handle = nullptr; - // Log the handle freeing directly with string concatenation instead of using stringstream - LOG("Freed SQL Handle of type: {}", type_str); + std::stringstream ss; + ss << "Freed SQL Handle of type: " << type_str; + LOG(ss.str()); } } diff --git a/tests/test_003_connection.py b/tests/test_003_connection.py index 8fd0a8213..6da2dee5a 100644 --- a/tests/test_003_connection.py +++ b/tests/test_003_connection.py @@ -10,10 +10,11 @@ Note: The cursor function is not yet implemented, so related tests are commented out. """ +from mssql_python.exceptions import InterfaceError import pytest import time from mssql_python import Connection, connect, pooling - + def drop_table_if_exists(cursor, table_name): """Drop the table if it exists""" try: @@ -223,3 +224,172 @@ def test_connection_pooling_basic(conn_str): conn1.close() conn2.close() + +# Add these tests at the end of the file + +def test_cursor_cleanup_on_connection_close(conn_str): + """Test that cursors are properly cleaned up when connection is closed""" + # Create a new connection for this test + conn = connect(conn_str) + + # Create multiple cursors + cursor1 = conn.cursor() + cursor2 = conn.cursor() + cursor3 = conn.cursor() + + # Execute something on each cursor to ensure they have statement handles + # Option 1: Fetch results immediately to free the connection + cursor1.execute("SELECT 1") + cursor1.fetchall() + + cursor2.execute("SELECT 2") + cursor2.fetchall() + + cursor3.execute("SELECT 3") + cursor3.fetchall() + + # Close one cursor explicitly + cursor1.close() + assert cursor1.closed is True, "Cursor1 should be closed" + + # Close the connection (should clean up remaining cursors) + conn.close() + + # Verify all cursors are closed + assert cursor1.closed is True, "Cursor1 should remain closed" + assert cursor2.closed is True, "Cursor2 should be closed by connection.close()" + assert cursor3.closed is True, "Cursor3 should be closed by connection.close()" + +def test_cursor_weakref_cleanup(conn_str): + """Test that WeakSet properly removes garbage collected cursors""" + conn = connect(conn_str) + + # Create cursors + cursor1 = conn.cursor() + cursor2 = conn.cursor() + + # Check initial cursor count + assert len(conn._cursors) == 2, "Should have 2 cursors" + + # Delete reference to cursor1 (should be garbage collected) + cursor1_id = id(cursor1) + del cursor1 + + # Force garbage collection + import gc + gc.collect() + + # Check cursor count after garbage collection + assert len(conn._cursors) == 1, "Should have 1 cursor after garbage collection" + + # Verify cursor2 is still there + assert cursor2 in conn._cursors, "Cursor2 should still be in the set" + + conn.close() + +def test_cursor_cleanup_order_no_segfault(conn_str): + """Test that proper cleanup order prevents segfaults""" + # This test ensures cursors are cleaned before connection + conn = connect(conn_str) + + # Create multiple cursors with active statements + cursors = [] + for i in range(5): + cursor = conn.cursor() + cursor.execute(f"SELECT {i}") + cursor.fetchall() + cursors.append(cursor) + + # Don't close any cursors explicitly + # Just close the connection - it should handle cleanup properly + conn.close() + + # Verify all cursors were closed + for cursor in cursors: + assert cursor.closed is True, "All cursors should be closed" + +def test_cursor_close_removes_from_connection(conn_str): + """Test that closing a cursor properly cleans up references""" + conn = connect(conn_str) + + # Create cursors + cursor1 = conn.cursor() + cursor2 = conn.cursor() + cursor3 = conn.cursor() + + assert len(conn._cursors) == 3, "Should have 3 cursors" + + # Close cursor2 + cursor2.close() + + # cursor2 should still be in the WeakSet (until garbage collected) + # but it should be marked as closed + assert cursor2.closed is True, "Cursor2 should be closed" + + # Delete the reference and force garbage collection + del cursor2 + import gc + gc.collect() + + # Now should have 2 cursors + assert len(conn._cursors) == 2, "Should have 2 cursors after closing and GC" + + conn.close() + +def test_connection_close_idempotent(conn_str): + """Test that calling close() multiple times is safe""" + conn = connect(conn_str) + cursor = conn.cursor() + cursor.execute("SELECT 1") + + # First close + conn.close() + assert conn._closed is True, "Connection should be closed" + + # Second close (should not raise exception) + conn.close() + assert conn._closed is True, "Connection should remain closed" + + # Cursor should also be closed + assert cursor.closed is True, "Cursor should be closed" + +def test_cursor_after_connection_close(conn_str): + """Test that creating cursor after connection close raises error""" + conn = connect(conn_str) + conn.close() + + # Should raise exception when trying to create cursor on closed connection + with pytest.raises(InterfaceError) as excinfo: + cursor = conn.cursor() + + assert "closed connection" in str(excinfo.value).lower(), "Should mention closed connection" + +def test_multiple_cursor_operations_cleanup(conn_str): + """Test cleanup with multiple cursor operations""" + conn = connect(conn_str) + + # Create table for testing + cursor_setup = conn.cursor() + drop_table_if_exists(cursor_setup, "#test_cleanup") + cursor_setup.execute("CREATE TABLE #test_cleanup (id INT, value VARCHAR(50))") + cursor_setup.close() + + # Create multiple cursors doing different operations + cursor_insert = conn.cursor() + cursor_insert.execute("INSERT INTO #test_cleanup VALUES (1, 'test1'), (2, 'test2')") + + cursor_select1 = conn.cursor() + cursor_select1.execute("SELECT * FROM #test_cleanup WHERE id = 1") + cursor_select1.fetchall() + + cursor_select2 = conn.cursor() + cursor_select2.execute("SELECT * FROM #test_cleanup WHERE id = 2") + cursor_select2.fetchall() + + # Close connection without closing cursors + conn.close() + + # All cursors should be closed + assert cursor_insert.closed is True + assert cursor_select1.closed is True + assert cursor_select2.closed is True \ No newline at end of file diff --git a/tests/test_005_exceptions.py b/tests/test_005_exceptions.py new file mode 100644 index 000000000..2bc97cbe4 --- /dev/null +++ b/tests/test_005_exceptions.py @@ -0,0 +1,139 @@ +import pytest +from mssql_python import connect, Connection +from mssql_python.exceptions import ( + Exception, + Warning, + Error, + InterfaceError, + DatabaseError, + DataError, + OperationalError, + IntegrityError, + InternalError, + ProgrammingError, + NotSupportedError, + raise_exception, + truncate_error_message +) + +def drop_table_if_exists(cursor, table_name): + """Drop the table if it exists""" + try: + cursor.execute(f"DROP TABLE IF EXISTS {table_name}") + except Exception as e: + pytest.fail(f"Failed to drop table {table_name}: {e}") + +def test_truncate_error_message(cursor): + with pytest.raises(ProgrammingError) as excinfo: + cursor.execute("SELEC database_id, name from sys.databases;") + assert str(excinfo.value) == "Driver Error: Syntax error or access violation; DDBC Error: [Microsoft][SQL Server]Incorrect syntax near the keyword 'from'." + +def test_raise_exception(): + with pytest.raises(ProgrammingError) as excinfo: + raise_exception('42000', 'Syntax error or access violation') + assert str(excinfo.value) == "Driver Error: Syntax error or access violation; DDBC Error: Syntax error or access violation" + +def test_warning_exception(): + with pytest.raises(Warning) as excinfo: + raise_exception('01000', 'General warning') + assert str(excinfo.value) == "Driver Error: General warning; DDBC Error: General warning" + +def test_data_error_exception(): + with pytest.raises(DataError) as excinfo: + raise_exception('22003', 'Numeric value out of range') + assert str(excinfo.value) == "Driver Error: Numeric value out of range; DDBC Error: Numeric value out of range" + +def test_operational_error_exception(): + with pytest.raises(OperationalError) as excinfo: + raise_exception('08001', 'Client unable to establish connection') + assert str(excinfo.value) == "Driver Error: Client unable to establish connection; DDBC Error: Client unable to establish connection" + +def test_integrity_error_exception(): + with pytest.raises(IntegrityError) as excinfo: + raise_exception('23000', 'Integrity constraint violation') + assert str(excinfo.value) == "Driver Error: Integrity constraint violation; DDBC Error: Integrity constraint violation" + +def test_internal_error_exception(): + with pytest.raises(IntegrityError) as excinfo: + raise_exception('40002', 'Integrity constraint violation') + assert str(excinfo.value) == "Driver Error: Integrity constraint violation; DDBC Error: Integrity constraint violation" + +def test_programming_error_exception(): + with pytest.raises(ProgrammingError) as excinfo: + raise_exception('42S02', 'Base table or view not found') + assert str(excinfo.value) == "Driver Error: Base table or view not found; DDBC Error: Base table or view not found" + +def test_not_supported_error_exception(): + with pytest.raises(NotSupportedError) as excinfo: + raise_exception('IM001', 'Driver does not support this function') + assert str(excinfo.value) == "Driver Error: Driver does not support this function; DDBC Error: Driver does not support this function" + +def test_unknown_error_exception(): + with pytest.raises(DatabaseError) as excinfo: + raise_exception('99999', 'Unknown error') + assert str(excinfo.value) == "Driver Error: An error occurred with SQLSTATE code: 99999; DDBC Error: Unknown error" + +def test_syntax_error(cursor): + with pytest.raises(ProgrammingError) as excinfo: + cursor.execute("SELEC * FROM non_existent_table") + assert "Syntax error or access violation" in str(excinfo.value) + +def test_table_not_found_error(cursor): + with pytest.raises(ProgrammingError) as excinfo: + cursor.execute("SELECT * FROM non_existent_table") + assert "Base table or view not found" in str(excinfo.value) + +def test_data_truncation_error(cursor, db_connection): + try: + cursor.execute("CREATE TABLE #pytest_test_truncation (id INT, name NVARCHAR(5))") + cursor.execute("INSERT INTO #pytest_test_truncation (id, name) VALUES (?, ?)", [1, 'TooLongName']) + except (ProgrammingError, DataError) as excinfo: + # DataError is raised on Windows but ProgrammingError on MacOS + # Included catching both ProgrammingError and DataError in this test + # TODO: Make this test platform independent + assert "String or binary data would be truncated" in str(excinfo) + finally: + drop_table_if_exists(cursor, "#pytest_test_truncation") + db_connection.commit() + +def test_unique_constraint_error(cursor, db_connection): + try: + drop_table_if_exists(cursor, "#pytest_test_unique") + cursor.execute("CREATE TABLE #pytest_test_unique (id INT PRIMARY KEY, name NVARCHAR(50))") + cursor.execute("INSERT INTO #pytest_test_unique (id, name) VALUES (?, ?)", [1, 'Name1']) + with pytest.raises(IntegrityError) as excinfo: + cursor.execute("INSERT INTO #pytest_test_unique (id, name) VALUES (?, ?)", [1, 'Name2']) + assert "Integrity constraint violation" in str(excinfo.value) + except Exception as e: + pytest.fail(f"Test failed: {e}") + finally: + drop_table_if_exists(cursor, "#pytest_test_unique") + db_connection.commit() + +def test_foreign_key_constraint_error(cursor, db_connection): + try: + # Using regular tables (not temp tables) because SQL Server doesn't support foreign keys on temp tables. + # Using dbo schema to avoid issues with Azure SQL with Azure AD/Entra ID authentication. It can misinterpret email-format usernames (e.g., user@domain.com) as schema names. + drop_table_if_exists(cursor, "dbo.pytest_child_table") + drop_table_if_exists(cursor, "dbo.pytest_parent_table") + cursor.execute("CREATE TABLE dbo.pytest_parent_table (id INT PRIMARY KEY)") + cursor.execute("CREATE TABLE dbo.pytest_child_table (id INT, parent_id INT, FOREIGN KEY (parent_id) REFERENCES dbo.pytest_parent_table(id))") + cursor.execute("INSERT INTO dbo.pytest_parent_table (id) VALUES (?)", [1]) + with pytest.raises(IntegrityError) as excinfo: + cursor.execute("INSERT INTO dbo.pytest_child_table (id, parent_id) VALUES (?, ?)", [1, 2]) + assert "Integrity constraint violation" in str(excinfo.value) + except Exception as e: + pytest.fail(f"Test failed: {e}") + finally: + drop_table_if_exists(cursor, "dbo.pytest_child_table") + drop_table_if_exists(cursor, "dbo.pytest_parent_table") + db_connection.commit() + +def test_connection_error(): + # RuntimeError is raised on Windows, while on MacOS it raises OperationalError + # In MacOS the error goes by "Client unable to establish connection" + # In Windows it goes by "Neither DSN nor SERVER keyword supplied" + # TODO: Make this test platform independent + with pytest.raises((RuntimeError, OperationalError)) as excinfo: + connect("InvalidConnectionString") + assert "Client unable to establish connection" in str(excinfo.value) or "Neither DSN nor SERVER keyword supplied" in str(excinfo.value) \ No newline at end of file diff --git a/tests/test_006_logging.py b/tests/test_006_logging.py new file mode 100644 index 000000000..a32185d6a --- /dev/null +++ b/tests/test_006_logging.py @@ -0,0 +1,86 @@ +import logging +import os +import pytest +from mssql_python.logging_config import setup_logging, get_logger, logger + +def get_log_file_path(): + repo_root_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + pid = os.getpid() + log_file = os.path.join(repo_root_dir, "mssql_python", f"mssql_python_trace_{pid}.log") + return log_file + +@pytest.fixture +def cleanup_logger(): + """Cleanup logger & log files before and after each test""" + def cleanup(): + logger = get_logger() + if logger is not None: + logger.handlers.clear() + log_file_path = get_log_file_path() + if os.path.exists(log_file_path): + os.remove(log_file_path) + logger = False + # Perform cleanup before the test + cleanup() + yield + # Perform cleanup after the test + cleanup() + +def test_no_logging(cleanup_logger): + """Test that logging is off by default""" + try: + logger = get_logger() + assert logger is None + except Exception as e: + pytest.fail(f"Logging not off by default. Error: {e}") + +def test_setup_logging(cleanup_logger): + """Test if logging is set up correctly""" + try: + setup_logging() # This must enable logging + logger = get_logger() + assert logger is not None + assert logger == logging.getLogger('mssql_python.logging_config') + assert logger.level == logging.DEBUG # DEBUG level + except Exception as e: + pytest.fail(f"Logging setup failed: {e}") + +def test_logging_in_file_mode(cleanup_logger): + """Test if logging works correctly in file mode""" + try: + setup_logging() + logger = get_logger() + assert logger is not None + # Log a test message + test_message = "Testing file logging mode" + logger.info(test_message) + # Check if the log file is created and contains the test message + log_file_path = get_log_file_path() + assert os.path.exists(log_file_path), "Log file not created" + # open the log file and check its content + with open(log_file_path, 'r') as f: + log_content = f.read() + assert test_message in log_content, "Log message not found in log file" + except Exception as e: + pytest.fail(f"Logging in file mode failed: {e}") + +def test_logging_in_stdout_mode(cleanup_logger, capsys): + """Test if logging works correctly in stdout mode""" + try: + setup_logging('stdout') + logger = get_logger() + assert logger is not None + # Log a test message + test_message = "Testing file + stdout logging mode" + logger.info(test_message) + # Check if the log file is created and contains the test message + log_file_path = get_log_file_path() + assert os.path.exists(log_file_path), "Log file not created in file+stdout mode" + with open(log_file_path, 'r') as f: + log_content = f.read() + assert test_message in log_content, "Log message not found in log file" + # Check if the message is printed to stdout + captured_stdout = capsys.readouterr().out + assert test_message in captured_stdout, "Log message not found in stdout" + except Exception as e: + pytest.fail(f"Logging in stdout mode failed: {e}") \ No newline at end of file diff --git a/tests/test_007_auth.py b/tests/test_007_auth.py new file mode 100644 index 000000000..52cea8178 --- /dev/null +++ b/tests/test_007_auth.py @@ -0,0 +1,175 @@ +""" +Copyright (c) Microsoft Corporation. +Licensed under the MIT license. +Tests for the auth module. +""" + +import pytest +import platform +import sys +from mssql_python.auth import ( + AADAuth, + process_auth_parameters, + remove_sensitive_params, + get_auth_token, + process_connection_string +) +from mssql_python.constants import AuthType + +# Test data +SAMPLE_TOKEN = "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsIng1dCI6I" + +@pytest.fixture(autouse=True) +def setup_azure_identity(): + """Setup mock azure.identity module""" + class MockToken: + token = SAMPLE_TOKEN + + class MockDefaultAzureCredential: + def get_token(self, scope): + return MockToken() + + class MockDeviceCodeCredential: + def get_token(self, scope): + return MockToken() + + class MockInteractiveBrowserCredential: + def get_token(self, scope): + return MockToken() + + class MockIdentity: + DefaultAzureCredential = MockDefaultAzureCredential + DeviceCodeCredential = MockDeviceCodeCredential + InteractiveBrowserCredential = MockInteractiveBrowserCredential + + # Create mock azure module if it doesn't exist + if 'azure' not in sys.modules: + sys.modules['azure'] = type('MockAzure', (), {})() + + # Add identity module to azure + sys.modules['azure.identity'] = MockIdentity() + + yield + + # Cleanup + if 'azure.identity' in sys.modules: + del sys.modules['azure.identity'] + +class TestAuthType: + def test_auth_type_constants(self): + assert AuthType.INTERACTIVE.value == "activedirectoryinteractive" + assert AuthType.DEVICE_CODE.value == "activedirectorydevicecode" + assert AuthType.DEFAULT.value == "activedirectorydefault" + +class TestAADAuth: + def test_get_token_struct(self): + token_struct = AADAuth.get_token_struct(SAMPLE_TOKEN) + assert isinstance(token_struct, bytes) + assert len(token_struct) > 4 + + def test_get_default_token(self): + token_struct = AADAuth.get_default_token() + assert isinstance(token_struct, bytes) + + def test_get_device_code_token(self): + token_struct = AADAuth.get_device_code_token() + assert isinstance(token_struct, bytes) + + def test_get_interactive_token(self): + token_struct = AADAuth.get_interactive_token() + assert isinstance(token_struct, bytes) + +class TestProcessAuthParameters: + def test_empty_parameters(self): + modified_params, auth_type = process_auth_parameters([]) + assert modified_params == [] + assert auth_type is None + + def test_interactive_auth_windows(self, monkeypatch): + monkeypatch.setattr(platform, "system", lambda: "Windows") + params = ["Authentication=ActiveDirectoryInteractive", "Server=test"] + modified_params, auth_type = process_auth_parameters(params) + assert "Authentication=ActiveDirectoryInteractive" not in modified_params + assert auth_type == None + + def test_interactive_auth_non_windows(self, monkeypatch): + monkeypatch.setattr(platform, "system", lambda: "Darwin") + params = ["Authentication=ActiveDirectoryInteractive", "Server=test"] + modified_params, auth_type = process_auth_parameters(params) + assert "Authentication=ActiveDirectoryInteractive" in modified_params + assert auth_type == "interactive" + + def test_device_code_auth(self): + params = ["Authentication=ActiveDirectoryDeviceCode", "Server=test"] + modified_params, auth_type = process_auth_parameters(params) + assert auth_type == "devicecode" + + def test_default_auth(self): + params = ["Authentication=ActiveDirectoryDefault", "Server=test"] + modified_params, auth_type = process_auth_parameters(params) + assert auth_type == "default" + +class TestRemoveSensitiveParams: + def test_remove_sensitive_parameters(self): + params = [ + "Server=test", + "UID=user", + "PWD=password", + "Encrypt=yes", + "TrustServerCertificate=yes", + "Authentication=ActiveDirectoryDefault", + "Database=testdb" + ] + filtered_params = remove_sensitive_params(params) + assert "Server=test" in filtered_params + assert "Database=testdb" in filtered_params + assert "UID=user" not in filtered_params + assert "PWD=password" not in filtered_params + assert "Encrypt=yes" not in filtered_params + assert "TrustServerCertificate=yes" not in filtered_params + assert "Authentication=ActiveDirectoryDefault" not in filtered_params + +class TestProcessConnectionString: + def test_process_connection_string_with_default_auth(self): + conn_str = "Server=test;Authentication=ActiveDirectoryDefault;Database=testdb" + result_str, attrs = process_connection_string(conn_str) + + assert "Server=test" in result_str + assert "Database=testdb" in result_str + assert attrs is not None + assert 1256 in attrs + assert isinstance(attrs[1256], bytes) + + def test_process_connection_string_no_auth(self): + conn_str = "Server=test;Database=testdb;UID=user;PWD=password" + result_str, attrs = process_connection_string(conn_str) + + assert "Server=test" in result_str + assert "Database=testdb" in result_str + assert "UID=user" in result_str + assert "PWD=password" in result_str + assert attrs is None + + def test_process_connection_string_interactive_non_windows(self, monkeypatch): + monkeypatch.setattr(platform, "system", lambda: "Darwin") + conn_str = "Server=test;Authentication=ActiveDirectoryInteractive;Database=testdb" + result_str, attrs = process_connection_string(conn_str) + + assert "Server=test" in result_str + assert "Database=testdb" in result_str + assert attrs is not None + assert 1256 in attrs + assert isinstance(attrs[1256], bytes) + +def test_error_handling(): + # Empty string should raise ValueError + with pytest.raises(ValueError, match="Connection string cannot be empty"): + process_connection_string("") + + # Invalid connection string should raise ValueError + with pytest.raises(ValueError, match="Invalid connection string format"): + process_connection_string("InvalidConnectionString") + + # Test non-string input + with pytest.raises(ValueError, match="Connection string must be a string"): + process_connection_string(None) \ No newline at end of file From 0ed415f6708f3016d8471a9b1412a3aa98a82470 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 16 Jul 2025 12:45:05 +0530 Subject: [PATCH 04/19] restoring ddbc bindings for branch: --- mssql_python/pybind/ddbc_bindings.cpp | 33 +++++++++++++++++++-------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index f030fdb06..afe99a004 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -530,15 +530,29 @@ void HandleZeroColumnSizeAtFetch(SQLULEN& columnSize) { // TODO: Revisit GIL considerations if we're using python's logger template void LOG(const std::string& formatString, Args&&... args) { - // TODO: Try to do this string concatenation at compile time - std::string ddbcFormatString = "[DDBC Bindings log] " + formatString; - static py::object logging = py::module_::import("mssql_python.logging_config") - .attr("get_logger")(); + // Get the logger each time to ensure we have the most up-to-date logger state + py::object logging = py::module_::import("mssql_python.logging_config").attr("get_logger")(); if (py::isinstance(logging)) { return; } - py::str message = py::str(ddbcFormatString).format(std::forward(args)...); - logging.attr("debug")(message); + + try { + // Add prefix to all logs + std::string ddbcFormatString = "[DDBC Bindings log] " + formatString; + + // Handle both formatted and non-formatted cases + if constexpr (sizeof...(args) == 0) { + // No formatting needed, just use the string directly + logging.attr("debug")(py::str(ddbcFormatString)); + } else { + // Apply formatting + py::str message = py::str(ddbcFormatString).format(std::forward(args)...); + logging.attr("debug")(message); + } + } catch (const std::exception& e) { + // Fallback in case of Python error - don't let logging errors crash the application + std::cerr << "Logging error: " << e.what() << std::endl; + } } // TODO: Add more nuanced exception classes @@ -659,7 +673,7 @@ DriverHandle LoadDriverOrThrowException() { (archStr == "arm64") ? "arm64" : "x86"; - fs::path dllDir = fs::path(moduleDir) / "libs" / archDir; + fs::path dllDir = fs::path(moduleDir) / "libs" / "windows" / archDir; fs::path authDllPath = dllDir / "mssql-auth.dll"; if (fs::exists(authDllPath)) { HMODULE hAuth = LoadLibraryW(std::wstring(authDllPath.native().begin(), authDllPath.native().end()).c_str()); @@ -779,9 +793,8 @@ void SqlHandle::free() { } SQLFreeHandle_ptr(_type, _handle); _handle = nullptr; - std::stringstream ss; - ss << "Freed SQL Handle of type: " << type_str; - LOG(ss.str()); + // Log the handle freeing directly with string concatenation instead of using stringstream + LOG("Freed SQL Handle of type: {}", type_str); } } From 7ba4115f0d1b7e6aeb9699f3fd52b6f0a8a2a9dd Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 16 Jul 2025 13:05:30 +0530 Subject: [PATCH 05/19] restoring logging_config --- mssql_python/logging_config.py | 172 ++++++++++++++++++++++++++------- 1 file changed, 135 insertions(+), 37 deletions(-) diff --git a/mssql_python/logging_config.py b/mssql_python/logging_config.py index d8d45cbca..9439f32ca 100644 --- a/mssql_python/logging_config.py +++ b/mssql_python/logging_config.py @@ -8,58 +8,156 @@ from logging.handlers import RotatingFileHandler import os import sys +import datetime -logger = False +class LoggingManager: + """ + Singleton class to manage logging configuration for the mssql_python package. + This class provides a centralized way to manage logging configuration and replaces + the previous approach using global variables. + """ + _instance = None + _initialized = False + _logger = None + _log_file = None + + def __new__(cls): + if cls._instance is None: + cls._instance = super(LoggingManager, cls).__new__(cls) + return cls._instance + + def __init__(self): + if not self._initialized: + self._initialized = True + self._enabled = False + + @classmethod + def is_logging_enabled(cls): + """Class method to check if logging is enabled for backward compatibility""" + if cls._instance is None: + return False + return cls._instance._enabled + + @property + def enabled(self): + """Check if logging is enabled""" + return self._enabled + + @property + def log_file(self): + """Get the current log file path""" + return self._log_file + + def setup(self, mode="file", log_level=logging.DEBUG): + """ + Set up logging configuration. + + This method configures the logging settings for the application. + It sets the log level, format, and log file location. + + Args: + mode (str): The logging mode ('file' or 'stdout'). + log_level (int): The logging level (default: logging.DEBUG). + """ + # Enable logging + self._enabled = True + + # Create a logger for mssql_python module + # Use a consistent logger name to ensure we're using the same logger throughout + self._logger = logging.getLogger("mssql_python") + self._logger.setLevel(log_level) + + # Configure the root logger to ensure all messages are captured + root_logger = logging.getLogger() + root_logger.setLevel(log_level) + + # Make sure the logger propagates to the root logger + self._logger.propagate = True + + # Clear any existing handlers to avoid duplicates during re-initialization + if self._logger.handlers: + self._logger.handlers.clear() + + # Construct the path to the log file + # Directory for log files - currentdir/logs + current_dir = os.path.dirname(os.path.abspath(__file__)) + log_dir = os.path.join(current_dir, 'logs') + # exist_ok=True allows the directory to be created if it doesn't exist + os.makedirs(log_dir, exist_ok=True) + + # Generate timestamp-based filename for better sorting and organization + timestamp = datetime.datetime.now().strftime("%Y%m%d_%H%M%S") + self._log_file = os.path.join(log_dir, f'mssql_python_trace_{timestamp}_{os.getpid()}.log') + + # Create a log handler to log to driver specific file + # By default we only want to log to a file, max size 500MB, and keep 5 backups + file_handler = RotatingFileHandler(self._log_file, maxBytes=512*1024*1024, backupCount=5) + file_handler.setLevel(log_level) + + # Create a custom formatter that adds [Python Layer log] prefix only to non-DDBC messages + class PythonLayerFormatter(logging.Formatter): + def format(self, record): + message = record.getMessage() + # Don't add [Python Layer log] prefix if the message already has [DDBC Bindings log] or [Python Layer log] + if "[DDBC Bindings log]" not in message and "[Python Layer log]" not in message: + # Create a copy of the record to avoid modifying the original + new_record = logging.makeLogRecord(record.__dict__) + new_record.msg = f"[Python Layer log] {record.msg}" + return super().format(new_record) + return super().format(record) + + # Use our custom formatter + formatter = PythonLayerFormatter('%(asctime)s - %(levelname)s - %(filename)s - %(message)s') + file_handler.setFormatter(formatter) + self._logger.addHandler(file_handler) + + if mode == 'stdout': + # If the mode is stdout, then we want to log to the console as well + stdout_handler = logging.StreamHandler(sys.stdout) + stdout_handler.setLevel(log_level) + # Use the same smart formatter + stdout_handler.setFormatter(formatter) + self._logger.addHandler(stdout_handler) + elif mode != 'file': + raise ValueError(f'Invalid logging mode: {mode}') + + return self._logger + + def get_logger(self): + """ + Get the logger instance. + + Returns: + logging.Logger: The logger instance, or None if logging is not enabled. + """ + if not self.enabled: + return None + return self._logger + + +# Create a singleton instance +_manager = LoggingManager() def setup_logging(mode="file", log_level=logging.DEBUG): """ Set up logging configuration. - - This method configures the logging settings for the application. - It sets the log level, format, and log file location. - + + This is a wrapper around the LoggingManager.setup method for backward compatibility. + Args: mode (str): The logging mode ('file' or 'stdout'). log_level (int): The logging level (default: logging.DEBUG). """ - global logger - logger = True - - # Create a logger for mssql_python module - logger = logging.getLogger(__name__) - logger.setLevel(log_level) - - # Construct the path to the log file - # TODO: Use a different dir to dump log file - current_dir = os.path.dirname(os.path.abspath(__file__)) - log_file = os.path.join(current_dir, f'mssql_python_trace_{os.getpid()}.log') - - # Create a log handler to log to driver specific file - # By default we only want to log to a file, max size 500MB, and keep 5 backups - # TODO: Rotate files based on time too? Ex: everyday - file_handler = RotatingFileHandler(log_file, maxBytes=512*1024*1024, backupCount=5) - file_handler.setLevel(log_level) - formatter = logging.Formatter('%(asctime)s - %(levelname)s - %(filename)s - %(message)s') - file_handler.setFormatter(formatter) - logger.addHandler(file_handler) - - if mode == 'stdout': - # If the mode is stdout, then we want to log to the console as well - stdout_handler = logging.StreamHandler(sys.stdout) - stdout_handler.setLevel(log_level) - stdout_handler.setFormatter(formatter) - logger.addHandler(stdout_handler) - elif mode != 'file': - raise ValueError(f'Invalid logging mode: {mode}') + return _manager.setup(mode, log_level) def get_logger(): """ Get the logger instance. + + This is a wrapper around the LoggingManager.get_logger method for backward compatibility. Returns: logging.Logger: The logger instance. """ - if not logger: - return None - return logging.getLogger(__name__) + return _manager.get_logger() \ No newline at end of file From 0562f8ad4d1c9c7d3576840dacd28f6e4377b998 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 16 Jul 2025 13:07:48 +0530 Subject: [PATCH 06/19] restoring logging_other files --- mssql_python/connection.py | 4 ++-- mssql_python/helpers.py | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/mssql_python/connection.py b/mssql_python/connection.py index 71e8d209d..ce3acbdb6 100644 --- a/mssql_python/connection.py +++ b/mssql_python/connection.py @@ -15,7 +15,7 @@ from mssql_python.cursor import Cursor from mssql_python.logging_config import get_logger from mssql_python.constants import ConstantsDDBC as ddbc_sql_const -from mssql_python.helpers import add_driver_to_connection_str, check_error +from mssql_python.helpers import add_driver_to_connection_str, sanitize_connection_string from mssql_python import ddbc_bindings from mssql_python.pooling import PoolingManager from mssql_python.exceptions import DatabaseError, InterfaceError @@ -127,7 +127,7 @@ def _construct_connection_string(self, connection_str: str = "", **kwargs) -> st conn_str += f"{key}={value};" if logger: - logger.info("Final connection string: %s", conn_str) + logger.info("Final connection string: %s", sanitize_connection_string(conn_str)) return conn_str diff --git a/mssql_python/helpers.py b/mssql_python/helpers.py index cecfc39dc..b82e99378 100644 --- a/mssql_python/helpers.py +++ b/mssql_python/helpers.py @@ -184,3 +184,17 @@ def get_driver_path(module_dir, architecture): raise RuntimeError(f"ODBC driver not found at: {driver_path_str}") return driver_path_str + + +def sanitize_connection_string(conn_str: str) -> str: + """ + Sanitize the connection string by removing sensitive information. + Args: + conn_str (str): The connection string to sanitize. + Returns: + str: The sanitized connection string. + """ + # Remove sensitive information from the connection string, Pwd section + # Replace Pwd=...; or Pwd=... (end of string) with Pwd=***; + import re + return re.sub(r"(Pwd\s*=\s*)[^;]*", r"\1***", conn_str, flags=re.IGNORECASE) \ No newline at end of file From 1b67d51c96d72ff116cc563e7d4cde89cdd4e43b Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 16 Jul 2025 13:49:38 +0530 Subject: [PATCH 07/19] eliminated auth changes --- mssql_python/auth.py | 168 ----------------------------------- mssql_python/connection.py | 12 +-- mssql_python/constants.py | 6 -- tests/test_007_auth.py | 175 ------------------------------------- 4 files changed, 1 insertion(+), 360 deletions(-) delete mode 100644 mssql_python/auth.py delete mode 100644 tests/test_007_auth.py diff --git a/mssql_python/auth.py b/mssql_python/auth.py deleted file mode 100644 index d67bc5efc..000000000 --- a/mssql_python/auth.py +++ /dev/null @@ -1,168 +0,0 @@ -""" -Copyright (c) Microsoft Corporation. -Licensed under the MIT license. -This module handles authentication for the mssql_python package. -""" - -import platform -import struct -from typing import Tuple, Dict, Optional, Union -from mssql_python.logging_config import get_logger -from mssql_python.constants import AuthType - -logger = get_logger() - -class AADAuth: - """Handles Azure Active Directory authentication""" - - @staticmethod - def get_token_struct(token: str) -> bytes: - """Convert token to SQL Server compatible format""" - token_bytes = token.encode("UTF-16-LE") - return struct.pack(f" bytes: - """Get token using DefaultAzureCredential""" - from azure.identity import DefaultAzureCredential - - try: - # DefaultAzureCredential will automatically use the best available method - # based on the environment (e.g., managed identity, environment variables) - credential = DefaultAzureCredential() - token = credential.get_token("https://database.windows.net/.default").token - return AADAuth.get_token_struct(token) - except Exception as e: - raise RuntimeError(f"Failed to create DefaultAzureCredential: {e}") - - @staticmethod - def get_device_code_token() -> bytes: - """Get token using DeviceCodeCredential""" - from azure.identity import DeviceCodeCredential - - try: - credential = DeviceCodeCredential() - token = credential.get_token("https://database.windows.net/.default").token - return AADAuth.get_token_struct(token) - except Exception as e: - raise RuntimeError(f"Failed to create DeviceCodeCredential: {e}") - - @staticmethod - def get_interactive_token() -> bytes: - """Get token using InteractiveBrowserCredential""" - from azure.identity import InteractiveBrowserCredential - - try: - credential = InteractiveBrowserCredential() - token = credential.get_token("https://database.windows.net/.default").token - return AADAuth.get_token_struct(token) - except Exception as e: - raise RuntimeError(f"Failed to create InteractiveBrowserCredential: {e}") - -def process_auth_parameters(parameters: list) -> Tuple[list, Optional[str]]: - """ - Process connection parameters and extract authentication type. - - Args: - parameters: List of connection string parameters - - Returns: - Tuple[list, Optional[str]]: Modified parameters and authentication type - - Raises: - ValueError: If an invalid authentication type is provided - """ - modified_parameters = [] - auth_type = None - - for param in parameters: - param = param.strip() - if not param: - continue - - if "=" not in param: - modified_parameters.append(param) - continue - - key, value = param.split("=", 1) - key_lower = key.lower() - value_lower = value.lower() - - if key_lower == "authentication": - # Check for supported authentication types and set auth_type accordingly - if value_lower == AuthType.INTERACTIVE.value: - # Interactive authentication (browser-based); only append parameter for non-Windows - if platform.system().lower() == "windows": - continue # Skip adding this parameter for Windows - auth_type = "interactive" - elif value_lower == AuthType.DEVICE_CODE.value: - # Device code authentication (for devices without browser) - auth_type = "devicecode" - elif value_lower == AuthType.DEFAULT.value: - # Default authentication (uses DefaultAzureCredential) - auth_type = "default" - modified_parameters.append(param) - - return modified_parameters, auth_type - -def remove_sensitive_params(parameters: list) -> list: - """Remove sensitive parameters from connection string""" - exclude_keys = [ - "uid=", "pwd=", "encrypt=", "trustservercertificate=", "authentication=" - ] - return [ - param for param in parameters - if not any(param.lower().startswith(exclude) for exclude in exclude_keys) - ] - -def get_auth_token(auth_type: str) -> Optional[bytes]: - """Get authentication token based on auth type""" - if not auth_type: - return None - - if auth_type == "default": - return AADAuth.get_default_token() - elif auth_type == "devicecode": - return AADAuth.get_device_code_token() - # If interactive authentication is requested, use InteractiveBrowserCredential - # but only if not on Windows, since in Windows: AADInteractive is supported. - elif auth_type == "interactive" and platform.system().lower() != "windows": - return AADAuth.get_interactive_token() - return None - -def process_connection_string(connection_string: str) -> Tuple[str, Optional[Dict]]: - """ - Process connection string and handle authentication. - - Args: - connection_string: The connection string to process - - Returns: - Tuple[str, Optional[Dict]]: Processed connection string and attrs_before dict if needed - - Raises: - ValueError: If the connection string is invalid or empty - """ - # Check type first - if not isinstance(connection_string, str): - raise ValueError("Connection string must be a string") - - # Then check if empty - if not connection_string: - raise ValueError("Connection string cannot be empty") - - parameters = connection_string.split(";") - - # Validate that there's at least one valid parameter - if not any('=' in param for param in parameters): - raise ValueError("Invalid connection string format") - - modified_parameters, auth_type = process_auth_parameters(parameters) - - if auth_type: - modified_parameters = remove_sensitive_params(modified_parameters) - token_struct = get_auth_token(auth_type) - if token_struct: - return ";".join(modified_parameters) + ";", {1256: token_struct} - - return ";".join(modified_parameters) + ";", None \ No newline at end of file diff --git a/mssql_python/connection.py b/mssql_python/connection.py index ce3acbdb6..72a37e328 100644 --- a/mssql_python/connection.py +++ b/mssql_python/connection.py @@ -66,17 +66,7 @@ def __init__(self, connection_str: str = "", autocommit: bool = False, attrs_bef connection_str, **kwargs ) self._attrs_before = attrs_before or {} - - # Check if the connection string contains authentication parameters - # This is important for processing the connection string correctly. - # If authentication is specified, it will be processed to handle - # different authentication types like interactive, device code, etc. - if re.search(r"authentication", self.connection_str, re.IGNORECASE): - connection_result = process_connection_string(self.connection_str) - self.connection_str = connection_result[0] - if connection_result[1]: - self._attrs_before.update(connection_result[1]) - + self._closed = False # Using WeakSet which automatically removes cursors when they are no longer in use diff --git a/mssql_python/constants.py b/mssql_python/constants.py index 81e60d37e..aade503c7 100644 --- a/mssql_python/constants.py +++ b/mssql_python/constants.py @@ -116,9 +116,3 @@ class ConstantsDDBC(Enum): SQL_C_WCHAR = -8 SQL_NULLABLE = 1 SQL_MAX_NUMERIC_LEN = 16 - -class AuthType(Enum): - """Constants for authentication types""" - INTERACTIVE = "activedirectoryinteractive" - DEVICE_CODE = "activedirectorydevicecode" - DEFAULT = "activedirectorydefault" \ No newline at end of file diff --git a/tests/test_007_auth.py b/tests/test_007_auth.py deleted file mode 100644 index 52cea8178..000000000 --- a/tests/test_007_auth.py +++ /dev/null @@ -1,175 +0,0 @@ -""" -Copyright (c) Microsoft Corporation. -Licensed under the MIT license. -Tests for the auth module. -""" - -import pytest -import platform -import sys -from mssql_python.auth import ( - AADAuth, - process_auth_parameters, - remove_sensitive_params, - get_auth_token, - process_connection_string -) -from mssql_python.constants import AuthType - -# Test data -SAMPLE_TOKEN = "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsIng1dCI6I" - -@pytest.fixture(autouse=True) -def setup_azure_identity(): - """Setup mock azure.identity module""" - class MockToken: - token = SAMPLE_TOKEN - - class MockDefaultAzureCredential: - def get_token(self, scope): - return MockToken() - - class MockDeviceCodeCredential: - def get_token(self, scope): - return MockToken() - - class MockInteractiveBrowserCredential: - def get_token(self, scope): - return MockToken() - - class MockIdentity: - DefaultAzureCredential = MockDefaultAzureCredential - DeviceCodeCredential = MockDeviceCodeCredential - InteractiveBrowserCredential = MockInteractiveBrowserCredential - - # Create mock azure module if it doesn't exist - if 'azure' not in sys.modules: - sys.modules['azure'] = type('MockAzure', (), {})() - - # Add identity module to azure - sys.modules['azure.identity'] = MockIdentity() - - yield - - # Cleanup - if 'azure.identity' in sys.modules: - del sys.modules['azure.identity'] - -class TestAuthType: - def test_auth_type_constants(self): - assert AuthType.INTERACTIVE.value == "activedirectoryinteractive" - assert AuthType.DEVICE_CODE.value == "activedirectorydevicecode" - assert AuthType.DEFAULT.value == "activedirectorydefault" - -class TestAADAuth: - def test_get_token_struct(self): - token_struct = AADAuth.get_token_struct(SAMPLE_TOKEN) - assert isinstance(token_struct, bytes) - assert len(token_struct) > 4 - - def test_get_default_token(self): - token_struct = AADAuth.get_default_token() - assert isinstance(token_struct, bytes) - - def test_get_device_code_token(self): - token_struct = AADAuth.get_device_code_token() - assert isinstance(token_struct, bytes) - - def test_get_interactive_token(self): - token_struct = AADAuth.get_interactive_token() - assert isinstance(token_struct, bytes) - -class TestProcessAuthParameters: - def test_empty_parameters(self): - modified_params, auth_type = process_auth_parameters([]) - assert modified_params == [] - assert auth_type is None - - def test_interactive_auth_windows(self, monkeypatch): - monkeypatch.setattr(platform, "system", lambda: "Windows") - params = ["Authentication=ActiveDirectoryInteractive", "Server=test"] - modified_params, auth_type = process_auth_parameters(params) - assert "Authentication=ActiveDirectoryInteractive" not in modified_params - assert auth_type == None - - def test_interactive_auth_non_windows(self, monkeypatch): - monkeypatch.setattr(platform, "system", lambda: "Darwin") - params = ["Authentication=ActiveDirectoryInteractive", "Server=test"] - modified_params, auth_type = process_auth_parameters(params) - assert "Authentication=ActiveDirectoryInteractive" in modified_params - assert auth_type == "interactive" - - def test_device_code_auth(self): - params = ["Authentication=ActiveDirectoryDeviceCode", "Server=test"] - modified_params, auth_type = process_auth_parameters(params) - assert auth_type == "devicecode" - - def test_default_auth(self): - params = ["Authentication=ActiveDirectoryDefault", "Server=test"] - modified_params, auth_type = process_auth_parameters(params) - assert auth_type == "default" - -class TestRemoveSensitiveParams: - def test_remove_sensitive_parameters(self): - params = [ - "Server=test", - "UID=user", - "PWD=password", - "Encrypt=yes", - "TrustServerCertificate=yes", - "Authentication=ActiveDirectoryDefault", - "Database=testdb" - ] - filtered_params = remove_sensitive_params(params) - assert "Server=test" in filtered_params - assert "Database=testdb" in filtered_params - assert "UID=user" not in filtered_params - assert "PWD=password" not in filtered_params - assert "Encrypt=yes" not in filtered_params - assert "TrustServerCertificate=yes" not in filtered_params - assert "Authentication=ActiveDirectoryDefault" not in filtered_params - -class TestProcessConnectionString: - def test_process_connection_string_with_default_auth(self): - conn_str = "Server=test;Authentication=ActiveDirectoryDefault;Database=testdb" - result_str, attrs = process_connection_string(conn_str) - - assert "Server=test" in result_str - assert "Database=testdb" in result_str - assert attrs is not None - assert 1256 in attrs - assert isinstance(attrs[1256], bytes) - - def test_process_connection_string_no_auth(self): - conn_str = "Server=test;Database=testdb;UID=user;PWD=password" - result_str, attrs = process_connection_string(conn_str) - - assert "Server=test" in result_str - assert "Database=testdb" in result_str - assert "UID=user" in result_str - assert "PWD=password" in result_str - assert attrs is None - - def test_process_connection_string_interactive_non_windows(self, monkeypatch): - monkeypatch.setattr(platform, "system", lambda: "Darwin") - conn_str = "Server=test;Authentication=ActiveDirectoryInteractive;Database=testdb" - result_str, attrs = process_connection_string(conn_str) - - assert "Server=test" in result_str - assert "Database=testdb" in result_str - assert attrs is not None - assert 1256 in attrs - assert isinstance(attrs[1256], bytes) - -def test_error_handling(): - # Empty string should raise ValueError - with pytest.raises(ValueError, match="Connection string cannot be empty"): - process_connection_string("") - - # Invalid connection string should raise ValueError - with pytest.raises(ValueError, match="Invalid connection string format"): - process_connection_string("InvalidConnectionString") - - # Test non-string input - with pytest.raises(ValueError, match="Connection string must be a string"): - process_connection_string(None) \ No newline at end of file From 558cdd6114e6190e480a0f91d7c0aa7107fb8d0b Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 16 Jul 2025 13:50:43 +0530 Subject: [PATCH 08/19] logging tests only --- tests/test_006_exceptions.py | 139 --------------------- tests/test_006_logging.py | 161 ++++++++++++++++++++++-- tests/test_007_logging.py | 229 ----------------------------------- 3 files changed, 152 insertions(+), 377 deletions(-) delete mode 100644 tests/test_006_exceptions.py delete mode 100644 tests/test_007_logging.py diff --git a/tests/test_006_exceptions.py b/tests/test_006_exceptions.py deleted file mode 100644 index 2bc97cbe4..000000000 --- a/tests/test_006_exceptions.py +++ /dev/null @@ -1,139 +0,0 @@ -import pytest -from mssql_python import connect, Connection -from mssql_python.exceptions import ( - Exception, - Warning, - Error, - InterfaceError, - DatabaseError, - DataError, - OperationalError, - IntegrityError, - InternalError, - ProgrammingError, - NotSupportedError, - raise_exception, - truncate_error_message -) - -def drop_table_if_exists(cursor, table_name): - """Drop the table if it exists""" - try: - cursor.execute(f"DROP TABLE IF EXISTS {table_name}") - except Exception as e: - pytest.fail(f"Failed to drop table {table_name}: {e}") - -def test_truncate_error_message(cursor): - with pytest.raises(ProgrammingError) as excinfo: - cursor.execute("SELEC database_id, name from sys.databases;") - assert str(excinfo.value) == "Driver Error: Syntax error or access violation; DDBC Error: [Microsoft][SQL Server]Incorrect syntax near the keyword 'from'." - -def test_raise_exception(): - with pytest.raises(ProgrammingError) as excinfo: - raise_exception('42000', 'Syntax error or access violation') - assert str(excinfo.value) == "Driver Error: Syntax error or access violation; DDBC Error: Syntax error or access violation" - -def test_warning_exception(): - with pytest.raises(Warning) as excinfo: - raise_exception('01000', 'General warning') - assert str(excinfo.value) == "Driver Error: General warning; DDBC Error: General warning" - -def test_data_error_exception(): - with pytest.raises(DataError) as excinfo: - raise_exception('22003', 'Numeric value out of range') - assert str(excinfo.value) == "Driver Error: Numeric value out of range; DDBC Error: Numeric value out of range" - -def test_operational_error_exception(): - with pytest.raises(OperationalError) as excinfo: - raise_exception('08001', 'Client unable to establish connection') - assert str(excinfo.value) == "Driver Error: Client unable to establish connection; DDBC Error: Client unable to establish connection" - -def test_integrity_error_exception(): - with pytest.raises(IntegrityError) as excinfo: - raise_exception('23000', 'Integrity constraint violation') - assert str(excinfo.value) == "Driver Error: Integrity constraint violation; DDBC Error: Integrity constraint violation" - -def test_internal_error_exception(): - with pytest.raises(IntegrityError) as excinfo: - raise_exception('40002', 'Integrity constraint violation') - assert str(excinfo.value) == "Driver Error: Integrity constraint violation; DDBC Error: Integrity constraint violation" - -def test_programming_error_exception(): - with pytest.raises(ProgrammingError) as excinfo: - raise_exception('42S02', 'Base table or view not found') - assert str(excinfo.value) == "Driver Error: Base table or view not found; DDBC Error: Base table or view not found" - -def test_not_supported_error_exception(): - with pytest.raises(NotSupportedError) as excinfo: - raise_exception('IM001', 'Driver does not support this function') - assert str(excinfo.value) == "Driver Error: Driver does not support this function; DDBC Error: Driver does not support this function" - -def test_unknown_error_exception(): - with pytest.raises(DatabaseError) as excinfo: - raise_exception('99999', 'Unknown error') - assert str(excinfo.value) == "Driver Error: An error occurred with SQLSTATE code: 99999; DDBC Error: Unknown error" - -def test_syntax_error(cursor): - with pytest.raises(ProgrammingError) as excinfo: - cursor.execute("SELEC * FROM non_existent_table") - assert "Syntax error or access violation" in str(excinfo.value) - -def test_table_not_found_error(cursor): - with pytest.raises(ProgrammingError) as excinfo: - cursor.execute("SELECT * FROM non_existent_table") - assert "Base table or view not found" in str(excinfo.value) - -def test_data_truncation_error(cursor, db_connection): - try: - cursor.execute("CREATE TABLE #pytest_test_truncation (id INT, name NVARCHAR(5))") - cursor.execute("INSERT INTO #pytest_test_truncation (id, name) VALUES (?, ?)", [1, 'TooLongName']) - except (ProgrammingError, DataError) as excinfo: - # DataError is raised on Windows but ProgrammingError on MacOS - # Included catching both ProgrammingError and DataError in this test - # TODO: Make this test platform independent - assert "String or binary data would be truncated" in str(excinfo) - finally: - drop_table_if_exists(cursor, "#pytest_test_truncation") - db_connection.commit() - -def test_unique_constraint_error(cursor, db_connection): - try: - drop_table_if_exists(cursor, "#pytest_test_unique") - cursor.execute("CREATE TABLE #pytest_test_unique (id INT PRIMARY KEY, name NVARCHAR(50))") - cursor.execute("INSERT INTO #pytest_test_unique (id, name) VALUES (?, ?)", [1, 'Name1']) - with pytest.raises(IntegrityError) as excinfo: - cursor.execute("INSERT INTO #pytest_test_unique (id, name) VALUES (?, ?)", [1, 'Name2']) - assert "Integrity constraint violation" in str(excinfo.value) - except Exception as e: - pytest.fail(f"Test failed: {e}") - finally: - drop_table_if_exists(cursor, "#pytest_test_unique") - db_connection.commit() - -def test_foreign_key_constraint_error(cursor, db_connection): - try: - # Using regular tables (not temp tables) because SQL Server doesn't support foreign keys on temp tables. - # Using dbo schema to avoid issues with Azure SQL with Azure AD/Entra ID authentication. It can misinterpret email-format usernames (e.g., user@domain.com) as schema names. - drop_table_if_exists(cursor, "dbo.pytest_child_table") - drop_table_if_exists(cursor, "dbo.pytest_parent_table") - cursor.execute("CREATE TABLE dbo.pytest_parent_table (id INT PRIMARY KEY)") - cursor.execute("CREATE TABLE dbo.pytest_child_table (id INT, parent_id INT, FOREIGN KEY (parent_id) REFERENCES dbo.pytest_parent_table(id))") - cursor.execute("INSERT INTO dbo.pytest_parent_table (id) VALUES (?)", [1]) - with pytest.raises(IntegrityError) as excinfo: - cursor.execute("INSERT INTO dbo.pytest_child_table (id, parent_id) VALUES (?, ?)", [1, 2]) - assert "Integrity constraint violation" in str(excinfo.value) - except Exception as e: - pytest.fail(f"Test failed: {e}") - finally: - drop_table_if_exists(cursor, "dbo.pytest_child_table") - drop_table_if_exists(cursor, "dbo.pytest_parent_table") - db_connection.commit() - -def test_connection_error(): - # RuntimeError is raised on Windows, while on MacOS it raises OperationalError - # In MacOS the error goes by "Client unable to establish connection" - # In Windows it goes by "Neither DSN nor SERVER keyword supplied" - # TODO: Make this test platform independent - with pytest.raises((RuntimeError, OperationalError)) as excinfo: - connect("InvalidConnectionString") - assert "Client unable to establish connection" in str(excinfo.value) or "Neither DSN nor SERVER keyword supplied" in str(excinfo.value) \ No newline at end of file diff --git a/tests/test_006_logging.py b/tests/test_006_logging.py index a32185d6a..fc9907acf 100644 --- a/tests/test_006_logging.py +++ b/tests/test_006_logging.py @@ -1,25 +1,53 @@ import logging import os import pytest -from mssql_python.logging_config import setup_logging, get_logger, logger +import glob +from mssql_python.logging_config import setup_logging, get_logger, LoggingManager def get_log_file_path(): + # Get the LoggingManager singleton instance + manager = LoggingManager() + # If logging is enabled, return the actual log file path + if manager.enabled and manager.log_file: + return manager.log_file + # For fallback/cleanup, try to find existing log files in the logs directory repo_root_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + log_dir = os.path.join(repo_root_dir, "mssql_python", "logs") + os.makedirs(log_dir, exist_ok=True) + + # Try to find existing log files + log_files = glob.glob(os.path.join(log_dir, "mssql_python_trace_*.log")) + if log_files: + # Return the most recently created log file + return max(log_files, key=os.path.getctime) + + # Fallback to default pattern pid = os.getpid() - log_file = os.path.join(repo_root_dir, "mssql_python", f"mssql_python_trace_{pid}.log") - return log_file + return os.path.join(log_dir, f"mssql_python_trace_{pid}.log") @pytest.fixture def cleanup_logger(): """Cleanup logger & log files before and after each test""" def cleanup(): + # Get the LoggingManager singleton instance + manager = LoggingManager() logger = get_logger() if logger is not None: logger.handlers.clear() - log_file_path = get_log_file_path() - if os.path.exists(log_file_path): - os.remove(log_file_path) - logger = False + + # Try to remove the actual log file if it exists + try: + log_file_path = get_log_file_path() + if os.path.exists(log_file_path): + os.remove(log_file_path) + except: + pass # Ignore errors during cleanup + + # Reset the LoggingManager instance + manager._enabled = False + manager._initialized = False + manager._logger = None + manager._log_file = None # Perform cleanup before the test cleanup() yield @@ -29,8 +57,11 @@ def cleanup(): def test_no_logging(cleanup_logger): """Test that logging is off by default""" try: + # Get the LoggingManager singleton instance + manager = LoggingManager() logger = get_logger() assert logger is None + assert manager.enabled == False except Exception as e: pytest.fail(f"Logging not off by default. Error: {e}") @@ -40,7 +71,8 @@ def test_setup_logging(cleanup_logger): setup_logging() # This must enable logging logger = get_logger() assert logger is not None - assert logger == logging.getLogger('mssql_python.logging_config') + # Fix: Check for the correct logger name + assert logger == logging.getLogger('mssql_python') assert logger.level == logging.DEBUG # DEBUG level except Exception as e: pytest.fail(f"Logging setup failed: {e}") @@ -83,4 +115,115 @@ def test_logging_in_stdout_mode(cleanup_logger, capsys): captured_stdout = capsys.readouterr().out assert test_message in captured_stdout, "Log message not found in stdout" except Exception as e: - pytest.fail(f"Logging in stdout mode failed: {e}") \ No newline at end of file + pytest.fail(f"Logging in stdout mode failed: {e}") + +def test_python_layer_prefix(cleanup_logger): + """Test that Python layer logs have the correct prefix""" + try: + setup_logging() + logger = get_logger() + assert logger is not None + + # Log a test message + test_message = "This is a Python layer test message" + logger.info(test_message) + + # Check if the log file contains the message with [Python Layer log] prefix + log_file_path = get_log_file_path() + with open(log_file_path, 'r') as f: + log_content = f.read() + + # The logged message should have the Python Layer prefix + assert "[Python Layer log]" in log_content, "Python Layer log prefix not found" + assert test_message in log_content, "Test message not found in log file" + except Exception as e: + pytest.fail(f"Python layer prefix test failed: {e}") + +def test_different_log_levels(cleanup_logger): + """Test that different log levels work correctly""" + try: + setup_logging() + logger = get_logger() + assert logger is not None + + # Log messages at different levels + debug_msg = "This is a DEBUG message" + info_msg = "This is an INFO message" + warning_msg = "This is a WARNING message" + error_msg = "This is an ERROR message" + + logger.debug(debug_msg) + logger.info(info_msg) + logger.warning(warning_msg) + logger.error(error_msg) + + # Check if the log file contains all messages + log_file_path = get_log_file_path() + with open(log_file_path, 'r') as f: + log_content = f.read() + + assert debug_msg in log_content, "DEBUG message not found in log file" + assert info_msg in log_content, "INFO message not found in log file" + assert warning_msg in log_content, "WARNING message not found in log file" + assert error_msg in log_content, "ERROR message not found in log file" + + # Also check for level indicators in the log + assert "DEBUG" in log_content, "DEBUG level not found in log file" + assert "INFO" in log_content, "INFO level not found in log file" + assert "WARNING" in log_content, "WARNING level not found in log file" + assert "ERROR" in log_content, "ERROR level not found in log file" + except Exception as e: + pytest.fail(f"Log levels test failed: {e}") + +def test_singleton_behavior(cleanup_logger): + """Test that LoggingManager behaves as a singleton""" + try: + # Create multiple instances of LoggingManager + manager1 = LoggingManager() + manager2 = LoggingManager() + + # They should be the same instance + assert manager1 is manager2, "LoggingManager instances are not the same" + + # Enable logging through one instance + manager1._enabled = True + + # The other instance should reflect this change + assert manager2.enabled == True, "Singleton state not shared between instances" + + # Reset for cleanup + manager1._enabled = False + except Exception as e: + pytest.fail(f"Singleton behavior test failed: {e}") + +def test_timestamp_in_log_filename(cleanup_logger): + """Test that log filenames include timestamps""" + try: + setup_logging() + + # Get the log file path + log_file_path = get_log_file_path() + filename = os.path.basename(log_file_path) + + # Extract parts of the filename + parts = filename.split('_') + + # The filename should follow the pattern: mssql_python_trace_YYYYMMDD_HHMMSS_PID.log + # Fix: Account for the fact that "mssql_python" contains an underscore + assert parts[0] == "mssql", "Incorrect filename prefix part 1" + assert parts[1] == "python", "Incorrect filename prefix part 2" + assert parts[2] == "trace", "Incorrect filename part" + + # Check date format (YYYYMMDD) + date_part = parts[3] + assert len(date_part) == 8 and date_part.isdigit(), "Date format incorrect in filename" + + # Check time format (HHMMSS) + time_part = parts[4] + assert len(time_part) == 6 and time_part.isdigit(), "Time format incorrect in filename" + + # Process ID should be the last part before .log + pid_part = parts[5].split('.')[0] + assert pid_part.isdigit(), "Process ID not found in filename" + except Exception as e: + pytest.fail(f"Timestamp in filename test failed: {e}") \ No newline at end of file diff --git a/tests/test_007_logging.py b/tests/test_007_logging.py deleted file mode 100644 index fc9907acf..000000000 --- a/tests/test_007_logging.py +++ /dev/null @@ -1,229 +0,0 @@ -import logging -import os -import pytest -import glob -from mssql_python.logging_config import setup_logging, get_logger, LoggingManager - -def get_log_file_path(): - # Get the LoggingManager singleton instance - manager = LoggingManager() - # If logging is enabled, return the actual log file path - if manager.enabled and manager.log_file: - return manager.log_file - # For fallback/cleanup, try to find existing log files in the logs directory - repo_root_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) - log_dir = os.path.join(repo_root_dir, "mssql_python", "logs") - os.makedirs(log_dir, exist_ok=True) - - # Try to find existing log files - log_files = glob.glob(os.path.join(log_dir, "mssql_python_trace_*.log")) - if log_files: - # Return the most recently created log file - return max(log_files, key=os.path.getctime) - - # Fallback to default pattern - pid = os.getpid() - return os.path.join(log_dir, f"mssql_python_trace_{pid}.log") - -@pytest.fixture -def cleanup_logger(): - """Cleanup logger & log files before and after each test""" - def cleanup(): - # Get the LoggingManager singleton instance - manager = LoggingManager() - logger = get_logger() - if logger is not None: - logger.handlers.clear() - - # Try to remove the actual log file if it exists - try: - log_file_path = get_log_file_path() - if os.path.exists(log_file_path): - os.remove(log_file_path) - except: - pass # Ignore errors during cleanup - - # Reset the LoggingManager instance - manager._enabled = False - manager._initialized = False - manager._logger = None - manager._log_file = None - # Perform cleanup before the test - cleanup() - yield - # Perform cleanup after the test - cleanup() - -def test_no_logging(cleanup_logger): - """Test that logging is off by default""" - try: - # Get the LoggingManager singleton instance - manager = LoggingManager() - logger = get_logger() - assert logger is None - assert manager.enabled == False - except Exception as e: - pytest.fail(f"Logging not off by default. Error: {e}") - -def test_setup_logging(cleanup_logger): - """Test if logging is set up correctly""" - try: - setup_logging() # This must enable logging - logger = get_logger() - assert logger is not None - # Fix: Check for the correct logger name - assert logger == logging.getLogger('mssql_python') - assert logger.level == logging.DEBUG # DEBUG level - except Exception as e: - pytest.fail(f"Logging setup failed: {e}") - -def test_logging_in_file_mode(cleanup_logger): - """Test if logging works correctly in file mode""" - try: - setup_logging() - logger = get_logger() - assert logger is not None - # Log a test message - test_message = "Testing file logging mode" - logger.info(test_message) - # Check if the log file is created and contains the test message - log_file_path = get_log_file_path() - assert os.path.exists(log_file_path), "Log file not created" - # open the log file and check its content - with open(log_file_path, 'r') as f: - log_content = f.read() - assert test_message in log_content, "Log message not found in log file" - except Exception as e: - pytest.fail(f"Logging in file mode failed: {e}") - -def test_logging_in_stdout_mode(cleanup_logger, capsys): - """Test if logging works correctly in stdout mode""" - try: - setup_logging('stdout') - logger = get_logger() - assert logger is not None - # Log a test message - test_message = "Testing file + stdout logging mode" - logger.info(test_message) - # Check if the log file is created and contains the test message - log_file_path = get_log_file_path() - assert os.path.exists(log_file_path), "Log file not created in file+stdout mode" - with open(log_file_path, 'r') as f: - log_content = f.read() - assert test_message in log_content, "Log message not found in log file" - # Check if the message is printed to stdout - captured_stdout = capsys.readouterr().out - assert test_message in captured_stdout, "Log message not found in stdout" - except Exception as e: - pytest.fail(f"Logging in stdout mode failed: {e}") - -def test_python_layer_prefix(cleanup_logger): - """Test that Python layer logs have the correct prefix""" - try: - setup_logging() - logger = get_logger() - assert logger is not None - - # Log a test message - test_message = "This is a Python layer test message" - logger.info(test_message) - - # Check if the log file contains the message with [Python Layer log] prefix - log_file_path = get_log_file_path() - with open(log_file_path, 'r') as f: - log_content = f.read() - - # The logged message should have the Python Layer prefix - assert "[Python Layer log]" in log_content, "Python Layer log prefix not found" - assert test_message in log_content, "Test message not found in log file" - except Exception as e: - pytest.fail(f"Python layer prefix test failed: {e}") - -def test_different_log_levels(cleanup_logger): - """Test that different log levels work correctly""" - try: - setup_logging() - logger = get_logger() - assert logger is not None - - # Log messages at different levels - debug_msg = "This is a DEBUG message" - info_msg = "This is an INFO message" - warning_msg = "This is a WARNING message" - error_msg = "This is an ERROR message" - - logger.debug(debug_msg) - logger.info(info_msg) - logger.warning(warning_msg) - logger.error(error_msg) - - # Check if the log file contains all messages - log_file_path = get_log_file_path() - with open(log_file_path, 'r') as f: - log_content = f.read() - - assert debug_msg in log_content, "DEBUG message not found in log file" - assert info_msg in log_content, "INFO message not found in log file" - assert warning_msg in log_content, "WARNING message not found in log file" - assert error_msg in log_content, "ERROR message not found in log file" - - # Also check for level indicators in the log - assert "DEBUG" in log_content, "DEBUG level not found in log file" - assert "INFO" in log_content, "INFO level not found in log file" - assert "WARNING" in log_content, "WARNING level not found in log file" - assert "ERROR" in log_content, "ERROR level not found in log file" - except Exception as e: - pytest.fail(f"Log levels test failed: {e}") - -def test_singleton_behavior(cleanup_logger): - """Test that LoggingManager behaves as a singleton""" - try: - # Create multiple instances of LoggingManager - manager1 = LoggingManager() - manager2 = LoggingManager() - - # They should be the same instance - assert manager1 is manager2, "LoggingManager instances are not the same" - - # Enable logging through one instance - manager1._enabled = True - - # The other instance should reflect this change - assert manager2.enabled == True, "Singleton state not shared between instances" - - # Reset for cleanup - manager1._enabled = False - except Exception as e: - pytest.fail(f"Singleton behavior test failed: {e}") - -def test_timestamp_in_log_filename(cleanup_logger): - """Test that log filenames include timestamps""" - try: - setup_logging() - - # Get the log file path - log_file_path = get_log_file_path() - filename = os.path.basename(log_file_path) - - # Extract parts of the filename - parts = filename.split('_') - - # The filename should follow the pattern: mssql_python_trace_YYYYMMDD_HHMMSS_PID.log - # Fix: Account for the fact that "mssql_python" contains an underscore - assert parts[0] == "mssql", "Incorrect filename prefix part 1" - assert parts[1] == "python", "Incorrect filename prefix part 2" - assert parts[2] == "trace", "Incorrect filename part" - - # Check date format (YYYYMMDD) - date_part = parts[3] - assert len(date_part) == 8 and date_part.isdigit(), "Date format incorrect in filename" - - # Check time format (HHMMSS) - time_part = parts[4] - assert len(time_part) == 6 and time_part.isdigit(), "Time format incorrect in filename" - - # Process ID should be the last part before .log - pid_part = parts[5].split('.')[0] - assert pid_part.isdigit(), "Process ID not found in filename" - except Exception as e: - pytest.fail(f"Timestamp in filename test failed: {e}") \ No newline at end of file From 3266fe65755af550984a9e5c4305214b0e7f707a Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 16 Jul 2025 13:52:16 +0530 Subject: [PATCH 09/19] refactor --- tests/{test_005_exceptions.py => test_006_exceptions.py} | 0 tests/{test_006_logging.py => test_007_logging.py} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename tests/{test_005_exceptions.py => test_006_exceptions.py} (100%) rename tests/{test_006_logging.py => test_007_logging.py} (100%) diff --git a/tests/test_005_exceptions.py b/tests/test_006_exceptions.py similarity index 100% rename from tests/test_005_exceptions.py rename to tests/test_006_exceptions.py diff --git a/tests/test_006_logging.py b/tests/test_007_logging.py similarity index 100% rename from tests/test_006_logging.py rename to tests/test_007_logging.py From d116a734141d3d2d9d5f648dce66011a7a4cfbad Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 16 Jul 2025 13:54:35 +0530 Subject: [PATCH 10/19] restored resource cleanup changes --- mssql_python/connection.py | 13 +++++++++++++ mssql_python/cursor.py | 15 ++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/mssql_python/connection.py b/mssql_python/connection.py index 72a37e328..f4e5cf8f0 100644 --- a/mssql_python/connection.py +++ b/mssql_python/connection.py @@ -271,3 +271,16 @@ def close(self) -> None: if logger: logger.info("Connection closed successfully.") + + def __del__(self): + """ + Destructor to ensure the connection is closed when the connection object is no longer needed. + This is a safety net to ensure resources are cleaned up + even if close() was not called explicitly. + """ + if not self._closed: + try: + self.close() + except Exception as e: + if logger: + logger.error(f"Error during connection cleanup in __del__: {e}") diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index d30efcdbe..245731fd1 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -727,4 +727,17 @@ def nextset(self) -> Union[bool, None]: check_error(ddbc_sql_const.SQL_HANDLE_STMT.value, self.hstmt, ret) if ret == ddbc_sql_const.SQL_NO_DATA.value: return False - return True \ No newline at end of file + return True + + def __del__(self): + """ + Destructor to ensure the cursor is closed when it is no longer needed. + This is a safety net to ensure resources are cleaned up + even if close() was not called explicitly. + """ + if not self.closed: + try: + self.close() + except Exception as e: + if logger: + logger.error(f"Error closing cursor: {e}") From 8baaff32804bf0079672e626715e9bc9179583d1 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 16 Jul 2025 13:57:46 +0530 Subject: [PATCH 11/19] fixed tests --- mssql_python/connection.py | 1 - mssql_python/pybind/ddbc_bindings.cpp | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/mssql_python/connection.py b/mssql_python/connection.py index f4e5cf8f0..c29943e24 100644 --- a/mssql_python/connection.py +++ b/mssql_python/connection.py @@ -19,7 +19,6 @@ from mssql_python import ddbc_bindings from mssql_python.pooling import PoolingManager from mssql_python.exceptions import DatabaseError, InterfaceError -from mssql_python.auth import process_connection_string logger = get_logger() diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index afe99a004..99af424e7 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -793,8 +793,9 @@ void SqlHandle::free() { } SQLFreeHandle_ptr(_type, _handle); _handle = nullptr; - // Log the handle freeing directly with string concatenation instead of using stringstream - LOG("Freed SQL Handle of type: {}", type_str); + std::stringstream ss; + ss << "Freed SQL Handle of type: " << type_str; + LOG(ss.str()); } } From fb1289f14a5e6571a3a689632e540cff0122ccfe Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 16 Jul 2025 14:16:41 +0530 Subject: [PATCH 12/19] escape connection string properly --- tests/test_005_connection_cursor_lifecycle.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/test_005_connection_cursor_lifecycle.py b/tests/test_005_connection_cursor_lifecycle.py index 12f7405ac..e1da44627 100644 --- a/tests/test_005_connection_cursor_lifecycle.py +++ b/tests/test_005_connection_cursor_lifecycle.py @@ -72,9 +72,11 @@ def test_cursor_cleanup_without_close(conn_str): def test_no_segfault_on_gc(conn_str): """Test that no segmentation fault occurs during garbage collection""" - code = """ + # Properly escape the connection string for embedding in code + escaped_conn_str = conn_str.replace('\\', '\\\\').replace('"', '\\"') + code = f""" from mssql_python import connect -conn = connect(\"""" + conn_str + """\") +conn = connect("{escaped_conn_str}") cursors = [conn.cursor() for _ in range(5)] for cur in cursors: cur.execute("SELECT 1") @@ -93,9 +95,11 @@ def test_no_segfault_on_gc(conn_str): assert result.returncode == 0, f"Expected no segfault, but got: {result.stderr}" def test_multiple_connections_interleaved_cursors(conn_str): - code = """ + # Properly escape the connection string for embedding in code + escaped_conn_str = conn_str.replace('\\', '\\\\').replace('"', '\\"') + code = f""" from mssql_python import connect -conns = [connect(\"""" + conn_str + """\") for _ in range(3)] +conns = [connect("{escaped_conn_str}") for _ in range(3)] cursors = [] for conn in conns: # Create a cursor for each connection and execute a simple query @@ -113,9 +117,11 @@ def test_multiple_connections_interleaved_cursors(conn_str): assert result.returncode == 0, f"Expected no segfault, but got: {result.stderr}" def test_cursor_outlives_connection(conn_str): - code = """ + # Properly escape the connection string for embedding in code + escaped_conn_str = conn_str.replace('\\', '\\\\').replace('"', '\\"') + code = f""" from mssql_python import connect -conn = connect(\"""" + conn_str + """\") +conn = connect("{escaped_conn_str}") cursor = conn.cursor() cursor.execute("SELECT 1") cursor.fetchall() From 388a3c38b6168d212714d2a9db47cb0192c0a375 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 16 Jul 2025 16:36:10 +0530 Subject: [PATCH 13/19] restore connection tests --- mssql_python/pybind/ddbc_bindings.cpp | 8 +- tests/test_003_connection.py | 169 ------------------ tests/test_005_connection_cursor_lifecycle.py | 12 +- 3 files changed, 11 insertions(+), 178 deletions(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 99af424e7..b1d7bdf38 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -678,12 +678,14 @@ DriverHandle LoadDriverOrThrowException() { if (fs::exists(authDllPath)) { HMODULE hAuth = LoadLibraryW(std::wstring(authDllPath.native().begin(), authDllPath.native().end()).c_str()); if (hAuth) { - LOG("Authentication DLL loaded: {}", authDllPath.string()); + LOG("mssql-auth.dll loaded: {}", authDllPath.string()); } else { LOG("Failed to load mssql-auth.dll: {}", GetLastErrorMessage()); + ThrowStdException("Failed to load mssql-auth.dll. Please ensure it is present in the expected directory."); } } else { LOG("Note: mssql-auth.dll not found. This is OK if Entra ID is not in use."); + ThrowStdException("mssql-auth.dll not found. If you are using Entra ID, please ensure it is present."); } #endif @@ -694,6 +696,10 @@ DriverHandle LoadDriverOrThrowException() { DriverHandle handle = LoadDriverLibrary(driverPath.string()); if (!handle) { LOG("Failed to load driver: {}", GetLastErrorMessage()); + // If this happens in linux, suggest installing libltdl7 + #ifdef __linux__ + ThrowStdException("Failed to load ODBC driver. If you are on Linux, please install libltdl7 package."); + #endif ThrowStdException("Failed to load ODBC driver. Please check installation."); } LOG("Driver library successfully loaded."); diff --git a/tests/test_003_connection.py b/tests/test_003_connection.py index 6da2dee5a..bef238151 100644 --- a/tests/test_003_connection.py +++ b/tests/test_003_connection.py @@ -224,172 +224,3 @@ def test_connection_pooling_basic(conn_str): conn1.close() conn2.close() - -# Add these tests at the end of the file - -def test_cursor_cleanup_on_connection_close(conn_str): - """Test that cursors are properly cleaned up when connection is closed""" - # Create a new connection for this test - conn = connect(conn_str) - - # Create multiple cursors - cursor1 = conn.cursor() - cursor2 = conn.cursor() - cursor3 = conn.cursor() - - # Execute something on each cursor to ensure they have statement handles - # Option 1: Fetch results immediately to free the connection - cursor1.execute("SELECT 1") - cursor1.fetchall() - - cursor2.execute("SELECT 2") - cursor2.fetchall() - - cursor3.execute("SELECT 3") - cursor3.fetchall() - - # Close one cursor explicitly - cursor1.close() - assert cursor1.closed is True, "Cursor1 should be closed" - - # Close the connection (should clean up remaining cursors) - conn.close() - - # Verify all cursors are closed - assert cursor1.closed is True, "Cursor1 should remain closed" - assert cursor2.closed is True, "Cursor2 should be closed by connection.close()" - assert cursor3.closed is True, "Cursor3 should be closed by connection.close()" - -def test_cursor_weakref_cleanup(conn_str): - """Test that WeakSet properly removes garbage collected cursors""" - conn = connect(conn_str) - - # Create cursors - cursor1 = conn.cursor() - cursor2 = conn.cursor() - - # Check initial cursor count - assert len(conn._cursors) == 2, "Should have 2 cursors" - - # Delete reference to cursor1 (should be garbage collected) - cursor1_id = id(cursor1) - del cursor1 - - # Force garbage collection - import gc - gc.collect() - - # Check cursor count after garbage collection - assert len(conn._cursors) == 1, "Should have 1 cursor after garbage collection" - - # Verify cursor2 is still there - assert cursor2 in conn._cursors, "Cursor2 should still be in the set" - - conn.close() - -def test_cursor_cleanup_order_no_segfault(conn_str): - """Test that proper cleanup order prevents segfaults""" - # This test ensures cursors are cleaned before connection - conn = connect(conn_str) - - # Create multiple cursors with active statements - cursors = [] - for i in range(5): - cursor = conn.cursor() - cursor.execute(f"SELECT {i}") - cursor.fetchall() - cursors.append(cursor) - - # Don't close any cursors explicitly - # Just close the connection - it should handle cleanup properly - conn.close() - - # Verify all cursors were closed - for cursor in cursors: - assert cursor.closed is True, "All cursors should be closed" - -def test_cursor_close_removes_from_connection(conn_str): - """Test that closing a cursor properly cleans up references""" - conn = connect(conn_str) - - # Create cursors - cursor1 = conn.cursor() - cursor2 = conn.cursor() - cursor3 = conn.cursor() - - assert len(conn._cursors) == 3, "Should have 3 cursors" - - # Close cursor2 - cursor2.close() - - # cursor2 should still be in the WeakSet (until garbage collected) - # but it should be marked as closed - assert cursor2.closed is True, "Cursor2 should be closed" - - # Delete the reference and force garbage collection - del cursor2 - import gc - gc.collect() - - # Now should have 2 cursors - assert len(conn._cursors) == 2, "Should have 2 cursors after closing and GC" - - conn.close() - -def test_connection_close_idempotent(conn_str): - """Test that calling close() multiple times is safe""" - conn = connect(conn_str) - cursor = conn.cursor() - cursor.execute("SELECT 1") - - # First close - conn.close() - assert conn._closed is True, "Connection should be closed" - - # Second close (should not raise exception) - conn.close() - assert conn._closed is True, "Connection should remain closed" - - # Cursor should also be closed - assert cursor.closed is True, "Cursor should be closed" - -def test_cursor_after_connection_close(conn_str): - """Test that creating cursor after connection close raises error""" - conn = connect(conn_str) - conn.close() - - # Should raise exception when trying to create cursor on closed connection - with pytest.raises(InterfaceError) as excinfo: - cursor = conn.cursor() - - assert "closed connection" in str(excinfo.value).lower(), "Should mention closed connection" - -def test_multiple_cursor_operations_cleanup(conn_str): - """Test cleanup with multiple cursor operations""" - conn = connect(conn_str) - - # Create table for testing - cursor_setup = conn.cursor() - drop_table_if_exists(cursor_setup, "#test_cleanup") - cursor_setup.execute("CREATE TABLE #test_cleanup (id INT, value VARCHAR(50))") - cursor_setup.close() - - # Create multiple cursors doing different operations - cursor_insert = conn.cursor() - cursor_insert.execute("INSERT INTO #test_cleanup VALUES (1, 'test1'), (2, 'test2')") - - cursor_select1 = conn.cursor() - cursor_select1.execute("SELECT * FROM #test_cleanup WHERE id = 1") - cursor_select1.fetchall() - - cursor_select2 = conn.cursor() - cursor_select2.execute("SELECT * FROM #test_cleanup WHERE id = 2") - cursor_select2.fetchall() - - # Close connection without closing cursors - conn.close() - - # All cursors should be closed - assert cursor_insert.closed is True - assert cursor_select1.closed is True - assert cursor_select2.closed is True \ No newline at end of file diff --git a/tests/test_005_connection_cursor_lifecycle.py b/tests/test_005_connection_cursor_lifecycle.py index e1da44627..5fa3d56cd 100644 --- a/tests/test_005_connection_cursor_lifecycle.py +++ b/tests/test_005_connection_cursor_lifecycle.py @@ -95,11 +95,9 @@ def test_no_segfault_on_gc(conn_str): assert result.returncode == 0, f"Expected no segfault, but got: {result.stderr}" def test_multiple_connections_interleaved_cursors(conn_str): - # Properly escape the connection string for embedding in code - escaped_conn_str = conn_str.replace('\\', '\\\\').replace('"', '\\"') - code = f""" + code = """ from mssql_python import connect -conns = [connect("{escaped_conn_str}") for _ in range(3)] +conns = [connect(\"""" + conn_str + """\") for _ in range(3)] cursors = [] for conn in conns: # Create a cursor for each connection and execute a simple query @@ -117,11 +115,9 @@ def test_multiple_connections_interleaved_cursors(conn_str): assert result.returncode == 0, f"Expected no segfault, but got: {result.stderr}" def test_cursor_outlives_connection(conn_str): - # Properly escape the connection string for embedding in code - escaped_conn_str = conn_str.replace('\\', '\\\\').replace('"', '\\"') - code = f""" + code = """ from mssql_python import connect -conn = connect("{escaped_conn_str}") +conn = connect(\"""" + conn_str + """\") cursor = conn.cursor() cursor.execute("SELECT 1") cursor.fetchall() From 56634ceda6945dae27707b94f041538be6442b60 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 16 Jul 2025 16:58:32 +0530 Subject: [PATCH 14/19] minor changes --- main.py | 8 +++++++- mssql_python/connection.py | 8 +++++--- mssql_python/cursor.py | 7 +++++-- mssql_python/logging_config.py | 9 ++++++++- 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/main.py b/main.py index b45b88d73..3481369a0 100644 --- a/main.py +++ b/main.py @@ -1,10 +1,16 @@ from mssql_python import connect -from mssql_python import setup_logging +from mssql_python import setup_logging, get_logger import os import decimal setup_logging('stdout') +logger = get_logger() +if logger: + logger.info("This should print") +else: + print("Logger is None - logging not enabled!") + conn_str = os.getenv("DB_CONNECTION_STRING") conn = connect(conn_str) diff --git a/mssql_python/connection.py b/mssql_python/connection.py index c29943e24..42b7f01fb 100644 --- a/mssql_python/connection.py +++ b/mssql_python/connection.py @@ -18,7 +18,7 @@ from mssql_python.helpers import add_driver_to_connection_str, sanitize_connection_string from mssql_python import ddbc_bindings from mssql_python.pooling import PoolingManager -from mssql_python.exceptions import DatabaseError, InterfaceError +from mssql_python.exceptions import InterfaceError logger = get_logger() @@ -281,5 +281,7 @@ def __del__(self): try: self.close() except Exception as e: - if logger: - logger.error(f"Error during connection cleanup in __del__: {e}") + raise InterfaceError( + driver_error=str(e), + ddbc_error="Error during python connection cleanup" + ) \ No newline at end of file diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index 245731fd1..f7dce1c1c 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -17,6 +17,7 @@ from mssql_python.helpers import check_error from mssql_python.logging_config import get_logger from mssql_python import ddbc_bindings +from mssql_python.exceptions import InterfaceError from .row import Row logger = get_logger() @@ -739,5 +740,7 @@ def __del__(self): try: self.close() except Exception as e: - if logger: - logger.error(f"Error closing cursor: {e}") + raise InterfaceError( + driver_error=str(e), + ddbc_error="Error during python connection cleanup" + ) \ No newline at end of file diff --git a/mssql_python/logging_config.py b/mssql_python/logging_config.py index 9439f32ca..99d6f48cc 100644 --- a/mssql_python/logging_config.py +++ b/mssql_python/logging_config.py @@ -132,6 +132,7 @@ def get_logger(self): logging.Logger: The logger instance, or None if logging is not enabled. """ if not self.enabled: + # If logging is not enabled, return None return None return self._logger @@ -160,4 +161,10 @@ def get_logger(): Returns: logging.Logger: The logger instance. """ - return _manager.get_logger() \ No newline at end of file + # Always return a logger instance, even if setup hasn't been called yet + # This ensures modules can get a logger reference that will work once setup is called + if _manager._logger is None: + # Return the logger that will be configured when setup() is called + # This logger won't output anything until handlers are added in setup() + return logging.getLogger("mssql_python") + return _manager._logger \ No newline at end of file From 6e00afb49b12c5b4cd20646f077433bf4d0d297e Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 16 Jul 2025 17:46:49 +0530 Subject: [PATCH 15/19] stopped logging during destruction since that cause GIL issues --- mssql_python/connection.py | 9 ++++----- mssql_python/cursor.py | 9 ++++----- mssql_python/pybind/ddbc_bindings.cpp | 25 ++++++++----------------- 3 files changed, 16 insertions(+), 27 deletions(-) diff --git a/mssql_python/connection.py b/mssql_python/connection.py index 42b7f01fb..2903e0581 100644 --- a/mssql_python/connection.py +++ b/mssql_python/connection.py @@ -277,11 +277,10 @@ def __del__(self): This is a safety net to ensure resources are cleaned up even if close() was not called explicitly. """ - if not self._closed: + if "_closed" not in self.__dict__ or not self._closed: try: self.close() except Exception as e: - raise InterfaceError( - driver_error=str(e), - ddbc_error="Error during python connection cleanup" - ) \ No newline at end of file + pass + # if logger: + # logger.error(f"Error during connection cleanup: {e}") diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index f7dce1c1c..67bc2f773 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -736,11 +736,10 @@ def __del__(self): This is a safety net to ensure resources are cleaned up even if close() was not called explicitly. """ - if not self.closed: + if "_closed" not in self.__dict__ or not self._closed: try: self.close() except Exception as e: - raise InterfaceError( - driver_error=str(e), - ddbc_error="Error during python connection cleanup" - ) \ No newline at end of file + pass + # if logger: + # logger.error(f"Error during cursor cleanup in __del__: {e}") diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index b1d7bdf38..33aae20fd 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -530,27 +530,20 @@ void HandleZeroColumnSizeAtFetch(SQLULEN& columnSize) { // TODO: Revisit GIL considerations if we're using python's logger template void LOG(const std::string& formatString, Args&&... args) { - // Get the logger each time to ensure we have the most up-to-date logger state - py::object logging = py::module_::import("mssql_python.logging_config").attr("get_logger")(); - if (py::isinstance(logging)) { - return; - } - + py::gil_scoped_acquire gil; // <---- this ensures safe Python API usage + + py::object logger = py::module_::import("mssql_python.logging_config").attr("get_logger")(); + if (py::isinstance(logger)) return; + try { - // Add prefix to all logs std::string ddbcFormatString = "[DDBC Bindings log] " + formatString; - - // Handle both formatted and non-formatted cases if constexpr (sizeof...(args) == 0) { - // No formatting needed, just use the string directly - logging.attr("debug")(py::str(ddbcFormatString)); + logger.attr("debug")(py::str(ddbcFormatString)); } else { - // Apply formatting py::str message = py::str(ddbcFormatString).format(std::forward(args)...); - logging.attr("debug")(message); + logger.attr("debug")(message); } } catch (const std::exception& e) { - // Fallback in case of Python error - don't let logging errors crash the application std::cerr << "Logging error: " << e.what() << std::endl; } } @@ -799,9 +792,7 @@ void SqlHandle::free() { } SQLFreeHandle_ptr(_type, _handle); _handle = nullptr; - std::stringstream ss; - ss << "Freed SQL Handle of type: " << type_str; - LOG(ss.str()); + // Don't log during destruction - it can cause segfaults during Python shutdown } } From 9642fd53ec1177bfa66a0cb052ce88fbc42ee8fd Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Wed, 16 Jul 2025 17:57:49 +0530 Subject: [PATCH 16/19] logger default status fixed --- mssql_python/logging_config.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/mssql_python/logging_config.py b/mssql_python/logging_config.py index 99d6f48cc..2e9eaaeaf 100644 --- a/mssql_python/logging_config.py +++ b/mssql_python/logging_config.py @@ -161,10 +161,4 @@ def get_logger(): Returns: logging.Logger: The logger instance. """ - # Always return a logger instance, even if setup hasn't been called yet - # This ensures modules can get a logger reference that will work once setup is called - if _manager._logger is None: - # Return the logger that will be configured when setup() is called - # This logger won't output anything until handlers are added in setup() - return logging.getLogger("mssql_python") - return _manager._logger \ No newline at end of file + return _manager.get_logger() \ No newline at end of file From 6acd6650fe046aaaf6d94adea18df65c95a30d5a Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Thu, 17 Jul 2025 08:18:27 +0530 Subject: [PATCH 17/19] refactored --- mssql_python/connection.py | 29 +++++++++----------------- mssql_python/cursor.py | 30 +++++++++++---------------- mssql_python/helpers.py | 16 +++++++++++++- mssql_python/pybind/ddbc_bindings.cpp | 7 +++++++ mssql_python/pybind/unix_utils.cpp | 27 +++++++++++++----------- 5 files changed, 59 insertions(+), 50 deletions(-) diff --git a/mssql_python/connection.py b/mssql_python/connection.py index 2903e0581..03c9a94f7 100644 --- a/mssql_python/connection.py +++ b/mssql_python/connection.py @@ -15,13 +15,11 @@ from mssql_python.cursor import Cursor from mssql_python.logging_config import get_logger from mssql_python.constants import ConstantsDDBC as ddbc_sql_const -from mssql_python.helpers import add_driver_to_connection_str, sanitize_connection_string +from mssql_python.helpers import add_driver_to_connection_str, sanitize_connection_string, log from mssql_python import ddbc_bindings from mssql_python.pooling import PoolingManager from mssql_python.exceptions import InterfaceError -logger = get_logger() - class Connection: """ @@ -115,8 +113,7 @@ def _construct_connection_string(self, connection_str: str = "", **kwargs) -> st continue conn_str += f"{key}={value};" - if logger: - logger.info("Final connection string: %s", sanitize_connection_string(conn_str)) + log('info', "Final connection string: %s", sanitize_connection_string(conn_str)) return conn_str @@ -139,8 +136,7 @@ def autocommit(self, value: bool) -> None: None """ self.setautocommit(value) - if logger: - logger.info("Autocommit mode set to %s.", value) + log('info', "Autocommit mode set to %s.", value) def setautocommit(self, value: bool = True) -> None: """ @@ -195,8 +191,7 @@ def commit(self) -> None: """ # Commit the current transaction self._conn.commit() - if logger: - logger.info("Transaction committed successfully.") + log('info', "Transaction committed successfully.") def rollback(self) -> None: """ @@ -211,8 +206,7 @@ def rollback(self) -> None: """ # Roll back the current transaction self._conn.rollback() - if logger: - logger.info("Transaction rolled back successfully.") + log('info', "Transaction rolled back successfully.") def close(self) -> None: """ @@ -244,12 +238,11 @@ def close(self) -> None: except Exception as e: # Collect errors but continue closing other cursors close_errors.append(f"Error closing cursor: {e}") - if logger: - logger.warning(f"Error closing cursor: {e}") + log('warning', f"Error closing cursor: {e}") # If there were errors closing cursors, log them but continue - if close_errors and logger: - logger.warning(f"Encountered {len(close_errors)} errors while closing cursors") + if close_errors: + log('warning', f"Encountered {len(close_errors)} errors while closing cursors") # Clear the cursor set explicitly to release any internal references self._cursors.clear() @@ -260,16 +253,14 @@ def close(self) -> None: self._conn.close() self._conn = None except Exception as e: - if logger: - logger.error(f"Error closing database connection: {e}") + log('error', f"Error closing database connection: {e}") # Re-raise the connection close error as it's more critical raise finally: # Always mark as closed, even if there were errors self._closed = True - if logger: - logger.info("Connection closed successfully.") + log('info', "Connection closed successfully.") def __del__(self): """ diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index 67bc2f773..bcfe56dba 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -14,14 +14,12 @@ import datetime from typing import List, Union from mssql_python.constants import ConstantsDDBC as ddbc_sql_const -from mssql_python.helpers import check_error +from mssql_python.helpers import check_error, log from mssql_python.logging_config import get_logger from mssql_python import ddbc_bindings from mssql_python.exceptions import InterfaceError from .row import Row -logger = get_logger() - class Cursor: """ @@ -432,8 +430,7 @@ def _reset_cursor(self) -> None: if self.hstmt: self.hstmt.free() self.hstmt = None - if logger: - logger.debug("SQLFreeHandle succeeded") + log('debug', "SQLFreeHandle succeeded") # Reinitialize the statement handle self._initialize_cursor() @@ -450,8 +447,7 @@ def close(self) -> None: if self.hstmt: self.hstmt.free() self.hstmt = None - if logger: - logger.debug("SQLFreeHandle succeeded") + log('debug', "SQLFreeHandle succeeded") self.closed = True def _check_closed(self): @@ -585,15 +581,14 @@ def execute( # Executing a new statement. Reset is_stmt_prepared to false self.is_stmt_prepared = [False] - if logger: - logger.debug("Executing query: %s", operation) - for i, param in enumerate(parameters): - logger.debug( - """Parameter number: %s, Parameter: %s, - Param Python Type: %s, ParamInfo: %s, %s, %s, %s, %s""", - i + 1, - param, - str(type(param)), + log('debug', "Executing query: %s", operation) + for i, param in enumerate(parameters): + log('debug', + """Parameter number: %s, Parameter: %s, + Param Python Type: %s, ParamInfo: %s, %s, %s, %s, %s""", + i + 1, + param, + str(type(param)), parameters_type[i].paramSQLType, parameters_type[i].paramCType, parameters_type[i].columnSize, @@ -638,8 +633,7 @@ def executemany(self, operation: str, seq_of_parameters: list) -> None: total_rowcount = 0 for parameters in seq_of_parameters: parameters = list(parameters) - if logger: - logger.info("Executing query with parameters: %s", parameters) + log('info', "Executing query with parameters: %s", parameters) prepare_stmt = first_execution first_execution = False self.execute( diff --git a/mssql_python/helpers.py b/mssql_python/helpers.py index b82e99378..267ede75c 100644 --- a/mssql_python/helpers.py +++ b/mssql_python/helpers.py @@ -197,4 +197,18 @@ def sanitize_connection_string(conn_str: str) -> str: # Remove sensitive information from the connection string, Pwd section # Replace Pwd=...; or Pwd=... (end of string) with Pwd=***; import re - return re.sub(r"(Pwd\s*=\s*)[^;]*", r"\1***", conn_str, flags=re.IGNORECASE) \ No newline at end of file + return re.sub(r"(Pwd\s*=\s*)[^;]*", r"\1***", conn_str, flags=re.IGNORECASE) + + +def log(level: str, message: str, *args) -> None: + """ + Universal logging helper that gets a fresh logger instance. + + Args: + level: Log level ('debug', 'info', 'warning', 'error') + message: Log message with optional format placeholders + *args: Arguments for message formatting + """ + logger = get_logger() + if logger: + getattr(logger, level)(message, *args) \ No newline at end of file diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 33aae20fd..6713198ae 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -780,6 +780,13 @@ SQLSMALLINT SqlHandle::type() const { return _type; } +/* + * IMPORTANT: Never log in destructors - it causes segfaults. + * During program exit, C++ destructors may run AFTER Python shuts down. + * LOG() tries to acquire Python GIL and call Python functions, which crashes + * if Python is already gone. Keep destructors simple - just free resources. + * If you need destruction logs, use explicit close() methods instead. + */ void SqlHandle::free() { if (_handle && SQLFreeHandle_ptr) { const char* type_str = nullptr; diff --git a/mssql_python/pybind/unix_utils.cpp b/mssql_python/pybind/unix_utils.cpp index 681cdccc8..c98a9e090 100644 --- a/mssql_python/pybind/unix_utils.cpp +++ b/mssql_python/pybind/unix_utils.cpp @@ -14,19 +14,22 @@ const size_t kUcsLength = 2; // SQLWCHAR is 2 bytes on all platform // TODO: Make Logger a separate module and import it across the project template void LOG(const std::string& formatString, Args&&... args) { - // Get the logger each time instead of caching it to ensure we get the latest state - py::object logging_module = py::module_::import("mssql_python.logging_config"); - py::object logger = logging_module.attr("get_logger")(); - - // If logger is None, don't try to log - if (py::isinstance(logger)) { - return; + py::gil_scoped_acquire gil; // <---- this ensures safe Python API usage + + py::object logger = py::module_::import("mssql_python.logging_config").attr("get_logger")(); + if (py::isinstance(logger)) return; + + try { + std::string ddbcFormatString = "[DDBC Bindings log] " + formatString; + if constexpr (sizeof...(args) == 0) { + logger.attr("debug")(py::str(ddbcFormatString)); + } else { + py::str message = py::str(ddbcFormatString).format(std::forward(args)...); + logger.attr("debug")(message); + } + } catch (const std::exception& e) { + std::cerr << "Logging error: " << e.what() << std::endl; } - - // Format the message and log it - std::string ddbcFormatString = "[DDBC Bindings log] " + formatString; - py::str message = py::str(ddbcFormatString).format(std::forward(args)...); - logger.attr("debug")(message); } // Function to convert SQLWCHAR strings to std::wstring on macOS From d85feccb8c5a51b7ce9f417f03acee36eb3c43aa Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Thu, 17 Jul 2025 13:03:20 +0530 Subject: [PATCH 18/19] main cleanup --- main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main.py b/main.py index 7cdbe9474..b45b88d73 100644 --- a/main.py +++ b/main.py @@ -1,5 +1,5 @@ from mssql_python import connect -from mssql_python import setup_logging, get_logger +from mssql_python import setup_logging import os import decimal From 5706644333a0844563eaaa559fb5731b2064621c Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Thu, 17 Jul 2025 13:06:49 +0530 Subject: [PATCH 19/19] fixes --- mssql_python/auth.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/mssql_python/auth.py b/mssql_python/auth.py index 30efd6d99..b117c1714 100644 --- a/mssql_python/auth.py +++ b/mssql_python/auth.py @@ -7,11 +7,8 @@ import platform import struct from typing import Tuple, Dict, Optional, Union -from mssql_python.logging_config import get_logger, ENABLE_LOGGING from mssql_python.constants import AuthType -logger = get_logger() - class AADAuth: """Handles Azure Active Directory authentication"""