Skip to content

REFACTOR: remove bundled ODBC driver from mssql-python wheel; require mssql-python-odbc (Phase 2) - #693

Open
jahnvi480 wants to merge 11 commits into
mainfrom
jahnvi/phase2-remove-bundled-libs
Open

REFACTOR: remove bundled ODBC driver from mssql-python wheel; require mssql-python-odbc (Phase 2)#693
jahnvi480 wants to merge 11 commits into
mainfrom
jahnvi/phase2-remove-bundled-libs

Conversation

@jahnvi480

@jahnvi480 jahnvi480 commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Work Item / Issue Reference

AB#46836


Summary

Phase 2 of the ODBC package split: the mssql-python wheel no longer bundles the Microsoft ODBC Driver 18 binaries. The driver now ships exclusively in the standalone mssql-python-odbc package (pinned via install_requires), and the native loader resolves it from there — raising a clear, actionable error if the package is missing or incomplete, with no bundled fallback.

Changes

  • Move binaries mssql_python/libs/mssql_python_odbc/libs/ (committed source of truth); un-ignore the new location in .gitignore.
  • setup.py: stop packaging libs/ (the wheel no longer ships the driver); keep msvcp140.dll beside the compiled extension; add mssql-python-odbc==18.6.2 to install_requires.
  • ddbc_bindings.cpp (GetOdbcLibsBaseDir): require mssql_python_odbc and raise a clear error if it is absent or its driver binaries are incomplete — no silent fallback to bundled libs.
  • setup_odbc.py: single-host cross-build of all 7 py3-none-<platform> ODBC wheels via ODBC_TARGET_* overrides + target-specific package_data.
  • Pipeline: build the ODBC package first (build-odbc-all-stage.yml) → consolidate → build mssql-python, installing the external ODBC wheel before pytest; always build both packages; drop the redundant post-consolidation TestBothWheels path and 6 now-unused stage templates.
  • tests/test_015: resolve the driver libs dir from mssql_python_odbc.

Validation

  • black --check --line-length=100 mssql_python/ tests/ — clean.
  • pytest tests/ — 1914 passed, 91 skipped (3 pre-existing environment-only failures unrelated to this change: Windows MAX_PATH, a no-password local connection string, and a bulkcopy geometry server quirk).
  • ADO OneBranch pipeline green: builds both packages and installs the external ODBC wheel before pytest on Windows/macOS/Linux.

…ython-odbc

Phase 2 of the ODBC package split. Move the driver binaries from mssql_python/libs to mssql_python_odbc/libs so only the mssql-python-odbc distribution ships them. Remove the C++ bundled-libs fallback in GetOdbcLibsBaseDir(): a missing or incomplete mssql_python_odbc now raises a clear error instead of silently using bundled libs. Drop libs packaging from setup.py, retire sync_libs() in setup_odbc.py, and repoint build.bat vcredist source. AB#46586
Piece A: setup_odbc.py builds all 7 py3-none-<platform> ODBC data wheels from one host via ODBC_TARGET_* overrides; per-target package_data (include_package_data=False) so each wheel ships only its own driver + LICENSING; plat_name_supplied=True fixes macOS wheel tagging. Piece B: one unconditional ODBC_BuildAll + ConsolidateOdbc stage runs first; Windows/macOS/Linux mssql-python build stages dependsOn ConsolidateOdbc and install the external mssql-python-odbc wheel before pytest (Linux via PIP_FIND_LINKS into the bind-mounted repo dir).
The mssql-python build stages now install the external mssql-python-odbc wheel and run the full pytest suite, so the separate post-consolidation wheel-installation test stages are redundant.
…cOS-14 agent

The pure-data ODBC wheel was tagged macosx_15_0_universal2, which pip rejects on the macOS-14 build/test agent (from versions: none). Lower it to the canonical universal2 floor (macosx_11_0); the driver dylibs run on macOS 11+ and the tag stays compatible everywhere the mssql-python wheel installs.
Phase 2 moved the bundled ODBC driver libs from mssql_python into the external mssql_python_odbc package. test_libs_directory_exists and test_auth_dll_exists_if_libs_present now resolve the libs/ base dir via a helper that prefers mssql_python_odbc (falling back to mssql_python for the pre-split layout).
Copilot AI review requested due to automatic review settings July 31, 2026 08:35
@github-actions github-actions Bot added the pr-size: large Substantial code update label Jul 31, 2026
@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown

