Skip to content

FEAT: Add Conda release and publish tooling for mssql-python - #720

Open
Jahnvi Thakkar (jahnvi480) wants to merge 152 commits into
mainfrom
jahnvi/conda-onboarding
Open

FEAT: Add Conda release and publish tooling for mssql-python#720
Jahnvi Thakkar (jahnvi480) wants to merge 152 commits into
mainfrom
jahnvi/conda-onboarding

Conversation

@jahnvi480

@jahnvi480 Jahnvi Thakkar (jahnvi480) commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Work Item / Issue Reference

AB#47315

Summary

This pull request introduces significant improvements to the conda packaging and release process for the mssql-python project. It adds a new GitHub Actions workflow to automatically audit conda packages on pull requests, ensuring early detection of packaging and dependency issues. Additionally, it introduces a standalone Azure DevOps pipeline for validating and publishing conda packages, decoupled from the PyPI release process. These changes enhance the reliability, security, and maintainability of the conda release workflow.

Conda packaging and release automation:

  • Added .github/workflows/conda-audit.yml to automatically build and audit a linux-64 conda package using shipped PyPI wheels on every relevant pull request, running comprehensive tests and a masking-immune binary audit to catch regressions early.
  • Introduced OneBranchPipelines/conda-release-pipeline.yml, a standalone Azure DevOps pipeline to validate and publish conda packages. This pipeline enforces strict readiness gates, supports dry-run validation, and cleanly separates the conda release from the PyPI/wheel release process.

Release safety and validation improvements:

  • The new conda release pipeline includes a dry-run mode (default) that prints the exact anaconda upload plan without publishing, allowing maintainers to review intended actions before production release.
  • When publishing, the pipeline enforces version pinning and provenance checks to ensure only the correct, validated packages are released to Anaconda.org, with clear gates and error messages for misconfigurations.

Final reviewed source: 967b03364c022109d5f617a883e5a94ae649ffdb (12 added files, +3,961/-0 against main). The final follow-up changes three files (+100/-58): staging labels must match <target>_staging_<numeric-build-id>, that namespace cannot be a public target, and the actual generated scope is validated before upload and every promoter/cleanup API entrypoint. The publisher lock still contains all 66 pins/hashes, including anaconda-client; the separate audit lock contains only zstandard. 340 focused source tests and offline PowerShell route checks passed; no live publication was performed.

The release version defaults to 1.15.0, with an explicit override. publishToConda=false remains the default; true enables Official publishing after validation. No manual ADO approval or locking system is added; secret-read permissions are unchanged and the team must serialize publication/recovery. The current-source ADO dry run has not been executed. The hash-pinned public 1.14.0 wheel pair remains an older packaging control, not 1.15.0 qualification; its update awaits verified public 1.15.0 wheel metadata and hashes.

Adds conda recipes for mssql-python and its mssql-python-odbc companion, a local build/test-before-live harness, and a draft OneBranch conda publish stage. Azure SDK deps resolve from the lean 'microsoft' Anaconda channel under --strict-channel-priority so conda-forge's azure-core recipe (which over-declares flask/six -> celery/boto3/botocore) does not bloat the environment. See conda-forge/azure-core-feedstock#71.
Copilot AI lite review requested due to automatic review settings August 14, 2026 08:50
@github-actions github-actions Bot added the pr-size: medium Moderate update size label Aug 14, 2026
Comment thread conda/mssql-python-odbc/meta.yaml Fixed

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

This PR adds initial conda packaging assets for mssql-python (and its mssql-python-odbc companion) so the project can be published to the Microsoft-owned microsoft Anaconda channel, alongside the existing PyPI wheel distribution.

Changes:

  • Adds conda recipes for mssql-python and mssql-python-odbc that repackage existing wheels (no compilation).
  • Adds a local PowerShell harness to build both recipes, create a local channel, and validate import / optional live-connect.
  • Adds a draft OneBranch stage to build/test/publish conda artifacts from signed release artifacts.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File Description
conda/onebranch-publish-conda-stage.yml Draft OneBranch stage to build from signed artifacts, gate on smoke tests, and publish to the microsoft Anaconda channel.
conda/mssql-python/meta.yaml Conda recipe for repackaging the mssql-python wheel and depending on a version-locked mssql-python-odbc.
conda/mssql-python-odbc/meta.yaml Conda recipe for repackaging the proprietary driver wheel into a companion conda package.
conda/build_and_test_local.ps1 Local “test-before-live” harness to build, index, install, and smoke test the conda packages.
Suppressed comments (1)

