ci: retry PyPI install smoke test to tolerate index propagation#437
Merged
ci: retry PyPI install smoke test to tolerate index propagation#437
Conversation
The post-publish install check ran seconds after upload and failed because PyPI's index had not yet surfaced the new version. Retry the pip install up to 8 times (15s apart) before failing, and normalize the tag-derived version to strip the leading "v" so pyproject.toml and the install specifier use the PEP 440 canonical form.
dosubot
Bot
added
the
size:XS
📝 WalkthroughWalkthroughThe publish package workflow now normalizes VERSION by removing leading Changes
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #437 +/- ##
=======================================
Coverage 78.02% 78.02%
=======================================
Files 35 35
Lines 4319 4319
=======================================
Hits 3370 3370
Misses 949 949 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/publish_package.yml:
- Around line 29-32: Compute and expose the canonical VERSION once in the
build-n-publish-pypi job and consume it in test-pip-installation via job outputs
to avoid duplicating the v-stripping logic: move the VERSION computation (use
parameter expansion that strips both lowercase and uppercase prefixes, e.g.
`${VERSION#[vV]}`) into the build-n-publish-pypi step, set it as an output
(jobs.build-n-publish-pypi.outputs.version), and update test-pip-installation to
read needs.build-n-publish-pypi.outputs.version instead of recomputing; update
any other duplicated occurrences (e.g., the block around lines 78-81) to use the
shared output as well.
- Around line 83-93: In the "Install Comfy CLI via pip and Test" step update the
retry loop to capture and surface pip's failure details and to bound each
attempt with a timeout: run pip install "comfy-cli==${VERSION}" under a
per-attempt timeout (e.g. with the timeout utility) and use pip's network
timeout flag (--timeout) as well; on a failed attempt capture pip's exit code
and stderr/stdout (or at least echo the exit code) before sleeping so the final
"::error::" contains diagnostic info; ensure the loop still exits on success
(exit 0) and preserves the existing retry count and sleep behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 033fb598-a19f-4de0-a5c7-94b24018af5d
📒 Files selected for processing (1)
.github/workflows/publish_package.yml
Comment on lines
+29
to
+32
| run: | | ||
| VERSION="${GITHUB_REF#refs/tags/}" | ||
| VERSION="${VERSION#v}" | ||
| echo "VERSION=${VERSION}" >> $GITHUB_ENV |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Tiny DRY nit: the v-stripping dance is duplicated in both jobs.
No bug here — just a pinch of repetition. You could compute VERSION once in build-n-publish-pypi and expose it via jobs.<id>.outputs, then consume it in test-pip-installation through needs.build-n-publish-pypi.outputs.version. One tag, one truth — no copy-paste to chase.
Also worth noting: ${VERSION#v} strips only a lowercase leading v. If a maintainer ever tags V1.2.3, it'll sneak through un-stripped. Unlikely given your tagging convention, but a ${VERSION#[vV]} would slam that door shut.
♻️ Optional refactor sketch (share the version across jobs)
build-n-publish-pypi:
...
+ outputs:
+ version: ${{ steps.get_version.outputs.version }}
steps:
...
- name: Extract version from tag
id: get_version
run: |
VERSION="${GITHUB_REF#refs/tags/}"
- VERSION="${VERSION#v}"
+ VERSION="${VERSION#[vV]}"
echo "VERSION=${VERSION}" >> $GITHUB_ENV
+ echo "version=${VERSION}" >> $GITHUB_OUTPUT
...
test-pip-installation:
needs: build-n-publish-pypi
...
steps:
...
- - name: Extract version from tag
- id: get_version
- run: |
- VERSION="${GITHUB_REF#refs/tags/}"
- VERSION="${VERSION#v}"
- echo "VERSION=${VERSION}" >> $GITHUB_ENV
+ - name: Resolve version from upstream job
+ run: echo "VERSION=${{ needs.build-n-publish-pypi.outputs.version }}" >> $GITHUB_ENVAlso applies to: 78-81
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/publish_package.yml around lines 29 - 32, Compute and
expose the canonical VERSION once in the build-n-publish-pypi job and consume it
in test-pip-installation via job outputs to avoid duplicating the v-stripping
logic: move the VERSION computation (use parameter expansion that strips both
lowercase and uppercase prefixes, e.g. `${VERSION#[vV]}`) into the
build-n-publish-pypi step, set it as an output
(jobs.build-n-publish-pypi.outputs.version), and update test-pip-installation to
read needs.build-n-publish-pypi.outputs.version instead of recomputing; update
any other duplicated occurrences (e.g., the block around lines 78-81) to use the
shared output as well.
Comment on lines
83
to
+93
| - name: Install Comfy CLI via pip and Test | ||
| run: pip install comfy-cli==${{env.VERSION}} | ||
| run: | | ||
| # PyPI's index can lag behind a successful upload by a minute or | ||
| # two, so retry before failing the job. | ||
| for i in 1 2 3 4 5 6 7 8; do | ||
| pip install --no-cache-dir "comfy-cli==${VERSION}" && exit 0 | ||
| echo "Attempt $i: package not yet available on PyPI, waiting 15s..." | ||
| sleep 15 | ||
| done | ||
| echo "::error::Failed to install comfy-cli==${VERSION} after 8 attempts" | ||
| exit 1 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Retry loop looks solid — here's a small polish to keep diagnostics crisp.
The logic holds up (thanks to GitHub's default bash -eo pipefail, the failing pip install inside a && list won't abort the script, so the retry actually retries — nice). Two small, optional tweaks:
- Capture why pip bailed. Right now any failure — stale index, network hiccup, resolver conflict — looks identical. Teeing pip's output (or echoing its exit code) on each attempt makes post-mortems far less mysterious when the loop exhausts.
- Guard against a pip-hang.
pip installhas no built-in per-attempt deadline, so in the pathological case you could burn the runner's job timeout on a single stuck attempt. A--timeout(network socket timeout) plus a wrappingtimeout 60sgives the retry loop teeth.
🛠️ Proposed polish for the retry loop
- name: Install Comfy CLI via pip and Test
run: |
# PyPI's index can lag behind a successful upload by a minute or
# two, so retry before failing the job.
for i in 1 2 3 4 5 6 7 8; do
- pip install --no-cache-dir "comfy-cli==${VERSION}" && exit 0
- echo "Attempt $i: package not yet available on PyPI, waiting 15s..."
+ if timeout 60s pip install --no-cache-dir --timeout 30 "comfy-cli==${VERSION}"; then
+ exit 0
+ fi
+ echo "Attempt $i failed (exit=$?); waiting 15s for PyPI index to catch up..."
sleep 15
done
echo "::error::Failed to install comfy-cli==${VERSION} after 8 attempts"
exit 1A rhyme to sum it up: the loop is sound and tightly bound; log the cause so the fault is found. 🐇
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/publish_package.yml around lines 83 - 93, In the "Install
Comfy CLI via pip and Test" step update the retry loop to capture and surface
pip's failure details and to bound each attempt with a timeout: run pip install
"comfy-cli==${VERSION}" under a per-attempt timeout (e.g. with the timeout
utility) and use pip's network timeout flag (--timeout) as well; on a failed
attempt capture pip's exit code and stderr/stdout (or at least echo the exit
code) before sleeping so the final "::error::" contains diagnostic info; ensure
the loop still exits on success (exit 0) and preserves the existing retry count
and sleep behavior.
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
1 participant
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.
Summary
The post-publish smoke test in
publish_package.ymlfailed on the v1.7.3 release (run 24876171699) even though the package itself uploaded successfully. The install job ran ~30s after publish and pip reported no matching distribution — PyPI's index simply had not yet surfaced the new version. The previous TestPyPI validation step had a retry loop for exactly this reason; it was lost when trusted publishing replaced it.Changes:
pip install comfy-cli==$VERSIONup to 8× with 15s between attempts (~2 min total) before failing the job, with a clear::error::annotation on final failure.vfrom the tag-derivedVERSIONin both jobs sopyproject.tomland the install specifier use the PEP 440 canonical form (1.7.3) rather than relying on implicit normalization. pip was already normalizing==v1.7.3transparently, so this is hygiene rather than a functional fix.