CHORE: Flaky Setup - Fail SQL Server setup loudly and dump container logs on every CI leg - #722
Open
Gaurav Sharma (bewithgaurav) wants to merge 6 commits into
Open
Conversation
the macOS and code-coverage legs poll SQL Server in a loop whose exit status is the final sleep, so the step reported success even when every probe failed. a dead container then surfaced as 30+ minutes of pytest errors instead of an infra failure. both loops now track readiness in a flag, re-probe once after the loop, and exit 1 with container logs when SQL never came up. the six linux legs end their step with CREATE DATABASE, whose exit status already fails them, so they are left alone. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Gaurav Sharma (bewithgaurav)
changed the title
Gaurav Sharma (bewithgaurav)
marked this pull request as ready for review
August 17, 2026 06:43
Copilot started reviewing on behalf of
Gaurav Sharma (bewithgaurav)
August 17, 2026 06:43
View session
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a flaky SQL Server readiness gate in the Azure Pipelines validation workflow (macOS and code-coverage legs) where the step could incorrectly succeed even if all readiness probes failed, causing long-running pytest failures against a non-existent DB.
Changes:
- Track SQL Server readiness via a
sql_readyflag instead of relying on the loop’s final command exit status. - Add a final “one last probe” after the loop to handle the case where SQL becomes reachable during the last sleep interval.
- Fail fast with a clear error and include
docker logs --tail 200output when SQL Server never becomes ready.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
📊 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.pybind.ddbc_bindings.h: 61.5%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 75.5%
mssql_python.__init__.py: 77.6%
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: 84.3%
mssql_python.logging.py: 85.5%🔗 Quick Links
|
…t-sql-readiness-check # Conflicts: # eng/pipelines/pr-validation-pipeline.yml
…github.com/microsoft/mssql-python into bewithgaurav/fail-fast-sql-readiness-check
the macOS setup_sql readiness loop reports that SQL never came up but not why, because sqlcmd only tells you the client could not connect. when the container itself exited (password policy, EULA, OOM, port clash) the reason is only in the container's own log, so dump docker logs on the failure path. also probe once more after the loop: the loop sleeps after its final attempt, so SQL can become reachable in that window and would otherwise be reported as a hard failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Gaurav Sharma (bewithgaurav)
changed the title
extends the macOS change to the remaining seven readiness loops. the CodeCoverageReport loop had the same silent-success shape as macOS did: its exit status is the final sleep, so a dead container let the step pass. the six linux distro legs already failed through CREATE DATABASE, but only as an opaque sqlcmd error with no container log. all eight sites now share one shape: track readiness in a flag, probe once more after the loop, and on failure print the attempt count plus docker logs before exiting non-zero. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
pr-size: medium
Gaurav Sharma (bewithgaurav)
changed the title
Gaurav Sharma (bewithgaurav)
changed the title
Jahnvi Thakkar (jahnvi480)
approved these changes
Aug 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Work Item / Issue Reference
Summary
when SQL Server fails to come up in CI, the pipeline either does not fail at all or fails without saying why. this makes all eight readiness loops behave the same way.
two of the eight never failed. the macOS one and the CodeCoverageReport one polled in a loop whose exit status is the final
sleep 2, so the step reported success even when every probe failed. build 166760 hit this on macOS: the container died on startup, all 30 probes loggedcontainer ... is not running, the step went green, and pytest then ran 32.6 minutes against a database that did not exist, erroring on every db-backed test from the 1% mark while the non-db tests passed. #723 has since fixed the macOS half as part of the setup parallelization, so what is left here is CodeCoverageReport.the other six linux distro legs do fail, through
CREATE DATABASE TestDBright after the loop, but you only get a sqlcmd connect error. nothing anywhere in the pipeline capturesdocker logs, so when the container exits on its own (SA password policy, EULA, OOM, port already bound) the reason is never recorded. that is why 166760's root cause is still unknown.every site now tracks readiness in a flag, probes once more after the loop, and on failure prints the attempt count followed by
docker logs --tail 200before exiting non-zero. the extra probe matters because the loop sleeps after its final attempt, so SQL can become reachable in that window and would otherwise be called a hard failure.verified locally against real containers: a container that prints a startup error and exits now surfaces that error instead of only the connect failure, a healthy container is unaffected, and a container that becomes reachable only after the loop is rescued by the re-probe. healthy macOS runs use 3 to 6 of the 30 attempts, so the gate keeps roughly 5x headroom and no green run changes behaviour.