conda/onebranch-publish-conda-stage.yml:133

  • Same string-vs-boolean condition issue here: quoting the template expression turns it into a string, which can cause the publish step to be skipped unexpectedly even when publishToConda is true.
          - task: PowerShell@2
            displayName: 'Publish to anaconda.org/microsoft'
            condition: and(succeeded(), eq('${{ parameters.publishToConda }}', true))

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread conda/mssql-python/meta.yaml Outdated
Comment thread conda/mssql-python-odbc/meta.yaml Outdated
Comment thread conda/build_and_test_local.ps1 Outdated
Comment thread conda/onebranch-publish-conda-stage.yml Outdated
Comment thread conda/mssql-python-odbc/meta.yaml Outdated
Comment thread conda/build_and_test_local.ps1 Outdated
Recipes now honor ARTIFACTS_PY/ARTIFACTS_ODBC to install signed wheels offline (--no-index --find-links) instead of always hitting PyPI. build_and_test_local.ps1 adds --override-channels for reproducible solves and its header no longer claims an offline/no-microsoft-channel run. Draft publish stage condition compares the boolean parameter directly (eq(param, true)) instead of a quoted string.
Comment thread conda/mssql-python-odbc/meta.yaml Fixed
- Single-source versions via MSSQL_PYTHON_VERSION/MSSQL_ODBC_VERSION env (wired from the publish-stage params) so the package version and the companion pin can't drift.

- Point mssql-python-odbc license_file at the actual ODBC Driver 18 EULA + VC++ license (was MIT-primary root LICENSE); remove the resolved TODO.

- Drop the no-op azure-identity >=1.12.0 floor (microsoft channel ships CalVer).

- Re-assert the wheel platform floor via __glibc/__osx virtual-package run constraints.

- Add conda/driver_load_probe.py + run it in the gate and local harness so we prove the native ODBC driver loads, not just the Python shim.

- Publish stage: require signed wheels (no PyPI fallback), publish companion-first with --skip-existing and a #706 pair guard, and document the required resources.pipelines declaration.
Comment thread conda/driver_load_probe.py Fixed
@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown

📊 Code Coverage Report

🔥 Diff Coverage

100%


🎯 Overall Coverage

83%


📈 Total Lines Covered: 8220 out of 9874
📁 Project: mssql-python


Diff Coverage

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

No lines with coverage information in this diff.


📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)
mssql_python.pybind.performance_counter.hpp: 0.7%
mssql_python.pybind.logger_bridge.cpp: 57.9%
mssql_python.pybind.ddbc_bindings.h: 61.5%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.row.py: 77.6%
mssql_python.pybind.ddbc_bindings.cpp: 77.7%
mssql_python.pybind.connection.connection_pool.cpp: 81.8%
mssql_python.logging.py: 85.5%
mssql_python.helpers.py: 89.3%
mssql_python.pooling.py: 90.1%

🔗 Quick Links

⚙️ Build Summary 📋 Coverage Details

View Azure DevOps Build

Browse Full Coverage Report

@jahnvi480
Jahnvi Thakkar (jahnvi480) marked this pull request as draft August 18, 2026 08:57
Port the productionized conda pipeline from the ADO conda-publish-pipeline branch onto GitHub conda-onboarding, replacing the earlier draft prototype:

- conda recipes: meta.yaml + build.sh + bld.bat for mssql-python and mssql-python-odbc, vendored ODBC/VC++ EULA text, and .gitattributes (LF for shell scripts).

- OneBranchPipelines conda glue: scripts/build-conda-packages.{sh,ps1}, steps/conda-build-validate-step{,-posix}.yml, steps/conda-publish-step.yml, steps/conda-release-step.yml, jobs/consolidate-conda-artifacts-job.yml.

- Wire conda legs into build/release pipelines (buildConda params on win-64/osx/linux legs + ConsolidateConda stage), preserving GitHub-only signWindowsBinaries content.

- Remove superseded draft prototype: conda/onebranch-publish-conda-stage.yml and conda/build_and_test_local.ps1.
@github-actions github-actions Bot added pr-size: large Substantial code update and removed pr-size: medium Moderate update size labels Aug 18, 2026

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

Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.

Suppressed comments (4)

OneBranchPipelines/scripts/build-conda-packages.sh:173

  • The verify env name only includes the Python version. On macOS this script is invoked twice on the same agent (native osx-arm64, then cross-target osx-64), so the second conda create -n verify_... can fail with an existing prefix or reuse the wrong-arch env. Include the effective subdir (native vs $CONDA_SUBDIR) in the env name to avoid collisions.
for py in $pyvers; do
  envName="verify_${py//./}"
  echo "=== [py $py] create verify env from local channel ==="

OneBranchPipelines/stages/build-linux-single-stage.yml:116

  • This apt install runs before the later apt-get update in the job, so it can fail on agents with stale apt indices (common on hosted Ubuntu). Add an apt-get update (and ideally noninteractive) before installing qemu-user-static/libc6-arm64-cross.
              # qemu-user-static: run aarch64 ELF binaries on the x86_64 host.
              # libc6-arm64-cross: the aarch64 glibc runtime (loader + libc/libm/...)
              # under /usr/aarch64-linux-gnu so qemu can resolve /lib/ld-linux-aarch64.so.1
              # for the emulated aarch64 conda build/verify (QEMU_LD_PREFIX points here).
              sudo apt-get install -y qemu-user-static libc6-arm64-cross

OneBranchPipelines/steps/conda-publish-step.yml:91

  • Installing anaconda-client from PyPI without pinning a version can make releases non-reproducible and may unexpectedly break publishing if a new release introduces behavioral changes. Consider pinning to a vetted version (or using a constraints file) so publish behavior is stable.
        $ErrorActionPreference = 'Stop'
        python -m pip install --upgrade pip
        python -m pip install anaconda-client
        # anaconda-client installs the `anaconda` console script onto PATH.
        anaconda --version

OneBranchPipelines/steps/conda-build-validate-step.yml:126

  • The error message references $links (the wheel find-links directory), but conda packages are searched under ${{ parameters.outputDir }}/bld. If this trips, the message will mislead troubleshooting.
      if (-not $built) { Write-Error "No conda packages were produced under $($links)"; exit 1 }

Comment thread conda/mssql-python/meta.yaml Outdated
Comment thread OneBranchPipelines/scripts/build-conda-packages.sh Outdated
Comment thread OneBranchPipelines/scripts/build-conda-packages.ps1 Outdated
…uildAll on Windows

The Windows mssql-python-odbc companion conda is now built ONCE as a Python-agnostic package in the ODBC_BuildAll stage (no python in host, wheel extracted via tar in bld.bat), mirroring the single py3-none-win_* PyPI wheel, instead of once per Python on every binding leg. The per-Python mssql-python binding legs seed that prebuilt companion into their local channel (-Package binding -DriverCondaDir) so the version-locked dependency still resolves. ConsolidateConda now also pulls the ODBC_BuildAll companion, and the #706 release/publish gates use presence-pairing (keeping strict 1:1 only for per-Python companions, c>1). macOS/Linux stay per-Python (unchanged).
P0-1: invert conda/driver_load_probe.py from a fail-OPEN denylist to a
fail-CLOSED allowlist. A repackaged native ODBC driver that fails to load now
FAILS the DB-less pre-publish gate instead of passing on any unrecognized
exception. Only a clean connect or a connection-stage diagnostic the loaded
msodbcsql driver alone can emit (ODBC branding, network provider, TLS, auth)
counts as PASS. Defer 'import mssql_python' into main() so the classifier is
unit-testable without the compiled extension. Adds tests/test_026 (22 no-DB
tests).

P0-3: gate the conda release on package METADATA, not folder names/counts.
New conda/validate_conda_release.py reads each package's authoritative
info/index.json (zstd) and validates real subdir == folder, allowed subdirs,
the full (subdir x Python) binding matrix, exact/consistent versions, and #706
binding<->companion pairing. Catches a mislabeled subdir and the 8e7f217
dropped-win-64-variant regression the count gate missed. Rewires
OneBranchPipelines/steps/conda-release-step.yml to call it. Adds tests/test_027
(11 tests incl. a real .conda round-trip).
Comment thread conda/driver_load_probe.py Fixed
…P0-2)

macos-latest is an Intel Mac and the arm64 Python cannot execute there (no
reverse Rosetta), so the previous osx-arm64 leg silently built NATIVE osx-64
packages and staged them under osx-arm64 (mislabeled). Cross-build for real:

- conda/*/build.sh: when the host-env Python is not executable (non-emulated
  cross-build), extract the universal2 wheel into \ with unzip instead of
  pip -- mirrors the Windows bld.bat tar path; the arm64 slice comes from the
  universal2 wheel. Native + QEMU-emulated legs keep the pip install.
- conda/*/meta.yaml: skip_compile_pyc on macOS so conda-build does not run the
  non-runnable arm64 Python for .pyc byte-compilation (Python regenerates it).
