📝 ReadTheDocs for Bindings using TestPyPI#833
Conversation
WalkthroughAdds a TestPyPI publish job to the CI workflow, introduces a retrying Read the Docs build step that pulls from TestPyPI, updates docs dependencies in pyproject to include mnt.pyfiction, and changes a C++ binding template’s default lattice parameter from "" to "TRIGGER". Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GA as GitHub Actions
participant Art as actions/download-artifact
participant Pub as pypa/gh-action-pypi-publish
participant TPyPI as Test PyPI
Dev->>GA: Push/Tag triggers workflow
GA->>GA: build_wheels, build_sdist
GA->>Art: Download artifacts (pattern: cibw-*)
Art-->>GA: Wheels in dist/
GA->>Pub: Publish dist/* to TestPyPI (skip-existing, verbose)
Pub->>TPyPI: Upload packages
TPyPI-->>Pub: 200 OK or Already exists
Pub-->>GA: Publish result
sequenceDiagram
autonumber
participant RTD as Read the Docs
participant Bash as build command loop
participant UV as uv
participant TPyPI as Test PyPI
RTD->>Bash: Start docs build
loop Up to 300 attempts
Bash->>UV: pip config set extra-index TestPyPI
Bash->>UV: run ... build docs
alt Success
UV-->>Bash: 0 exit
Bash-->>RTD: Proceed (break)
else Failure
UV-->>Bash: non-zero exit
Bash-->>Bash: Sleep 30s then retry
end
end
note over Bash: On exhaustion: print failure and exit 1
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
Signed-off-by: GitHub Actions <actions@github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyproject.toml (1)
236-243: Remove self-dependency from pyproject (docs + test); install package in CI/RTD instead.Listing "mnt.pyfiction" in optional-dependencies causes circular resolution/TestPyPI pulls and lockfile drift. Occurrences: pyproject.toml:49 (test = ["pytest>=7.2", "mnt.pyfiction"]) and pyproject.toml:242 (docs); pyproject.toml:9–11 are the project name/version.
Apply this diff to drop it from docs:
docs = [ "setuptools-scm>=8.1", "sphinx==8.2.3; python_version >= '3.11'", "breathe==4.36.0", "sphinx-rtd-theme==3.0.2", "sphinx-tabs==3.4.7", - "mnt.pyfiction" ]Also remove "mnt.pyfiction" from [project.optional-dependencies].test and instead install the built package in CI/test jobs; for RTD run:
uv pip install --extra-index-url https://test.pypi.org/simple/ mnt.pyfiction==${PROJECT_VERSION} before Sphinx.
🧹 Nitpick comments (3)
bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp (1)
18812-18812: Remove temporary trigger marker from user-facing docstring.The string ">>>REMOVETHIS_ONLYFORTRIGGER<<<" will leak into published docs. Please revert before merge (or use a non-user-facing knob to trigger RTD rebuilds).
- FCN concept.>>>REMOVETHIS_ONLYFORTRIGGER<<<)doc"; + FCN concept.)doc";include/fiction/technology/cell_technologies.hpp (1)
14-15: Remove temporary trigger marker from user-facing docs.The marker ">>>REMOVETHIS_ONLYFORTRIGGER<<<" should not ship in main. Drop it before merge.
- * Quantum-dot Cellular Automata (QCA) technology implementation of the FCN concept.>>>REMOVETHIS_ONLYFORTRIGGER<<< + * Quantum-dot Cellular Automata (QCA) technology implementation of the FCN concept..readthedocs.yml (1)
29-44: Retry loop: fix edge-case failure, reduce retries, and avoid per-iteration config.
- If success happens exactly on attempt 300, the post-loop check still fails. Track success explicitly.
- 300×30s ≈ 150 minutes > RTD timeouts; reduce attempts/backoff or cap total wait.
- Set extra-index once before the loop.
html: - - | - set -euo pipefail - for i in {1..300}; do - uv pip config set global.extra-index-url https://test.pypi.org/simple/ - if uv run --frozen --no-dev --group docs -m sphinx -T -b html -d docs/_build/doctrees -D language=en docs "$READTHEDOCS_OUTPUT/html"; then - break - fi - echo "Retrying in 30s... ($i/300)" - sleep 30 - done - # If we didn't break early, fail explicitly - if [ "${i:-300}" -eq 300 ]; then - echo "Obtaining bindings from TestPyPI failed after 300 attempts." - exit 1 - fi + - | + set -euo pipefail + uv pip config set global.extra-index-url https://test.pypi.org/simple/ + max=20 + delay=15 + ok=0 + for i in $(seq 1 "$max"); do + if uv run --frozen --no-dev --group docs -m sphinx -T -b html -d docs/_build/doctrees -D language=en docs "$READTHEDOCS_OUTPUT/html"; then + ok=1; break + fi + echo "Retrying in ${delay}s... ($i/$max)" + sleep "$delay" + done + if [ "$ok" -ne 1 ]; then + echo "Obtaining bindings from TestPyPI failed after $max attempts." + exit 1 + fiAlso, in post_checkout, you reference ".readthedocs.yaml" but the file is ".readthedocs.yml". Consider fixing that path to avoid skipping builds unintentionally.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/pyfiction-pypi-deployment.yml(1 hunks).readthedocs.yml(1 hunks)bindings/mnt/pyfiction/include/pyfiction/pybind11_mkdoc_docstrings.hpp(1 hunks)include/fiction/technology/cell_technologies.hpp(1 hunks)pyproject.toml(1 hunks)
| publish_to_testpypi: | ||
| runs-on: ubuntu-latest | ||
| name: 🧪 Publish to TestPyPI | ||
| needs: [build_wheels, build_sdist] | ||
| steps: | ||
| - name: Download the previously stored artifacts | ||
| uses: actions/download-artifact@v4 | ||
| with: | ||
| pattern: cibw-* | ||
| path: dist | ||
| merge-multiple: true | ||
|
|
||
| - name: Deploy to TestPyPI | ||
| uses: pypa/gh-action-pypi-publish@release/v1 | ||
| with: | ||
| repository-url: https://test.pypi.org/legacy/ | ||
| password: ${{ secrets.TEST_PYPI_DEPLOY_TOKEN }} | ||
| skip-existing: true | ||
| verbose: true | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Harden TestPyPI publish for forks and align artifact action version.
- Forked PRs don’t receive repository secrets; this job will fail and break RTD expectations. Gate the job on the presence of the secret.
- Use the same download-artifact major version across jobs (v5 is used below).
publish_to_testpypi:
runs-on: ubuntu-latest
name: 🧪 Publish to TestPyPI
- needs: [build_wheels, build_sdist]
+ needs: [build_wheels, build_sdist]
+ if: ${{ secrets.TEST_PYPI_DEPLOY_TOKEN != '' }}
steps:
- name: Download the previously stored artifacts
- uses: actions/download-artifact@v4
+ uses: actions/download-artifact@v5
with:
pattern: cibw-*
path: dist
merge-multiple: trueOptional: also add permissions: contents: read at the job level if your org uses restrictive defaults.
Run to confirm actions versions:
🏁 Script executed:
#!/bin/bash
rg -n 'uses:\s*actions/download-artifact@' -n -C1 .github/workflowsLength of output: 538
Gate TestPyPI publish for forks and align download-artifact to v5
Gate the job on TEST_PYPI_DEPLOY_TOKEN and standardize the download-artifact action to v5; optionally add job-level permissions: contents: read.
publish_to_testpypi:
runs-on: ubuntu-latest
name: 🧪 Publish to TestPyPI
- needs: [build_wheels, build_sdist]
+ needs: [build_wheels, build_sdist]
+ if: ${{ secrets.TEST_PYPI_DEPLOY_TOKEN != '' }}
steps:
- name: Download the previously stored artifacts
- uses: actions/download-artifact@v4
+ uses: actions/download-artifact@v5
with:
pattern: cibw-*
path: dist
merge-multiple: true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| publish_to_testpypi: | |
| runs-on: ubuntu-latest | |
| name: 🧪 Publish to TestPyPI | |
| needs: [build_wheels, build_sdist] | |
| steps: | |
| - name: Download the previously stored artifacts | |
| uses: actions/download-artifact@v4 | |
| with: | |
| pattern: cibw-* | |
| path: dist | |
| merge-multiple: true | |
| - name: Deploy to TestPyPI | |
| uses: pypa/gh-action-pypi-publish@release/v1 | |
| with: | |
| repository-url: https://test.pypi.org/legacy/ | |
| password: ${{ secrets.TEST_PYPI_DEPLOY_TOKEN }} | |
| skip-existing: true | |
| verbose: true | |
| publish_to_testpypi: | |
| runs-on: ubuntu-latest | |
| name: 🧪 Publish to TestPyPI | |
| needs: [build_wheels, build_sdist] | |
| if: ${{ secrets.TEST_PYPI_DEPLOY_TOKEN != '' }} | |
| steps: | |
| - name: Download the previously stored artifacts | |
| uses: actions/download-artifact@v5 | |
| with: | |
| pattern: cibw-* | |
| path: dist | |
| merge-multiple: true | |
| - name: Deploy to TestPyPI | |
| uses: pypa/gh-action-pypi-publish@release/v1 | |
| with: | |
| repository-url: https://test.pypi.org/legacy/ | |
| password: ${{ secrets.TEST_PYPI_DEPLOY_TOKEN }} | |
| skip-existing: true | |
| verbose: true |
🤖 Prompt for AI Agents
.github/workflows/pyfiction-pypi-deployment.yml around lines 133 to 152: the
TestPyPI publish job should be gated on the presence of TEST_PYPI_DEPLOY_TOKEN,
the download-artifact action must be standardized to v5, and job-level read
permissions should be set; add an if condition to the job like if: ${{
secrets.TEST_PYPI_DEPLOY_TOKEN != '' }}, change uses:
actions/download-artifact@v4 to uses: actions/download-artifact@v5, and add a
permissions block for the job (permissions: contents: read) to restrict scope.
Signed-off-by: GitHub Actions <actions@github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
bindings/mnt/pyfiction/include/pyfiction/technology/charge_distribution_surface.hpp (1)
282-285: If you must keep "TRIGGER", at least preserve the old default binding name.Explicitly pass an empty suffix for the default instantiation so it still exports
charge_distribution_surface:- detail::charge_distribution_surface_layout<py_sidb_layout>(m); + detail::charge_distribution_surface_layout<py_sidb_layout>(m, "");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bindings/mnt/pyfiction/include/pyfiction/technology/charge_distribution_surface.hpp(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: 🐍 windows-latest
- GitHub Check: 🐧 ubuntu-24.04 with g++-13
- GitHub Check: 🐧 ubuntu-24.04-arm with clang++-18
- GitHub Check: 🐧 ubuntu-24.04-arm with clang++-16
- GitHub Check: 🐧 ubuntu-24.04 with g++-11
- GitHub Check: 🐧 ubuntu-24.04 with clang++-17
- GitHub Check: 🛞 Wheels for macos-13
- GitHub Check: 🛞 Wheels for ubuntu-24.04
- GitHub Check: 🛞 Wheels for macos-14
- GitHub Check: 🛞 Wheels for windows-latest
- GitHub Check: 🛞 Wheels for ubuntu-24.04-arm
- GitHub Check: 🪟 windows-2022 with Visual Studio 17 2022 and v143 toolset
- GitHub Check: 🪟 windows-2025 with Visual Studio 17 2022 and ClangCL toolset
- GitHub Check: 🪟 windows-2025 with Visual Studio 17 2022 and v143 toolset
- GitHub Check: 🪟 windows-2022 with Visual Studio 17 2022 and ClangCL toolset
- GitHub Check: Analyze python
- GitHub Check: Analyze cpp
- GitHub Check: Coverage on ubuntu-22.04 with g++-11
- GitHub Check: 🐳 Build and publish Docker image
- GitHub Check: 🚨 Clang-Tidy
2 participants
Description
Currently, ReadTheDocs builds fiction within the github action, which uses limited resources. This can cause OOM issues, which can be prevented by simply installing the bindings through pulling them from TestPyPI. This PR implements this.
Checklist:
Summary by CodeRabbit