Skip to content
Open
Show file tree
Hide file tree
Changes from 21 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 141 additions & 0 deletions .github/workflows/code-quality-check.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
name: Code Quality

permissions:
contents: read
issues: write

on:
pull_request:
types: [opened, edited, reopened, synchronize]
paths:
- '**.py'
- '**.cpp'
- '**.h'

jobs:
lint:
name: Code Linting
runs-on: ubuntu-latest
steps:
- name: Checkout Code
uses: actions/checkout@v3

- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: '3.11'

- name: Cache pip dependencies
uses: actions/cache@v3
with:
path: ~/.cache/pip
key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements*.txt') }}
restore-keys: |
${{ runner.os }}-pip-

- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install -e .
pip install pylint cpplint

- name: Run Python Linter
id: pylint
continue-on-error: true
run: |
echo "Running pylint with threshold 8.5..."
python -m pylint --fail-under=8.5 --disable=fixme,no-member,too-many-arguments,too-many-positional-arguments,invalid-name,useless-parent-delegation --output-format=colorized mssql_python
echo "pylint_status=$?" >> $GITHUB_ENV

- name: Run C++ Linter
id: cpplint
continue-on-error: true
run: |
echo "Running cpplint with maximum 10 errors per file..."

# Find C++ files excluding build directories
FILES=$(find mssql_python -name "*.cpp" -o -name "*.h" | grep -v "/build/")
if [ -z "$FILES" ]; then
echo "No C++ files found to check!"
echo "cpplint_status=0" >> $GITHUB_ENV
exit 0
fi

echo "Found files to check:"
echo "$FILES"

# Process each file individually with better error handling
MAX_ERRORS=10
FAILED_FILES=""

for FILE in $FILES; do
echo "Checking $FILE..."

# Run cpplint on a single file and capture output
OUTPUT=$(python -m cpplint --filter=-readability/todo --linelength=100 "$FILE" 2>&1 || true)

# Display the output for this file
echo "$OUTPUT"

# Extract error count more reliably
ERROR_COUNT=$(echo "$OUTPUT" | grep -o 'Total errors found: [0-9]*' | grep -o '[0-9]*' || echo "0")

# If we couldn't extract a count, default to 0
if [ -z "$ERROR_COUNT" ]; then
ERROR_COUNT=0
fi

echo "File $FILE has $ERROR_COUNT errors"

# Check if over threshold
if [ "$ERROR_COUNT" -gt "$MAX_ERRORS" ]; then
FAILED_FILES="$FAILED_FILES\n- $FILE ($ERROR_COUNT errors)"
fi
done

# Output results
if [ ! -z "$FAILED_FILES" ]; then
echo -e "\n⛔ The following files have more than $MAX_ERRORS errors:$FAILED_FILES"
echo "cpplint_status=1" >> $GITHUB_ENV
else
echo -e "\n✅ All files have $MAX_ERRORS or fewer errors."
echo "cpplint_status=0" >> $GITHUB_ENV
fi

- name: Determine overall status
run: |
if [[ "${{ env.pylint_status }}" != "0" || "${{ env.cpplint_status }}" != "0" ]]; then
echo "Linting checks failed!"
exit 1
else
echo "All linting checks passed!"
fi

- name: Comment on PR
if: github.event_name == 'pull_request' && (env.pylint_status != '0' || env.cpplint_status != '0')
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we put this comment regardless of error? lets try adding this first, if looks spammy we can restore this condition

uses: actions/github-script@v7
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
script: |
let comment = '## Code Quality Check Results\n\n';

if ('${{ env.pylint_status }}' !== '0') {
comment += '⚠️ **Python linting failed** - Please check the [workflow logs](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) for details.\n\n';
} else {
comment += '✅ **Python linting passed**\n\n';
}

if ('${{ env.cpplint_status }}' !== '0') {
comment += '⚠️ **C++ linting failed** - Some files exceed the maximum error threshold of 10.\n\n';
} else {
comment += '✅ **C++ linting passed**\n\n';
}

comment += 'See [code quality guidelines](https://github.com/microsoft/mssql-python/blob/main/CONTRIBUTING.md) for more information.';

github.rest.issues.createComment({
issue_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
body: comment
});
16 changes: 16 additions & 0 deletions .pre-commit-config.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
repos:
- repo: https://github.com/pre-commit/mirrors-pylint
rev: v3.0.0a5
hooks:
- id: pylint
args: [--fail-under=8.5, --disable=fixme,no-member,too-many-arguments,too-many-positional-arguments,invalid-name,useless-parent-delegation]

- repo: local
hooks:
- id: cpplint
name: cpplint
entry: python -m cpplint
language: python
types: [c++]
args: [--filter=-readability/todo, --linelength=100]
exclude: ^.*build/.*$
4 changes: 4 additions & 0 deletions cpplint.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
set noparent
filter=-readability/todo
linelength=100
exclude_files=build
24 changes: 12 additions & 12 deletions mssql_python/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@

# Connection Objects
from .db_connection import connect, Connection
from .pooling import PoolingManager

# Cursor Objects
from .cursor import Cursor
Expand All @@ -58,20 +59,19 @@
paramstyle = "qmark"
threadsafety = 1

from .pooling import PoolingManager
def pooling(max_size=100, idle_timeout=600, enabled=True):
# """
# Enable connection pooling with the specified parameters.
# By default:
# - If not explicitly called, pooling will be auto-enabled with default values.
"""
Enable connection pooling with the specified parameters.
By default:
- If not explicitly called, pooling will be auto-enabled with default values.

Args:
max_size (int): Maximum number of connections in the pool.
idle_timeout (int): Time in seconds before idle connections are closed.

# Args:
# max_size (int): Maximum number of connections in the pool.
# idle_timeout (int): Time in seconds before idle connections are closed.

# Returns:
# None
# """
Returns:
None
"""
if not enabled:
PoolingManager.disable()
else:
Expand Down
58 changes: 35 additions & 23 deletions mssql_python/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@

import platform
import struct
from typing import Tuple, Dict, Optional, Union
from typing import Tuple, Dict, Optional
from mssql_python.constants import AuthType


class AADAuth:
"""Handles Azure Active Directory authentication"""

@staticmethod
def get_token_struct(token: str) -> bytes:
"""Convert token to SQL Server compatible format"""
Expand All @@ -22,21 +23,21 @@ def get_token_struct(token: str) -> bytes:
def get_token(auth_type: str) -> bytes:
"""Get token using the specified authentication type"""
from azure.identity import (
DefaultAzureCredential,
DeviceCodeCredential,
InteractiveBrowserCredential
DefaultAzureCredential,
DeviceCodeCredential,
InteractiveBrowserCredential,
)
from azure.core.exceptions import ClientAuthenticationError

# Mapping of auth types to credential classes
credential_map = {
"default": DefaultAzureCredential,
"devicecode": DeviceCodeCredential,
"interactive": InteractiveBrowserCredential,
}

credential_class = credential_map[auth_type]

try:
credential = credential_class()
token = credential.get_token("https://database.windows.net/.default").token
Expand All @@ -50,18 +51,21 @@ def get_token(auth_type: str) -> bytes:
) from e
except Exception as e:
# Catch any other unexpected exceptions
raise RuntimeError(f"Failed to create {credential_class.__name__}: {e}") from e
raise RuntimeError(
f"Failed to create {credential_class.__name__}: {e}"
) from 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
"""
Expand All @@ -88,7 +92,7 @@ def process_auth_parameters(parameters: list) -> Tuple[list, Optional[str]]:
# Interactive authentication (browser-based); only append parameter for non-Windows
if platform.system().lower() == "windows":
auth_type = None # Let Windows handle AADInteractive natively

elif value_lower == AuthType.DEVICE_CODE.value:
# Device code authentication (for devices without browser)
auth_type = "devicecode"
Expand All @@ -99,40 +103,48 @@ def process_auth_parameters(parameters: list) -> Tuple[list, Optional[str]]:

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="
"uid=",
"pwd=",
"encrypt=",
"trustservercertificate=",
"authentication=",
]
return [
param for param in parameters
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

# Handle platform-specific logic for interactive auth
if auth_type == "interactive" and platform.system().lower() == "windows":
return None # Let Windows handle AADInteractive natively

try:
return AADAuth.get_token(auth_type)
except (ValueError, RuntimeError):
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
"""
Expand All @@ -145,9 +157,9 @@ def process_connection_string(connection_string: str) -> Tuple[str, Optional[Dic
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):
if not any("=" in param for param in parameters):
raise ValueError("Invalid connection string format")

modified_parameters, auth_type = process_auth_parameters(parameters)
Expand All @@ -158,4 +170,4 @@ def process_connection_string(connection_string: str) -> Tuple[str, Optional[Dic
if token_struct:
return ";".join(modified_parameters) + ";", {1256: token_struct}

return ";".join(modified_parameters) + ";", None
return ";".join(modified_parameters) + ";", None
Loading