- build-conda-packages.sh: section-7 verify auto-skips the runtime import when
  the target Python can't run on the host (osx-arm64 on Intel); the static
  arm64-slice audit is the stand-in. Native/QEMU legs still import for real.
- build-macos-single-stage.yml: add condaTargetSubdir: osx-arm64 +
  continueOnError to the arm64 leg; mark osx-64 as the native/blocking leg; add
  a BLOCKING static arm64 slice audit (lipo/otool/file) asserting the shipped
  ddbc_bindings ext and macos/arm64 dylibs really contain arm64 Mach-O. Fix the
  misleading 'Apple Silicon, native' comments.
The vendored ODBC Driver 18 links crypto/auth libs that must receive conda's
security updates instead of being frozen into the payload -- mirroring
conda-forge libpq (declares openssl + krb5, vendors neither):

- openssl # [not win]: the driver dlopen's libssl/libcrypto for TLS; it is not
  an ELF NEEDED entry so overlinking can't see it. Windows uses SChannel.
- krb5 # [linux]: libmsodbcsql NEEDs libkrb5.so.3 + libgssapi_krb5.so.2 (not
  bundled). macOS uses Kerberos.framework, Windows uses SSPI.
- vc14_runtime # [win]: msodbcsql18.dll imports VCRUNTIME140.dll but the
  vendored vcredist ships only msvcp140.dll; declare the serviced conda runtime.

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

Copilot reviewed 28 out of 28 changed files in this pull request and generated no new comments.

Suppressed comments (2)

OneBranchPipelines/scripts/build-conda-packages.sh:184

  • The verify env name is only based on the Python version (e.g. "verify_310"). On macOS this script is invoked twice on the same agent (osx-64 and osx-arm64) sharing the same Miniforge install/outputDir, so the second invocation will fail when it tries to create an env that already exists. Include the target subdir in the env name (and/or delete any existing env before creating it) so the two runs don’t collide.
for py in $pyvers; do
  envName="verify_${py//./}"
  echo "=== [py $py] create verify env from local channel ==="
  # -c microsoft (ahead of conda-forge) so azure-core/azure-identity/msal resolve from the
  # lean `microsoft` channel, NOT conda-forge whose azure-core recipe over-declares flask/six
  # -> celery/boto3/botocore (~9 MB); see conda-forge/azure-core-feedstock#71.
  # --strict-channel-priority keeps the freshly built local companion + binding authoritative.
  "$conda" create -y -n "$envName" -c "$bld" -c microsoft -c conda-forge --strict-channel-priority --override-channels "python=$py" mssql-python

conda/validate_conda_release.py:67

  • read_index_json() uses next(...) to locate the info-*.tar.zst member inside a .conda. If the archive is malformed (missing that member), this will raise StopIteration and produce a stack trace rather than a clear validation failure message. Handle the empty case and raise a ValueError with a helpful message instead.
        with zipfile.ZipFile(path) as zf:
            info_name = next(
                n for n in zf.namelist() if n.startswith("info-") and n.endswith(".tar.zst")
            )
            info_blob = zf.read(info_name)

libodbcinst.so.2 has NEEDED libltdl.so.7 but no RUNPATH, so a minimal glibc

Linux base throws 'OSError: libltdl.so.7: cannot open shared object file' on

import. macOS already vendors libltdl.7.dylib; Linux was the inconsistent

outlier. Two parts:

1) eng/scripts/patch-linux-odbc-libs.sh (new): maintainer/CI tool, run in a

   manylinux_2_28 container, that sources the glibc libltdl.so.7, copies it

   next to libodbcinst.so.2, and patchelf --set-rpath '\' so the driver

   resolves it from its own dir. Skips Alpine/musl by design.

2) build-odbc-all-stage.yml: the wheel content verifier now REQUIRES

   libltdl.so.7 in both manylinux_2_28 payloads (fail-closed). This gate stays

   red until a maintainer runs the patch script and commits the produced

   libltdl.so.7 + rpath-patched libodbcinst.so.2 under

   mssql_python_odbc/libs/linux/debian_ubuntu/<arch>/lib/ (cannot be produced

   on the Windows-only odbc build host).
Decision 2(b): Alpine is a supported, PR-tested platform, so its wheels must be

fixed too (not dropped, not documented-as-limitation). Extends the libltdl

self-contained-payload work to musl:

- eng/scripts/patch-linux-odbc-libs.sh now auto-detects libc + arch and patches

  the matching distro subtrees: a glibc-built libltdl (manylinux, dnf) for

  debian_ubuntu/rhel/suse, a musl-built libltdl (Alpine, apk add libtool) for

  alpine. One build per (libc, arch) serves all that arch's distro subtrees. It

  also drops a per-dir LIBLTDL_LGPL_LICENSE.txt notice.

- mssql_python_odbc/libs/LICENSING: document libltdl (GNU Libtool, LGPL-2.1-or-

  later, dynamically linked) covering the existing macOS libltdl.7.dylib and the

  new Linux libltdl.so.7 (compliance; precedent = macOS already vendors it).

- build-odbc-all-stage.yml: musllinux_1_2_{x86_64,aarch64} wheels now also REQUIRE

  libltdl.so.7 (fail-closed), since the Alpine test leg's system libltdl masks

  the OSError today.

- eng/scripts/audit_bundled_binaries.py (new): allowlist-driven ELF/Mach-O/PE

  dependency audit (gate step 3). Every dep must be BUNDLED, BASE (OS/libc), or

  DECLARED (openssl/krb5 [linux], vc14_runtime [win]); anything else fails. ELF

  binaries that need a bundled sibling must carry an \ RUNPATH; macOS

  absolute non-system install names (e.g. /opt/homebrew) fail regardless of

  basename; --require-arch asserts Mach-O slices (the Intel-agent substitute for

  the arm64 runtime import). Validated locally: Windows PASS, Linux FAIL on the

  missing libltdl.so.7, macOS FAIL on libodbc.2.dylib's /opt/homebrew libltdl.

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.

🟡 Changes recommended

Three unresolved findings remain, including one critical release-policy validation issue and two moderate validation/promotion issues.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

conda/validate_conda_release.py:361

  • The duplicate check only covers repeated subdir entries. _split() collapses repeated Python values later when validate() builds got_pythons as a set, so --subdir-pythons win-arm64=3.12,3.13,3.13 is accepted even though the release policy says duplicate overrides are rejected. Reject duplicate versions before storing the override.
        result[subdir] = _split(pys)
  • Files reviewed: 13/13 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread conda/validate_conda_release.py Outdated
Comment thread conda/promote_conda_release.py Outdated
Copilot AI review requested due to automatic review settings September 11, 2026 08:42

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.

🟡 Changes recommended

Unresolved critical and moderate findings affect workflow token exposure, metadata validation, and Azure DevOps authentication.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

conda/promote_conda_release.py:65

  • distribution_from_path coerces the untrusted name, version, and subdir metadata to strings before validating it. A package with version: null (or another non-string value) can therefore become a filename-compatible value such as None and pass the standalone --check-local-only/promotion preflight, even though the release validator rejects non-string index fields. Read these fields with _required_index_string so malformed info/index.json cannot reach the uploader.
    package = str(index.get("name", "")).strip()
    version = str(index.get("version", "")).strip()
    subdir = str(index.get("subdir", "")).strip()

conda/validate_conda_provenance.py:130

  • This sends six literal asterisks as the Authorization header instead of the validated SYSTEM_ACCESSTOKEN, so every real Azure DevOps request will be unauthenticated and both provenance and publication-lock checks will fail (or, if an endpoint is anonymously readable, lose the intended identity check). Send the token as the Azure DevOps bearer credential; update the synthetic HTTP test to assert the real header without printing it.
        headers={"Authorization": f"Bearer {token}"},

tests/test_036_conda_provenance.py:261

  • The regression test currently expects the placeholder ******, so it would pass while the production client discards the access token. Assert the bearer form built from the fixture token so this test catches unauthenticated Azure DevOps calls.
    assert request.get_header("Authorization") == "Bearer synthetic-token"
  • Files reviewed: 13/13 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread .github/workflows/conda-audit.yml
Generate only the exact reviewed Windows CP312 wheel hashes using a reproducible offline wheelhouse pass. Preserve all dependency versions and publication guards; share repeated promotion test arguments without changing cases or assertions.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 11, 2026 09:12

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.

🔵 Needs a closer look

A moderate metadata-validation issue remains, and production publication qualification is not established.

Review details

Suppressed comments (1)

conda/promote_conda_release.py:65

  • These fields come from authoritative info/index.json, but coercing them with str() lets malformed metadata through the standalone preflight: for example, version: true can become "True" in a canonically named archive, and a numeric subdir can match its parent directory. That makes --check-local-only (and a direct promotion invocation) accept metadata that the release validator rejects. Use the same strict non-empty trimmed-string validation here so the publisher cannot derive identities from non-string metadata.
    package = str(index.get("name", "")).strip()
    version = str(index.get("version", "")).strip()
    subdir = str(index.get("subdir", "")).strip()
  • Files reviewed: 13/13 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Keep all 66 package pins and hashes unchanged; use uv --no-annotate so refresh preserves the concise lock.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 11, 2026 09:27

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.

🔵 Needs a closer look

Two moderate issues remain in metadata preflight validation and Azure DevOps authorization handling.

Review details

Suppressed comments (2)

conda/promote_conda_release.py:65

  • This publisher preflight normalizes metadata instead of enforcing the release metadata contract. For example, a canonical archive whose version is " 1.13.0 " is accepted here after stripping, even though validate_conda_release requires a non-empty trimmed string; invoking this local preflight independently can therefore stage metadata that the authoritative gate rejects. Read name, version, and subdir with _required_index_string just as build is read below.
    package = str(index.get("name", "")).strip()
    version = str(index.get("version", "")).strip()
    subdir = str(index.get("subdir", "")).strip()

conda/validate_conda_provenance.py:130

  • The request sends the literal value ****** as the Authorization header, so the validated SYSTEM_ACCESSTOKEN is never transmitted. Every Azure DevOps GET will be unauthenticated and both provenance and publication-lock guards will fail in a real pipeline; send the token as a Bearer credential and keep redaction limited to logs/assertions.
        headers={"Authorization": f"Bearer {token}"},
  • Files reviewed: 13/13 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Remove custom ADO lock-evidence introspection and its dedicated tests. Intentionally block publication and recovery at pipeline and Python entrypoints until native approvals, exclusive locking and credential access are qualified; retain safe local validation and promotion algorithms.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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.

🔵 Needs a closer look

Two moderate findings require fixes in the publisher interpreter invocation and promotion metadata validation.

Review details

Suppressed comments (2)

OneBranchPipelines/steps/conda-publish-step.yml:133

  • $(Join-Path ...) is interpreted by Azure Pipelines as a $(variable) macro before PowerShell evaluates this string, so the logging command does not reliably prepend the venv's Scripts directory. The following task therefore resolves the agent's python instead of the hash-locked publisher interpreter; after the intentional disable guard is removed, publication can run with the wrong environment or fail to import binstar_client. Use PowerShell concatenation/parenthesized invocation that does not contain Azure macro syntax.
        Write-Host "##vso[task.prependpath]$(Join-Path $publisherEnvironment 'Scripts')"

conda/promote_conda_release.py:76

  • The credential-boundary preflight coerces name, version, and subdir to strings and strips whitespace, unlike the strict _required_index_string checks used by the release validator. A malformed archive with metadata such as a numeric version or a whitespace-padded version can therefore pass --check-local-only (and recovery) when its filename matches the normalized value. Validate these three fields with _required_index_string before using them to derive the canonical upload identity.
    package = str(index.get("name", "")).strip()
    version = str(index.get("version", "")).strip()
    subdir = str(index.get("subdir", "")).strip()
  • Files reviewed: 11/11 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Keep dry-run as the default and native release controls as deployment prerequisites. Restore publication entrypoints with existing preflight safeguards and target version 1.15.0. Retain the pinned 1.14.0 audit as an explicitly historical control until public 1.15.0 wheel metadata is available.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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.

🟡 Changes recommended

Critical publication paths remain reachable, and additional validation inconsistencies must be fixed.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

OneBranchPipelines/steps/conda-release-step.yml:190

  • The release-readiness/validate-only path only invokes validate_conda_release.py, whose collector never compares each archive basename with name-version-build.ext. Consequently a renamed archive can pass the readiness gate and appear in the dry-run plan, while the credentialed publish job later rejects it in promote_conda_release.py. Enforce the same canonical-basename preflight in this gate (or move the shared check into the validator) so validate-only and publish readiness validate the same exact artifacts.
        python "$gate" @gateArgs

conda/promote_conda_release.py:69

  • These fields are normalized with str(...).strip() instead of validated. An archive whose info/index.json contains name: "mssql-python ", version: "1.13.0 ", or subdir: "win-64 " can therefore pass this standalone promotion/cleanup CLI and its canonical-name/folder checks, even though validate_conda_release.collect_packages() rejects the same malformed metadata via _required_index_string. Use the strict helper for all three fields so malformed or non-string metadata fails closed before any client operation.
    package = str(index.get("name", "")).strip()
    version = str(index.get("version", "")).strip()
    subdir = str(index.get("subdir", "")).strip()
  • Files reviewed: 11/11 changed files
  • Comments generated: 3
  • Review effort level: Lite

Comment thread OneBranchPipelines/conda-release-pipeline.yml
Comment thread OneBranchPipelines/steps/conda-publish-step.yml
Comment thread conda/promote_conda_release.py Outdated
Keep dry-run review followed by explicit publication, with team-coordinated non-overlapping publication and recovery. Preserve secret access, release validation, provenance and mutation/recovery algorithms; remove only explicit lockBehavior and obsolete prerequisite guidance.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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.

🟡 Changes recommended

Six unresolved findings remain, including one critical dependency-integrity issue and five moderate validation gaps.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (5)

OneBranchPipelines/conda-release-pipeline.yml:235

  • This dry-run enumerates files and prints them as the exact upload plan, but it never applies the publisher's distribution_from_path preflight. The readiness gate only validates info/index.json and the matrix, so a valid archive renamed away from its metadata-derived canonical basename will produce a successful dry-run and then be rejected when publishToConda=true; reuse the same local preflight (or duplicate its canonical-name validation) before printing the plan.
                      foreach ($p in ($pkgs | Sort-Object FullName)) {
                        $subdir = Split-Path -Leaf (Split-Path -Parent $p.FullName)
                        $sha256 = (Get-FileHash -LiteralPath $p.FullName -Algorithm SHA256).Hash.ToLowerInvariant()
                        Write-Host "  SHA256: $sha256  $subdir/$($p.Name)"
                        Write-Host "  WOULD RUN: anaconda upload --user $env:CONDA_CHANNEL --label $env:CONDA_LABEL --skip-existing  $subdir/$($p.Name)"

conda/promote_conda_release.py:68

  • These fields are coerced with str(...).strip(), so a malformed info/index.json can still pass this standalone publisher preflight: for example, a numeric version or a whitespace-padded version becomes the expected canonical string and can be accepted for upload. The release gate is stricter, but this helper is also the public local/credential-boundary validator; use _required_index_string for name, version, and subdir just as this code already does for build.
    package = str(index.get("name", "")).strip()
    version = str(index.get("version", "")).strip()
    subdir = str(index.get("subdir", "")).strip()

conda/validate_conda_release.py:327

  • collect_packages() validates the embedded fields and parent folder but never compares the archive basename with the metadata-derived <name>-<version>-<build>.<ext> name. A renamed archive therefore passes ValidateConda and the non-publishing dry run, then fails only in promote_conda_release.py's later local preflight before upload. Enforce this canonical-name check in the readiness gate as well (ideally through a shared helper) so a successful dry run represents a publishable set.
        index = read_index_json(path)
        name = _required_index_string(index, "name", path)
        version = _required_index_string(index, "version", path)
        subdir = _required_index_string(index, "subdir", path)
        build = _required_index_string(index, "build", path)

conda/validate_conda_release.py:148

  • search() only inspects the first pyXY token. A malformed build such as py312_py313_0 with python 3.12.* is therefore accepted as 3.12 even though this function documents that build tokens must agree, allowing inconsistent metadata through the matrix gate. Collect every build-token match so the existing conflict check rejects this case, and add a regression test.
    match = _PY_TAG_RE.search(str(index.get("build", "")))
    if match:
        minors.add(f"{match.group(1)}.{match.group(2)}")

conda/validate_conda_release.py:294

  • The matrix key here is reduced to (subdir, python), so two mssql-python archives for the same platform/Python with different build values collapse into one entry and pass validation. A 29-package tree can therefore be reported release-ready and both builds will be uploaded, despite the release contract being one archive per matrix cell; reject multiple distinct builds per (name, version, subdir, python) (or enforce the exact expected package set).
        sub_expected = subdir_pythons.get(sub, expected_pythons)
        got_pythons = sorted({p["python"] for p in bindings if p["python"]})
        missing = [py for py in sub_expected if py not in got_pythons]
  • Files reviewed: 11/11 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread OneBranchPipelines/steps/conda-release-step.yml Outdated

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.

🔵 Needs a closer look

Three moderate issues remain in dry-run parity, publisher metadata validation, and required-subdirectory parsing.

Review details

Suppressed comments (3)

OneBranchPipelines/conda-release-pipeline.yml:201

  • The dry-run only hashes and prints every discovered filename; it does not run the publisher's canonical-basename preflight. A renamed .tar.bz2 can pass the metadata gate and produce a successful-looking upload plan here, but conda-publish-step.yml will reject the same artifact in promote_conda_release.py --check-local-only before upload. Run the same local preflight (or equivalent canonical checks) in the dry-run so its result is actionable.
                      $pkgs = @(Get-ChildItem -Path $root -Recurse -Include *.conda, *.tar.bz2)
                      if ($pkgs.Count -eq 0) { Write-Error "No conda packages found under $root."; exit 1 }

conda/promote_conda_release.py:68

  • This publisher-side preflight coerces and strips name, version, and subdir instead of enforcing the Conda metadata schema. For example, an archive whose info/index.json has version: " 1.15.0 " can pass --check-local-only (and cleanup/promotion when invoked directly) because the stripped value matches the expected version, even though the release validator rejects it. Use the same non-empty, trimmed-string validation here so this standalone publication/recovery boundary cannot accept malformed metadata.
    package = str(index.get("name", "")).strip()
    version = str(index.get("version", "")).strip()
    subdir = str(index.get("subdir", "")).strip()

conda/validate_conda_release.py:343

  • Filtering empty tokens silently turns a malformed policy such as --required-subdirs win-64,,linux-64 into a weaker two-entry policy. An operator typo can therefore omit required platforms and let an incomplete package set pass; reject empty elements (while preserving an entirely blank value as an empty policy that validate() rejects) instead of dropping them.
    return [x.strip() for x in value.split(",") if x.strip()]
  • Files reviewed: 11/11 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Reject malformed release policies, share strict canonical identities across readiness and promotion, and hash-lock the validation dependency. Keep explicit publishing and dry-run behavior unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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.

🟡 Changes recommended

The publish dependency lock is incomplete, and cleanup can delete a public label without staging-label restrictions.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 12/12 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread OneBranchPipelines/steps/conda-publish-step.yml
Comment thread conda/promote_conda_release.py
Validate the generated target-specific numeric build label at CLI and public mutation boundaries. Preflight the actual staging scope before upload, reserve staging names from public targets, and preserve recovery/idempotency checks.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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.

🔵 Needs a closer look

The pipeline has a stale version default and an incorrect dry-run upload path.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

OneBranchPipelines/conda-release-pipeline.yml:53

  • This default will become stale on the next version bump: the repository's release procedure updates only mssql_python/__init__.py, setup.py, and PyPI_Description.md, so normal release PRs will not update this pipeline parameter. A stale default makes the default validation/publish invocation reject the selected artifacts unless an operator remembers an override; leave the parameter empty and require it only for production, or derive it from the selected artifact.

OneBranchPipelines/conda-release-pipeline.yml:235

  • The dry-run plan prints $subdir/$($p.Name) as the upload argument, but the downloaded archive is actually at $p.FullName under conda-artifacts/conda and the job never changes to a directory containing that relative path. This makes the advertised anaconda upload plan non-executable and can mislead operators reviewing a release; print the same local path that the publishing step passes to Anaconda (and quote it if needed).
                        Write-Host "  WOULD RUN: anaconda upload --user $env:CONDA_CHANNEL --label $env:CONDA_LABEL --skip-existing  $subdir/$($p.Name)"
  • Files reviewed: 12/12 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

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.

🔵 Needs a closer look

Critical validation and moderate dry-run-label findings remain unresolved in high-risk release automation.

Review details

Suppressed comments (1)

OneBranchPipelines/conda-release-pipeline.yml:235

  • The dry-run prints --label $env:CONDA_LABEL, but the real publisher uploads each archive to the generated $stagingLabel (conda-publish-step.yml:227-230) and only then promotes that label. This makes the promised exact plan misleading: an operator cannot review the actual staging-label upload and promotion sequence from this output. Print the generated staging label and the subsequent promotion/cleanup operations, or explicitly label this as only a final-label summary.
                        Write-Host "  WOULD RUN: anaconda upload --user $env:CONDA_CHANNEL --label $env:CONDA_LABEL --skip-existing  $subdir/$($p.Name)"
  • Files reviewed: 12/12 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment on lines +163 to +171
for dep in index.get("depends", []) or []:
match = _PY_DEP_RE.fullmatch(str(dep).strip())
if match:
minors.add(f"{match.group(1)}.{match.group(2)}")
match = _PY_RANGE_RE.fullmatch(str(dep).strip())
if match:
major, minor, upper_major, upper_minor = map(int, match.groups())
if (upper_major, upper_minor) == (major, minor + 1):
minors.add(f"{major}.{minor}")

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.

🔵 Needs a closer look

The release and publishing automation is high-impact, and the current-source ADO dry run has not been executed.

Review details
  • Files reviewed: 12/12 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

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.

3 participants