📊 Code Coverage Report

🔥 Diff Coverage

35%


🎯 Overall Coverage

81%


📈 Total Lines Covered: 7055 out of 8672
📁 Project: mssql-python


Diff Coverage

Diff: main...HEAD, staged and unstaged changes

  • mssql_python/pybind/ddbc_bindings.cpp (35.7%): Missing lines 1113-1117,1163,1177-1179

Summary

  • Total: 14 lines
  • Missing: 9 lines
  • Coverage: 35%

mssql_python/pybind/ddbc_bindings.cpp

Lines 1109-1121

  1109 // its directory as the base that `GetDriverPathCpp` (and the Windows
  1110 // `mssql-auth.dll` lookup) append `libs` to.
  1111 //
  1112 // Post-split the standalone package is REQUIRED: if it is missing or does not
! 1113 // ship this platform's driver binaries we raise a clear, actionable error
! 1114 // instead of silently falling back to bundled libs (there are none). Importing
! 1115 // `mssql_python_odbc` here is Alpine/musl-safe precisely because it is a
! 1116 // separate pure package: it cannot trigger the partially-initialized-module
! 1117 // circular import that motivated resolving these paths in C++ in the first place.
  1118 //
  1119 // (`GetDriverPathCpp` is defined further below; forward-declared here so we can
  1120 // verify the external package actually ships this platform's driver binary.)
  1121 std::string GetDriverPathCpp(const std::string& moduleDir);

Lines 1159-1167

  1159         if (!externalComplete) {
  1160             LOG("GetOdbcLibsBaseDir: mssql_python_odbc present at '%s' but its ODBC driver "
  1161                 "binaries are missing or incomplete for this platform",
  1162                 parentDir.string().c_str());
! 1163             ThrowStdException(
  1164                 "The 'mssql-python-odbc' package is installed but its ODBC driver binaries "
  1165                 "are missing or incomplete for this platform. Reinstall it with: "
  1166                 "pip install --force-reinstall mssql-python-odbc");
  1167         }

Lines 1173-1183

  1173             // Phase 2: the standalone package is required. Turn the missing
  1174             // dependency into a clear, actionable error instead of a fallback.
  1175             LOG("GetOdbcLibsBaseDir: required package mssql_python_odbc is not installed (%s)",
  1176                 e.what());
! 1177             ThrowStdException(
! 1178                 "The required 'mssql-python-odbc' package (which ships the ODBC driver "
! 1179                 "binaries) is not installed. Install it with: pip install mssql-python-odbc");
  1180         }
  1181         // A different import-time error means the package is installed but
  1182         // broken; surface it instead of silently masking the real problem.
  1183         LOG("GetOdbcLibsBaseDir: importing mssql_python_odbc failed unexpectedly (%s); "


📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)
mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 59.9%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 76.1%
mssql_python.__init__.py: 77.3%
mssql_python.row.py: 77.6%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 81.4%
mssql_python.pybind.connection.connection.cpp: 83.7%
mssql_python.connection.py: 84.7%

🔗 Quick Links

⚙️ Build Summary 📋 Coverage Details

View Azure DevOps Build

Browse Full Coverage Report

Copilot stopped reviewing on behalf of jahnvi480 due to an error July 31, 2026 08:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR finalizes the Phase 2 split of the bundled ODBC driver into the standalone mssql-python-odbc package, makes the external package required at runtime, and updates build/test pipelines to produce and consume the new wheel layout.

Changes:

  • Make mssql_python_odbc required for driver resolution (no fallback to bundled mssql_python/libs), with clearer runtime errors.
  • Update ODBC wheel packaging to include only the target platform’s libs/ subtree and enable single-host cross-building of all ODBC wheels.
  • Refactor CI pipelines to build all ODBC wheels in one Windows stage, install the ODBC wheel before pytest in platform builds, and remove now-redundant “both wheels” installation test stages.

Reviewed changes

