ci: harden client regeneration#108
Conversation
| python -m pip install dist/*.whl | ||
| # cd away from the source tree so the import resolves against the installed wheel. | ||
| cd /tmp && python -c "import hotdata; print(hotdata.__version__)" | ||
| # NOTE: regeneration deliberately does NOT build/import the package. A |
There was a problem hiding this comment.
super nit: the rationale is sound, but note one small coverage gap (not blocking): the old step ran python -m build + installed the wheel, whereas integration-tests.yml only does pip install -e . (editable). So a packaging/wheel-build regression (sdist/MANIFEST, build-backend config) now surfaces only at publish time, not on the regen PR. Low risk here since pyproject.toml is hand-maintained and ignored by the generator, so regen rarely touches packaging — just worth being aware of.
There was a problem hiding this comment.
Verified the three changes against the surrounding workflows:
- Clean step:
hotdata/api,hotdata/models,docs,testare the only generator subtrees with per-entity orphan accumulation; root-level generated modules are fixed-name (overwritten in place). Confirmed none of the cleaned paths appear in.openapi-generator-ignore, so nothing removed is left un-regenerated.tests/and the hand-written root modules are correctly untouched. - CHANGELOG seeding: insertion anchors correctly before the first released section, escapes the version in the regex, is idempotent, and produces a section that satisfies
scripts/check-release.py's## [x.y.z]match. - Dropping the wheel build/import gate:
integration-tests.ymlrunspip install -e .+ pytest on every PR to main (secrets are injected since the regen branch is in-repo), so import/build breakage surfaces as red CI. Left one super-nit on the wheel-packaging coverage gap.
No blocking issues.
|
Addressed the packaging-coverage nit in 9539d80: moved the |
| # This job runs on any PR touching pyproject.toml/CHANGELOG.md — which every | ||
| # regen PR does (version bump + seeded changelog) — so a packaging or import | ||
| # regression surfaces as red CI on the PR instead of silently aborting the | ||
| # regen before it can open one. (Step name kept stable to preserve the |
There was a problem hiding this comment.
super nit: the parenthetical "Step name kept stable to preserve the required-check / branch-protection wiring" is misleading (not blocking). Required status checks key off the job name (check / "Verify changelog matches version bump"), not step names — and this step's name actually changed from the old regen step ("Verify built wheel installs and imports" → "Build, install, and import the wheel"). So nothing was kept stable here, and step naming has no bearing on branch protection. Consider dropping that clause so a future maintainer doesn't preserve the step name thinking it matters for required checks.
There was a problem hiding this comment.
Verified the clean step targets only generator-owned subtrees (hand-written modules and the tests/ integration suite are spared), the changelog seed logic is correct and idempotent, and the wheel build moved to check-release.yml closes the publish-time coverage gap raised in the prior thread. One non-blocking super-nit on a misleading code comment. LGTM.
Brings the python regen workflow in line with sdk-rust (#49, #51) and fixes a clean-step bug that left removed endpoints shipping as dead code.
hotdata/, but the clean step wasrm -rf src/— a no-op. So endpoints/models dropped from the spec were never deleted: the sandbox APIs (removed from the spec in www#221) still linger onmainas 14 orphaned generated files even after the indexes regen (feat(api): add list-indexes endpoint, exclude internal migrate-backend #107). Now cleans the generator-owned subtrees (hotdata/api,hotdata/models,docs,test); hand-written modules (hotdata/_auth.py,hotdata/arrow.py) and thetests/integration suite are untouched.check-releaserequires a## [x.y.z]section matching the bumped version, so every regen PR failed it (e.g. chore(api): clean up workspace spec, remove sandbox APIs #106). Inserts a stub (the spec-change title under### Changed) above the latest release; idempotent; the author refines wording before merge.Create PR, so a broken regen failed silently with no PR.integration-tests.ymlalready installs the package and runs pytest on every PR to main, so breakage now surfaces as red CI on the filed PR (and auto-merge, gated on those checks, won't merge it).The JWT-exchange regen-safety AST guard stays. After merge, the next regen will also drop the lingering orphaned sandbox files.