-
Notifications
You must be signed in to change notification settings - Fork 29
CHORE: Build Pipeline Modernization - Multi-Platform Support, Security Compliance & Test Infrastructure #328
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: David Engel <dengel1012@gmail.com>
Syncing Github main to ADO main Related work items: #38037
…rosoft/mssql-python into bewithgaurav/publish_symbols
…uring Build #### AI description (iteration 1) #### PR Classification This pull request is a code improvement and cleanup that overhauls the logging system and updates build and test configurations. #### PR Summary The changes replace the legacy logging configuration with a new, unified logging infrastructure based on a dedicated `mssql_python/logging.py` module and a C++ logger bridge (`pybind/logger_bridge.cpp`/`logger_bridge.hpp`), and the modules throughout the codebase (e.g. connection, cursor, pooling, exceptions) now use the new logging macros instead of the old `logging_config.py` approach. In addition, the build pipelines (in `build-windows-single-stage.yml`) have been updated to download Windows Python ARM64 libraries from NuGet and several new performance, stress, and integration tests have been added or updated. <!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot --> Related work items: #40402
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changesNo lines with coverage information in this diff. 📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.helpers.py: 66.6%
mssql_python.row.py: 67.4%
mssql_python.pybind.ddbc_bindings.cpp: 70.4%
mssql_python.pybind.connection.connection.cpp: 76.3%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 79.6%
mssql_python.pybind.ddbc_bindings.h: 79.7%
mssql_python.connection.py: 82.5%
mssql_python.cursor.py: 83.6%🔗 Quick Links
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request modernizes the build pipeline with comprehensive documentation improvements, multi-platform support enhancements, and security compliance updates. However, there are critical bugs that will prevent the pipeline from running successfully.
Key Issues Found:
Critical Bugs:
- Pipeline Dependency Mismatch: The Consolidate stage depends on Python 3.14 stages (
Win_py314_x64,Win_py314_arm64,MacOS_py314) that are commented out in the configuration, which will cause the pipeline to fail waiting for non-existent stages. - Incorrect Variable Usage: The Consolidate stage uses
${{ parameters.oneBranchType }}instead of${{ variables.effectiveOneBranchType }}, breaking the intended behavior where scheduled builds should always use Official mode.
Security & Best Practices:
3. Password Exposure Risk: DB_PASSWORD is passed through environment variables in shell scripts with set -x enabled, potentially exposing credentials in build logs.
4. ESRP Scanning Inconsistency: Windows builds run ESRP malware scanning unconditionally, while macOS and Linux only run it for Official builds.
Documentation Issues:
5. Extensive Comment Inconsistencies: 13 comments reference Python 3.14 support or incorrect stage counts that don't match the actual configuration.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 22 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_000_dependencies.py | Fixed module directory detection to use mssql_python.__file__ instead of string manipulation (correct fix) |
| OneBranchPipelines/build-release-package-pipeline.yml | Added comprehensive documentation, but introduced critical bugs with Python 3.14 stage dependencies and variable usage |
| OneBranchPipelines/stages/build-windows-single-stage.yml | Added Python 3.14 NuGet installation logic and ARM64 cross-compilation support; ESRP scanning runs unconditionally unlike other platforms |
| OneBranchPipelines/stages/build-macos-single-stage.yml | Improved documentation and Colima Docker setup; properly conditionalizes ESRP scanning |
| OneBranchPipelines/stages/build-linux-single-stage.yml | Added comprehensive test infrastructure with isolated pytest execution per Python version; contains duplicate commented code and password exposure risk |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The merge-base changed after approval.
Work Item / Issue Reference
Summary
This pull request makes a comprehensive overhaul to the
OneBranchPipelines/build-release-package-pipeline.ymlfile to improve clarity, documentation, and security compliance for building and releasing themssql-pythonpackage. The changes include detailed comments, improved parameterization, expanded platform support, and enhanced security scanning. The pipeline is now more maintainable and easier to understand, with explicit configuration for each platform and build stage.Pipeline Structure and Documentation Improvements
Platform and Build Configuration Enhancements
Security and Compliance Improvements
Variable and Resource Management
Build Stage Details