Repository navigation
Bump mssql-python-rs to 0.3.0 - #669
Conversation
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The targeted version bump matches the documented release pipeline and preserves the Rust crate version.
Review effort: Balanced
Findings: None
What changed in this PR
Updates the independently versioned Python distribution for the next release.
Changes:
- Bumps
mssql-python-rsfrom 0.2.0 to 0.3.0.
| File | Description |
|---|---|
mssql-py-core/pyproject.toml |
Updates the Python distribution version. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
David Engel (David-Engel)
left a comment
David Engel (David-Engel)
left a comment
There was a problem hiding this comment.
Automated review
Posted by an unattended review run. These findings were not checked by a human before posting.
Verdict: clean. No blocking issues, no suggestions. One process nit below. No code-local findings, so there are no inline comments.
What I verified
- Diff scope.
git diff $(git merge-base origin/main HEAD)..HEADis exactly one line:[project].versioninmssql-py-core/pyproject.toml,0.2.0→0.3.0. The merge commit frommainadds nothing else. - Matches the documented process.
docs/release-management.md(Python and NuGet versions; Scenario 3, steps 2–3) states the NuGet transport package and themssql-python-rsPyPI distribution both take their version frommssql-py-core/pyproject.toml, and that the Rust crate inmssql-py-core/Cargo.tomlis versioned independently. Leavingmssql-py-coreat0.1.10is correct, not an oversight. - Matches precedent. The previous bump,
f64092bf("Bump mssql-python-rs to 0.2.0", #640), was also a single-line change to the same file. - No version collision. PyPI currently publishes
mssql-python-rs0.0.0,0.1.0,0.2.0.0.3.0is unpublished, so the ESRP/PyPI publish path has no conflict. - Nothing else pins this version. Every consumer reads it dynamically:
.pipeline/OneBranch/stages.yml(lines 699–722) and.pipeline/scripts/get-python-release-metadata.ps1(lines 58–60) extract[project].versionby regex, and the wheel/NuGet templates match on globs (mssql_python_rs-*). A repo-wide grep for0.2.0outsideCargo.lockturns up only unrelated crate versions (mssql-tds,mssql-mock-tds) and synthetic test fixtures — no test or script asserts the literal distribution version. - Minor rather than patch is the right increment here:
CHANGELOG.md[Unreleased]lists feature additions since0.2.0(newmssql-tdspublicSqlInfoMessage/SqlServerDiagnosticssurface,SqlType::Variant,mssql-odbcparameter binding), not just fixes. - CI: all ADO validation legs were still pending on
550ef8d7at review time;CodeQL,Analyze (actions),check, andlicense/clapass. Nothing to reconcile against the checklist yet. Absence of a Code Coverage Report comment is expected for a metadata-only change. - Description accurately describes the diff and links ADO work item 48371 (not cross-checked — ADO lookup is out of scope for this run).
Nit (non-blocking)
The checklist marks "New/changed functionality has tests" and "Public API changes are documented" as complete, but this PR adds no tests and changes no public API. Unchecking them (or noting "N/A — version bump only") keeps the checklist meaningful on PRs where those boxes carry weight. cargo btest being left unchecked with the local-environment explanation in the description is the right call.
David Engel (David-Engel)
added
the
ready for human review
Shiwani Gupta (shiwanigupta0809)
left a comment
Shiwani Gupta (shiwanigupta0809)
left a comment
There was a problem hiding this comment.
Automated review
Posted by an unattended review run. These findings were not checked by a human before posting.
The one-line version change follows the documented independent Python-distribution release flow. I found one new traceability issue, noted inline.
|
Unattended review on |
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changesNo lines with coverage information in this diff. 🔗 Quick Links |
David Engel (David-Engel)
left a comment
David Engel (David-Engel)
left a comment
There was a problem hiding this comment.
Automated review — generated by GitHub Copilot on behalf of David Engel (@David-Engel). This is not an approval and does not satisfy the human review requirement. Findings may be incomplete or wrong; push back on anything that looks off.
Summary
No new findings. The diff against the merge base is exactly one line: [project].version in mssql-py-core/pyproject.toml goes 0.2.0 → 0.3.0. The merge commit from main contributes nothing else. This looks ready for human review, but it is not approved and the human review requirement still stands.
Two earlier automated reviews on this head already raised the checklist nit and the AB#48371 traceability suggestion (the work item is titled RELEASE 0.2.0 and was already linked from #640). Those threads stand on their own; I am not re-filing them, and nothing in this run changes their severity.
Verification
Diff scope confirmed with git diff $(git merge-base origin/main HEAD)..HEAD — one file, +1/-1.
Matches the documented process. docs/release-management.md ("Python and NuGet versions"; Scenario 3, steps 2–3) states the NuGet transport package and the mssql-python-rs PyPI distribution both take their version from mssql-py-core/pyproject.toml, while the Rust crate in mssql-py-core/Cargo.toml is versioned independently. Leaving that crate at 0.1.10 is correct, not an oversight. It also matches precedent: the previous bump (f64092bf, "Bump mssql-python-rs to 0.2.0", #640) was the same single-line change to the same file.
No version collision. PyPI currently serves mssql-python-rs 0.0.0, 0.1.0, and 0.2.0; 0.3.0 is unpublished, so the publish path has no conflict.
Nothing else pins this value. .pipeline/OneBranch/stages.yml (lines 699–722) and .pipeline/scripts/get-python-release-metadata.ps1 (lines 58–60) extract [project].version by regex at build time, and check-version-not-published.py is wired only to mssql-mock-tds-py. A repo-wide grep for 0.2.0 outside lockfiles turns up only unrelated crate versions (mssql-tds, mssql-mock-tds), synthetic test fixtures, and illustrative examples in docs/release-management.md — no test or script asserts the literal distribution version.
Minor rather than patch is the right increment: CHANGELOG.md [Unreleased] lists feature additions since 0.2.0 (new mssql-tds SqlInfoMessage / SqlServerDiagnostics surface, SqlType::Variant, mssql-odbc parameter binding), not just fixes.
The file still parses: tomllib reads mssql-python-rs 0.3.0 from the updated manifest. No Rust code changed, so no build or test run was warranted for this diff.
Findings
Blocking
None.
Suggestion
None new.
Nit
None new.
CI
mssql-rs Pull request validation is failing on this head because of the Kerberos Test (PR) Kerberos Authentication Tests job (build 179066). That job failed after 3m43s, whereas it passes in roughly 7m45s on PRs #662–#668, which points at an early setup or infrastructure failure rather than a test regression. A [project].version string is not read by the Kerberos path, so this failure is very unlikely to be caused by the change. Please confirm from the job log and re-run validation; a green run on 550ef8d7 is still needed before merge. All GitHub checks (CodeQL, Analyze (actions), check, coverage-report, license/cla) and every other ADO leg pass.
Because required validation is currently red, this run made no label change.
ttk (Theekshna)
left a comment
ttk (Theekshna)
left a comment
There was a problem hiding this comment.
Automated review
Posted by an unattended review run. These findings were not checked by a human before posting.
Verdict: one Suggestion, evidence-quality only. No blocking issues. No code-local findings — the diff is one line — so there are no inline comments; the finding below concerns the PR description's own validation claims, not the diff.
What I verified
- Diff scope.
git diff $(git merge-base origin/main HEAD)..HEADon the current head (550ef8d7) is exactly one line:mssql-py-core/pyproject.toml,version = "0.2.0"->"0.3.0". - Prior discussion read in full (reviews, inline comments, issue comments, checks) before drafting anything. One open thread (shiwanigupta0809, AB#48371/
RELEASE 0.2.0traceability) and one open Nit (David-Engel, checklist boxes checked for tests/API docs on a change with neither) — both still valid, not re-filing either. CI: only the Kerberos job fails (build 179066, ~3m43s vs the ~7-8min it normally takes to pass), already called out as infra/unrelated to apyproject.tomlstring — not re-filing.
Suggestion (new)
The PR's own full-suite evidence for test_release_pipeline.py does not reproduce, and its two component counts don't sum to the file's actual test count. The description states: python -m pytest scripts/test_release_pipeline.py -q -k 'version or metadata' (13 passed) and a full run of the file (157 passed; one unrelated Windows ANSI-formatting assertion failed). I re-ran both on this checkout (Windows):
-k 'version or metadata': 13 passed — matches exactly.- Full file, no filter: 163 passed, 0 failed in my run — not 157 passed + 1 failed. Those two numbers only sum to 158, five short of the file's actual collected total (163, confirmed via
--collect-only), and no failure reproduced here. - The "ANSI-formatting" description doesn't match anything in
test_release_pipeline.py: the only ANSI-escape-stripping code in this test area (normalized_stderr,re.sub(r"\x1b\[[0-9;]*m", ...)) lives inscripts/test_verify_python_wheels.py, which the description separately claims ran clean (17/17, matching my own run) with no failure mentioned.
This doesn't change the underlying one-line change's correctness — verified independently via CI, precedent (#640), and the documented release process — so it's a Suggestion, not a Blocker. But the description is currently misattributing a failure to the wrong file/count, or reporting a flake that isn't reproducing; worth a quick re-run before merge so the validation section is accurate for whoever reads it next.
CI
Unchanged since the last review on this head: only the Kerberos job is red (infra, ~10x faster failure than a normal pass), everything else including coverage and both GitHub-native checks is green.
| Severity | Count |
|---|---|
| Blocking | 0 |
| Suggestion | 1 |
| Nit | 0 |
ttk (Theekshna)
removed
the
ready for human review
ttk (Theekshna)
left a comment
ttk (Theekshna)
left a comment
There was a problem hiding this comment.
Unattended re-review of PR #669 at 16bcc53f (merge-base 1a6911cf against main). Since my last review at 550ef8d7, exactly one commit landed: a merge of main that brings in only a dependabot bump to .github/workflows/codeql.yml (git diff 550ef8d7..HEAD --stat) — the PR's own net diff against the merge-base is unchanged: still the single mssql-py-core/pyproject.toml line, version = "0.2.0" -> "0.3.0".
Read all prior discussion first. One open thread (shiwanigupta0809: AB#48371 is titled RELEASE 0.2.0 and was already used for the 0.2.0 bump, so it doesn't document intent for 0.3.0) — still unanswered, not re-filing since it's not mine to adjudicate and the mechanism is unchanged. My own prior Suggestion (below) was in the review body, not an inline thread, so there is nothing to check for a reply to.
Six required checks
- msodbcsql parity: N/A — diff is a Python package version string, no ODBC/C++ surface.
- Test sufficiency: N/A in the mutation sense — a version-string bump has no branch or error path to guard. The only thing worth checking is whether the PR's own validation claims hold (see check 6).
- Divergences: N/A — no behavior change.
- PR description currency: unchanged and still inaccurate (see check 6) — the description was not updated since my last pass despite the mismatch being called out.
- Slop: N/A — no code or comments added.
- Evidence audit — re-derived, not carried forward blindly. Re-ran both commands on this exact head:
python -m pytest scripts/test_release_pipeline.py -q -k 'version or metadata': 13 passed — matches the description.- Full
scripts/test_release_pipeline.py, no filter: 163 passed, 0 failed on this run — the description's "157 passed; one unrelated Windows ANSI-formatting assertion failed" still does not reproduce, and 157+1=158 is still 5 short of the file's actual 163 collected tests. Unchanged from my last review since nothing in the merge touched this file (confirmed viagit diff 550ef8d7..HEAD -- scripts/test_release_pipeline.py: empty).
Carrying forward, not re-filing as new: the Suggestion below is the same one from my 550ef8d7 review, restated because the SHA changed and the marker must point at the current head. It remains unaddressed.
Suggestion
The PR description's own test-evidence table for scripts/test_release_pipeline.py still doesn't reproduce and still doesn't sum correctly (158 vs. an actual 163 collected tests), and the described "ANSI-formatting" failure doesn't match anything in that file. Doesn't affect the correctness of the one-line version bump itself — Suggestion, not Blocking — but worth a quick re-run before merge so the validation section is accurate for whoever reads it next.
CI
Same shape as last review: everything green except two still-pending jobs (Build Stage Test MacOS, mssql-python macOS cross-repo) on this rebuilt head — not re-triggering, per the run's authorization; nothing indicates a real failure.
| Severity | Count |
|---|---|
| Blocking | 0 |
| Suggestion | 1 |
| Nit | 0 |
This is an unattended automated review; findings may be incomplete or wrong, push back on anything that looks off.
ttk (Theekshna)
left a comment
ttk (Theekshna)
left a comment
There was a problem hiding this comment.
Unattended re-review of #669 at 6d4cb2e8 (merge-base a4d46fd5 against main). Since my last review at 16bcc53f, one new commit landed: a merge of main bringing in only an unrelated mssql-js dependency bump (package.json/yarn.lock) — the PR's own net diff against the merge-base is unchanged: still the single mssql-py-core/pyproject.toml line, version = "0.2.0" -> "0.3.0".
Read all prior discussion first. One open inline thread (shiwanigupta0809: AB#48371 is titled RELEASE 0.2.0 and was already used for the 0.2.0 bump, doesn't document intent for 0.3.0) — still open, not mine to adjudicate, not re-filing. My own prior Suggestion was a review-body finding, not an inline thread, so there's nothing to check for a reply to; the PR description is unchanged since my last pass.
Six required checks
- msodbcsql parity: N/A — diff is a Python package version string, no ODBC/C++ surface.
- Test sufficiency: N/A in the mutation sense — a version-string bump has no branch or error path to guard.
- Divergences: N/A — no behavior change.
- PR description currency: unchanged and still inaccurate (see check 6) — not updated since my last pass despite the mismatch being called out twice now.
- Slop: N/A — no code or comments added.
- Evidence audit — re-derived on this head, not carried forward blindly.
python -m pytest scripts/test_release_pipeline.py -q -k "version or metadata": 13 passed — matches the description. The description's claim for the unfiltered full-file run (157 passed, one unrelated Windows ANSI-formatting failure) still does not match what I've measured on two separate heads now (163 collected, 0 failures on16bcc53f); nothing in this merge touched that test file, so it's unchanged. Not re-running the full file a third time given the run budget — the filtered subset re-derivation is sufficient to confirm the mechanism (version bump correctness) while the description's own reporting remains the open item.
Carrying forward, not re-filing as new: same Suggestion as 16bcc53f, restated because the head SHA changed again.
Suggestion
The PR description's test-evidence for scripts/test_release_pipeline.py still doesn't reproduce (158 vs. an actual ~163 collected tests) and the described "ANSI-formatting" failure doesn't match anything in that file. Doesn't affect the correctness of the one-line version bump — Suggestion, not Blocking — worth a quick re-run before merge so the validation section is accurate.
CI
coverage-report's "Wait for ADO build to start" step failed shortly after this merge landed — reads as a timing/infra issue (the ADO validation pipeline was still queuing for the new head at check time), not a defect in this change. Not re-running anything per this run's authorization; flagging for a human to confirm it clears once the ADO build registers.
| Severity | Count |
|---|---|
| Blocking | 0 |
| Suggestion | 1 |
| Nit | 0 |
This is an unattended automated review; findings may be incomplete or wrong, push back on anything that looks off.
David Engel (David-Engel)
left a comment
David Engel (David-Engel)
left a comment
There was a problem hiding this comment.
Automated review — generated by GitHub Copilot on behalf of David Engel (@David-Engel). This is not an approval and does not satisfy the human review requirement. Findings may be incomplete or wrong; push back on anything that looks off.
Summary
Re-review at 6d4cb2e8 (merge-base a4d46fd5 against main). No new findings. My previous review ran at 550ef8d7; since then two merges of main landed, bringing in only unrelated upstream commits (1a6911cf github-actions dependabot group, a4d46fd5 mssql-js lodash bump). The PR's own net diff against the merge-base is still exactly one line:
mssql-py-core/pyproject.toml | 2 +-
-version = "0.2.0"
+version = "0.3.0"
No inline comments: there are no new code-local findings, and the only changed line already carries an open thread from Shiwani Gupta (@shiwanigupta0809) about work-item traceability (AB#48371 is titled RELEASE 0.2.0 and was already linked from #640). That thread stands on its own; I am not re-filing it and nothing in this run changes its severity.
Verification (re-confirmed at this head)
- Diff scope.
git diff $(git merge-base origin/main HEAD)..HEAD— one file,+1/-1. - Matches the documented process.
docs/release-management.md("Python and NuGet versions"; Scenario 3, steps 2–3) has the NuGet transport package and themssql-python-rsPyPI distribution both take their version frommssql-py-core/pyproject.toml, while the Rust crate inmssql-py-core/Cargo.tomlis versioned independently. That crate is still at0.1.10— correct, not an oversight. Precedent matches: the0.2.0bump (#640) was the same single-line change to the same file. - No version collision. PyPI currently serves
mssql-python-rs0.0.0,0.1.0,0.2.0.0.3.0is unpublished, so the publish path is clear. - Nothing else pins this value.
.pipeline/OneBranch/stages.ymland.pipeline/scripts/get-python-release-metadata.ps1extract[project].versionby regex at build time;check-version-not-published.pyis wired only tomssql-mock-tds-py. A repo-wide grep for0.2.0outside lockfiles turns up only unrelated crate versions (mssql-tds,mssql-mock-tds), a#[deprecated(since = "0.2.0")]attribute, synthetic test fixtures, and doc examples — no test or script asserts the literal distribution version. - Manifest still parses.
tomllibreadsmssql-python-rs 0.3.0from the updated file. - Minor rather than patch is right.
CHANGELOG.md[Unreleased]lists feature additions since0.2.0, not only fixes. - No Rust code changed, so no build or test run was warranted for this diff.
Findings
Blocking: none. Suggestion: none new. Nit: none new.
CI
Required validation is not green on this head, so this run made no label change.
coverage-reportfailed in 10s, and the failure is an infrastructure race, not a signal about this change. TheWait for ADO build to startstep took the first build it found for PR #669 — build 179109, alreadycompleted/failedfrom an earlier superseded run — and exited 1 (❌ Azure DevOps build 179109 failed; cannot report coverage.) instead of waiting for the build actually queued for6d4cb2e8, which is 179127. Worth noting to whoever owns that workflow: the lookup should match the build to the head SHA (or skip superseded/cancelled builds) rather than taking the newest build for the PR number.mssql-rs Pull request validation(build 179127) is still pending — several legs (Build Windows, Build Linux, Test MacOS, Build mssql-python macOS) have not reported. The legs that have finished pass, including the Kerberos job that was red on my previous review, so that earlier failure does look like the transient infra failure I suspected.- All other GitHub checks pass:
CodeQL,Analyze (actions),check,license/cla.
Please confirm 179127 goes green (and re-run coverage-report once it does) before merge.
David Engel (David-Engel)
left a comment
David Engel (David-Engel)
left a comment
There was a problem hiding this comment.
Automated review — generated by GitHub Copilot on behalf of David Engel (@David-Engel). This is not an approval and does not satisfy the human review requirement. Findings may be incomplete or wrong; push back on anything that looks off.
Summary
Re-review at 6d4cb2e8, unchanged head since my previous run. No new findings, and nothing here supersedes or re-litigates the earlier automated reviews on this PR — see them for the full verification trace rather than repeating it. The only material change since then is CI: the ADO validation build (179127) and coverage-report are now green on this head, which resolves the two CI items my last run asked to be confirmed. The coverage-report infrastructure race I flagged (the workflow attaching to superseded build 179109 instead of the one queued for this head) is a workflow-ownership observation, not a finding against this PR.
The net diff against merge base a4d46fd5 remains exactly one line: [project].version in mssql-py-core/pyproject.toml, 0.2.0 → 0.3.0. No inline comments, because there are no code-local findings and the single changed line already carries an open traceability thread from Shiwani Gupta (@shiwanigupta0809) that stands on its own.
Verification
Re-confirmed independently at this head: the diff is one file, +1/-1; mssql-py-core/Cargo.toml correctly stays at its independent 0.1.10, matching docs/release-management.md ("Python and NuGet versions", Scenario 3 steps 2–3) and the precedent set by f64092bf (#640), which was the same single-line change; PyPI serves mssql-python-rs 0.0.0, 0.1.0, 0.2.0, so 0.3.0 is unpublished and the publish path is clear; the pipeline reads [project].version by regex (.pipeline/OneBranch/stages.yml, .pipeline/scripts/get-python-release-metadata.ps1) and check-version-not-published.py is wired only to mssql-mock-tds-py, so no test or script asserts the literal distribution version; a minor rather than patch bump is right given the feature additions listed under CHANGELOG.md [Unreleased]. No Rust code changed, so no build or test run was warranted.
gh pr checks 669 is green across the board: Analyze (actions), CodeQL, check, coverage-report, license/cla, and every leg of mssql-rs Pull request validation (build 179127, 53m18s).
Findings
Blocking: none. Suggestion: none new. Nit: none new.
This looks ready for human review, but it is not approved — the human review requirement still stands.
David Engel (David-Engel)
added
the
ready for human review
### Work Item / Issue Reference > AB#48322 ------------------------------------------------------------------- ### Summary Update the mssql-python-rs installers to select the stable-ABI `cp310-abi3` wheel for each target platform, with focused tests for the wheel-selection contract. Adopt the released `mssql-python-rs` 0.3.0 package for both the public dependency and internal NuGet transport before the next `mssql-python` release. Validation: - PyPI `mssql-python-rs==0.3.0` is published with all nine stable-ABI wheels. - Clean Windows venv installed `mssql_python_rs-0.3.0-cp310-abi3-win_amd64.whl`; package metadata reported `0.3.0` and `mssql_py_core` imported successfully. - `python -m pytest --noconftest tests/test_037_rs_wheel_download.py tests/test_038_mssql_odbc_daily_validation.py -q` (13 passed, 5 platform skips). - Generated package metadata contains `mssql-python-rs==0.3.0`. - `black --check --line-length=100 mssql_python tests` (101 files unchanged). Producer: microsoft/mssql-rs#669
Description
Bump the
mssql-python-rsPython distribution version from 0.2.0 to 0.3.0. The independently versionedmssql-py-coreRust crate remains unchanged.Related Issues
https://sqlclientdrivers.visualstudio.com/mssql-rs/_workitems/edit/48371
Validation
cargo bfmtcargo bclippypython -m pytest scripts/test_verify_python_wheels.py -q(17 passed)python -m pytest scripts/test_release_pipeline.py -q -k 'version or metadata'(13 passed)scripts/test_release_pipeline.py: 157 passed; one unrelated Windows ANSI-formatting assertion failedcargo btestcompiled successfully; SQL Server integration tests failed because the local test environment is not configuredChecklist
cargo bfmtpassescargo bclippypassescargo btestpasses