Repository navigation
CI: retry dependency downloads and stabilize the quoted deadlock fixture - #4125
Merged
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
No unresolved issues were identified, and integrity checks remain unchanged.
Review effort: Lite
Findings: None
What changed in this PR
Improves resilience of SQL Server smoke-test dependency downloads against transient HTTP and network failures.
Changes:
- Adds bounded curl retries with backoff and
Retry-Aftersupport. - Configures connection, transfer, and retry timeouts.
- Preserves pinned URLs, revisions, and checksum verification.
| File | Description |
|---|---|
.github/scripts/run-sql-server-smoke-tests.sh |
Adds resilient curl options for pinned dependency downloads. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Fixes #4124. Fixes #4126.
The SQL Server smoke tests can fail before execution on transient dependency-download errors, or during the quoted-output regression once its fixed deadlock XML ages out of sp_BlitzLock's default seven-day window.
Enable curl's built-in transient-error retries, including connection refusals, with up to five retries after the initial attempt. Curl honors Retry-After and otherwise uses exponential backoff. Set a 180-second retry budget, a 10-second connection timeout, and a 30-second timeout per transfer; the final transfer can extend past the retry budget. The dependency revision, URLs, and SHA-256 checks before installation remain unchanged, with no token required.
For the quoted sp_BlitzLock output test, supply the same explicit September 11–14, 2026 date range on both create/append calls. It contains the XML fixture's September 12 timestamp with timezone margin. The table-identity, nonempty-output, two-participant, and append assertions remain intact.
The failure reported in #4126 occurred on both SQL Server 2017 and 2025. The logs end in sp_BlitzLock's
Results to table/Findings to tablemessages followed by the empty-output assertion. The September 12 event was already older than seven days at the September 19 run. This fix addresses that expired test input; it does not require changes to production identifier handling.Validation:
git diff --checkpassed.