Copilot reviewed 17 out of 85 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/test_015_utf8_path_handling.py Updates tests to locate driver libs in mssql_python_odbc (but still includes a fallback).
setup_odbc.py Packages only the target platform’s ODBC libs/ subtree; adds env overrides for cross-building and forces wheel to trust supplied platform tags.
setup.py Removes inclusion/exclusion rules for bundled libs/ since the driver is now shipped separately.
mssql_python/pybind/ddbc_bindings.cpp Requires mssql_python_odbc and throws actionable errors if missing/incomplete; removes fallback behavior.
mssql_python/pybind/build.bat Copies VC++ runtime from mssql_python_odbc/libs/.../vcredist instead of mssql_python/libs.
eng/pipelines/pr-validation-pipeline.yml Updates validation paths to the new mssql_python_odbc/libs/... locations.
OneBranchPipelines/stages/wheel-installation-test-windows-stage.yml Deleted: prior end-user “both wheels” installation test stage (Windows).
OneBranchPipelines/stages/wheel-installation-test-stage.yml Deleted: prior end-user “both wheels” installation test stage (Linux).
OneBranchPipelines/stages/wheel-installation-test-macos-stage.yml Deleted: prior end-user “both wheels” installation test stage (macOS).
OneBranchPipelines/stages/build-windows-single-stage.yml Adds optional dependency/install of consolidated mssql-python-odbc wheel before pytest.
OneBranchPipelines/stages/build-odbc-windows-stage.yml Deleted: replaced by a single-host “build all ODBC wheels” stage.
OneBranchPipelines/stages/build-odbc-macos-stage.yml Deleted: replaced by a single-host “build all ODBC wheels” stage.
OneBranchPipelines/stages/build-odbc-linux-stage.yml Deleted: replaced by a single-host “build all ODBC wheels” stage.
OneBranchPipelines/stages/build-odbc-all-stage.yml New: builds all 7 ODBC wheels on one Windows agent via ODBC_TARGET_* overrides and validates wheel contents.
OneBranchPipelines/stages/build-macos-single-stage.yml Adds optional dependency/install of consolidated mssql-python-odbc wheel before pytest.
OneBranchPipelines/stages/build-linux-single-stage.yml Adds optional dependency/install of consolidated mssql-python-odbc wheel before pytest (via PIP_FIND_LINKS).
OneBranchPipelines/build-release-package-pipeline.yml Always builds both packages; makes mssql-python stages depend on ConsolidateOdbc; swaps 7 ODBC stages for one build-all stage.
.gitignore Stops ignoring mssql_python_odbc/libs/ now that it’s the committed source of truth.
Suppressed comments (2)

tests/test_015_utf8_path_handling.py:1

  • This test helper still falls back to mssql_python when mssql_python_odbc is missing/incomplete, but the runtime resolver (GetOdbcLibsBaseDir) now requires the external package and throws if it’s missing or incomplete. The fallback makes the tests inconsistent with production behavior and can mask packaging issues. Recommendation (mandatory): remove the fallback and instead fail (or explicitly pytest.skip) when mssql_python_odbc is not importable or does not contain a complete libs/ payload for the platform.
    setup_odbc.py:1
  • When ODBC_TARGET_PLATFORM_TAG is set, get_platform_info() returns an empty arch if ODBC_TARGET_ARCH is unset. That empty value is later used to build the target libs/ globs (e.g., libs/windows/{arch}), which can silently produce wrong/empty package_data and yield broken wheels. Recommendation (mandatory): validate the override inputs—require a non-empty, expected arch for win* and *linux* tags (and optionally validate allowed values per tag), and raise a clear error when the override is incomplete/invalid.

Comment thread OneBranchPipelines/stages/build-odbc-all-stage.yml
…); validate installed mssql-python-odbc wheel on Win/macOS pytest; repoint configure_dylibs to mssql_python_odbc/libs
…exact driver filenames

- setup_odbc.get_platform_info now raises OSError if ODBC_TARGET_PLATFORM_TAG is set but ODBC_TARGET_ARCH is empty; an empty arch expanded the libs/ package_data globs to every architecture and could leak foreign-platform driver binaries into the wheel.

- build-odbc-all-stage leak-check Must lists now assert the exact driver filenames (msodbcsql18.dll / libmsodbcsql.18.dylib / libmsodbcsql-18) instead of loose substrings that also matched support files.

- test_015 prose updated to reference GetOdbcLibsBaseDir (the successor to the removed GetModuleDirectory).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-size: large Substantial code update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants