Upstream fragility and narrow dependency#140
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR switches the Weblate pin to ChangesWeblate pin handling and release parsing
Sequence Diagram(s)sequenceDiagram
participant W as .github/workflows/weblate-pin-bump.yml
participant S as scripts/check-weblate-internal-contract.sh
participant U as uv
participant P as PyPI
participant Py as pytest
participant T as tests/test_weblate_internal_contract.py
W->>S: start contract-latest with --latest
S->>U: query PyPI JSON for latest Weblate release
U->>P: fetch release list
S->>U: install Weblate[postgres]==<latest>
S->>Py: run pytest -m weblate_contract
Py->>T: execute contract tests
Py-->>S: exit status
S-->>W: pass or fail
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
scripts/check-weblate-internal-contract.sh (1)
59-60: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a timeout to the PyPI request.
urllib.request.urlopenwithout atimeoutcan hang indefinitely if PyPI is unresponsive, stalling the workflow until the job-level timeout. Pass an explicit timeout.🔧 Proposed fix
-with urllib.request.urlopen("https://pypi.org/pypi/Weblate/json") as resp: +with urllib.request.urlopen("https://pypi.org/pypi/Weblate/json", timeout=30) as resp: data = json.load(resp)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check-weblate-internal-contract.sh` around lines 59 - 60, The PyPI fetch in check-weblate-internal-contract.sh can hang indefinitely because urllib.request.urlopen is called without a timeout. Update the request in the Weblate JSON loading block to pass an explicit timeout value through urlopen so the workflow fails fast if PyPI is unresponsive. Keep the change localized to the urllib.request.urlopen/json.load flow used for the Weblate package lookup.tests/test_weblate_internal_contract.py (1)
60-71: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueReorder the tuple-type check before the emptiness check.
if not formatswould also be True ifformatsisNoneor a non-tuple falsy value, surfacing the "empty tuple" message for a type-contract break. Checkingisinstance(..., tuple)first yields a clearer diagnostic for the actual contract violation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_weblate_internal_contract.py` around lines 60 - 71, The contract check in weblate_formats_with_plugin_formats() is validating tuple emptiness before confirming the value is actually a tuple, which can mask type-contract failures with an “empty tuple” message. In the test_weblate_internal_contract assertion block, move the isinstance(formats, tuple) check ahead of the if not formats check so non-tuple or None values fail with the correct type diagnostic first, then keep the empty-tuple assertion afterward.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/WORKFLOWS.md:
- Line 25: The “Weblate version pinning” table still shows the wrong example
package pin; update the `pyproject.toml` example entry to match the current pin
used by the repo. Locate the table row in `.github/WORKFLOWS.md` and change the
`Weblate[all]==2026.5` example to `Weblate[postgres]==2026.5` so the documented
example aligns with the actual pin.
---
Nitpick comments:
In `@scripts/check-weblate-internal-contract.sh`:
- Around line 59-60: The PyPI fetch in check-weblate-internal-contract.sh can
hang indefinitely because urllib.request.urlopen is called without a timeout.
Update the request in the Weblate JSON loading block to pass an explicit timeout
value through urlopen so the workflow fails fast if PyPI is unresponsive. Keep
the change localized to the urllib.request.urlopen/json.load flow used for the
Weblate package lookup.
In `@tests/test_weblate_internal_contract.py`:
- Around line 60-71: The contract check in weblate_formats_with_plugin_formats()
is validating tuple emptiness before confirming the value is actually a tuple,
which can mask type-contract failures with an “empty tuple” message. In the
test_weblate_internal_contract assertion block, move the isinstance(formats,
tuple) check ahead of the if not formats check so non-tuple or None values fail
with the correct type diagnostic first, then keep the empty-tuple assertion
afterward.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e9d38824-b113-46ec-978a-27ef438dc80f
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
.github/WORKFLOWS.md.github/workflows/release.yml.github/workflows/weblate-pin-bump.ymlCHANGELOG.mdpyproject.tomlscripts/bump-weblate-version.shscripts/check-weblate-internal-contract.shscripts/check-weblate-pin-sync.shscripts/weblate-version-map.shtests/test_weblate_internal_contract.py
Close #130, close #132.
Summary by CodeRabbit
pyproject.tomldependency formats.Weblate[postgres]==<version>pin format.Weblate[all]toWeblate[postgres]and updated the changelog/workflows to match.