Skip to content

Commit 7539cdd

Browse files
committed
ci: Harden the nightly model regeneration after review
1 parent c854c4b commit 7539cdd

5 files changed

Lines changed: 183 additions & 30 deletions

File tree

.github/workflows/on_schedule_regenerate_models.yaml

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ env:
3232
# A valid Conventional Commits title, so the pull request is mergeable without anyone having to rename it.
3333
# Reviewers retitle it to `fix:`/`feat:` when the diff is user-facing - see the pull request body.
3434
PR_TITLE: "chore: Regenerate models from the published OpenAPI spec"
35+
ASSIGNEE: vdusek
3536
LABEL: t-tooling
3637

3738
jobs:
@@ -83,13 +84,25 @@ jobs:
8384
- name: Check whether the models are already up for review
8485
id: review
8586
if: steps.changes.outputs.has-changes == 'true'
87+
env:
88+
GH_TOKEN: ${{ secrets.APIFY_SERVICE_ACCOUNT_GITHUB_TOKEN }}
8689
run: |
8790
if ! git ls-remote --exit-code --heads origin "$BRANCH_NAME" >/dev/null 2>&1; then
8891
echo "No auto-update branch exists yet."
8992
echo "is-new=true" >> "$GITHUB_OUTPUT"
9093
exit 0
9194
fi
9295
96+
# The branch only counts as "already up for review" while a pull request is actually open on it. A
97+
# leftover branch - an earlier run that failed before opening one, or a review that closed the pull
98+
# request without deleting the branch - must be rebuilt, otherwise its contents would look proposed
99+
# and silently suppress every future regeneration.
100+
if [[ -z "$(gh pr list --head "$BRANCH_NAME" --base master --state open --json number --jq '.[0].number // empty')" ]]; then
101+
echo "Branch $BRANCH_NAME has no open pull request - rebuilding it."
102+
echo "is-new=true" >> "$GITHUB_OUTPUT"
103+
exit 0
104+
fi
105+
93106
git fetch origin "$BRANCH_NAME"
94107
if git diff --quiet FETCH_HEAD -- src/apify_client/_models.py src/apify_client/_typeddicts.py src/apify_client/_literals.py; then
95108
echo "The open pull request already carries these models - nothing to do."
@@ -156,4 +169,55 @@ jobs:
156169
--body "$BODY" \
157170
--base master \
158171
--head "$BRANCH_NAME" \
172+
--assignee "$ASSIGNEE" \
159173
--label "$LABEL"
174+
175+
# Alert the team when a scheduled run fails. Without this a broken sync stops model regeneration silently:
176+
# for a scheduled workflow GitHub only notifies whoever last touched the cron, and this workflow is not part
177+
# of the checks that releases wait on. Skipped on manual dispatch so ad-hoc triggers don't spam the channel.
178+
notify_on_failure:
179+
name: Notify Slack on failure
180+
needs: regenerate-models
181+
if: failure() && github.event_name == 'schedule'
182+
runs-on: ubuntu-latest
183+
permissions:
184+
contents: read
185+
186+
steps:
187+
- name: Build Slack payload
188+
env:
189+
REPO: ${{ github.repository }}
190+
WORKFLOW_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
191+
HEADING: ":red_circle: Nightly model regeneration failed"
192+
run: |
193+
jq -n \
194+
--arg repo "${REPO}" \
195+
--arg url "${WORKFLOW_URL}" \
196+
--arg heading "${HEADING}" \
197+
'{
198+
text: "\($heading) in \($repo)",
199+
blocks: [
200+
{
201+
type: "header",
202+
text: { type: "plain_text", text: $heading, emoji: true }
203+
},
204+
{
205+
type: "section",
206+
fields: [
207+
{ type: "mrkdwn", text: "*Repository:*\n\($repo)" },
208+
{ type: "mrkdwn", text: "*Workflow run:*\n<\($url)|View on GitHub>" }
209+
]
210+
},
211+
{
212+
type: "section",
213+
text: { type: "mrkdwn", text: "The generated API models are no longer being kept in sync with the published OpenAPI specification." }
214+
}
215+
]
216+
}' > slack-payload.json
217+
218+
- name: Send Slack notification
219+
uses: slackapi/slack-github-action@v4.0.0
220+
with:
221+
webhook: ${{ secrets.SLACK_WEBHOOK_URL }}
222+
webhook-type: incoming-webhook
223+
payload-file-path: slack-payload.json

.rules.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ Each input-side TypedDict ships in two casings: snake_case (`RequestDict`) and c
8383
- Generated by `datamodel-code-generator` from `spec/openapi.json` (config in `pyproject.toml` under `[tool.datamodel-codegen]`, aliases in `datamodel_codegen_aliases.json`)
8484
- After generation, `scripts/postprocess_generated_models.py` is run to apply additional fixes
8585
- To regenerate locally: `uv run poe update-spec` to refresh the snapshot, then `uv run poe generate-models`. To try a candidate spec, overwrite `spec/openapi.json` and regenerate
86-
- `uv run poe check-models` (part of `check-code`, and the `Models check` CI job) regenerates and fails on any diff. So a `datamodel-code-generator` or `ruff` upgrade that changes codegen output turns **its own** dependency-bump PR red — that PR carries the regenerated files, instead of the drift contaminating an unrelated spec change later
86+
- `uv run poe check-models` (part of `check-code`, and the `Models check` CI job) regenerates, compares, and restores the working tree — it never leaves the generated files modified, and it doesn't care whether they're committed. So a `datamodel-code-generator` or `ruff` upgrade that changes codegen output turns **its own** dependency-bump PR red — that PR carries the regenerated files, instead of the drift contaminating an unrelated spec change later
8787
- In CI, the `Regenerate models` workflow (`on_schedule_regenerate_models.yaml`) runs nightly at 02:00 UTC: it refreshes the snapshot, regenerates, and opens a PR when the models change. It always generates on master and rebuilds its `ci/regenerate-models` branch from master instead of appending, so the PR diff is always "current spec vs current master". The PR opens with a mergeable `chore:` title — retitle it to `fix:`/`feat:` when the diff is user-facing, so it lands in the changelog and triggers a release
8888
- The gate is the **generated models**, not the spec: `info.version` in the published spec carries a build timestamp that apify-docs bumps on every spec change, so the snapshot only moves together with the models it produces
8989
- Manual regeneration is also possible from the GitHub Actions UI (`Regenerate models` workflow)

pyproject.toml

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,8 @@ integration-tests-cov = "uv run pytest --numprocesses=${TESTS_CONCURRENCY:-auto}
279279
check-docstrings = "uv run python -m scripts.check_docstrings"
280280
fix-docstrings = "uv run python -m scripts.fix_docstrings"
281281
update-spec = "uv run python -m scripts.update_openapi_spec"
282+
# Regenerates to compare, then restores the working tree - running it never modifies the generated files.
283+
check-models = "uv run python -m scripts.check_generated_models"
282284
check-code = ["lint", "type-check", "check-docstrings", "check-models", "unit-tests"]
283285

284286
[tool.poe.tasks.install-dev]
@@ -317,14 +319,3 @@ uv run datamodel-codegen --input spec/openapi.json \
317319
--no-use-closed-typed-dict \
318320
&& python scripts/postprocess_generated_models.py
319321
"""
320-
321-
# Fails when the generated files don't match what the committed spec and the pinned tooling produce - which
322-
# catches both a hand-edited generated file and a codegen/formatter upgrade whose output nobody regenerated.
323-
[tool.poe.tasks.check-models]
324-
shell = """
325-
uv run poe generate-models \
326-
&& git diff --exit-code -- \
327-
src/apify_client/_models.py \
328-
src/apify_client/_typeddicts.py \
329-
src/apify_client/_literals.py
330-
"""

scripts/check_generated_models.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
"""Check that the generated models match what the committed OpenAPI snapshot produces.
2+
3+
Regenerating is the only way to know, so this runs codegen and then puts the working tree back exactly as it
4+
was - a check must not rewrite tracked source files, and it must not care whether they happen to be committed.
5+
That keeps `poe check-code` safe to run mid-change, and keeps this independent of git state.
6+
7+
A failure means the generated files no longer follow from `spec/openapi.json` plus the pinned tooling: either
8+
someone edited them by hand, or a codegen/formatter upgrade changed the output. Both are fixed the same way -
9+
run `poe generate-models` and commit the result.
10+
"""
11+
12+
from __future__ import annotations
13+
14+
import difflib
15+
import subprocess
16+
import sys
17+
from pathlib import Path
18+
19+
REPO_ROOT = Path(__file__).resolve().parent.parent
20+
21+
GENERATED_FILES = (
22+
Path('src/apify_client/_models.py'),
23+
Path('src/apify_client/_typeddicts.py'),
24+
Path('src/apify_client/_literals.py'),
25+
)
26+
27+
28+
def main() -> None:
29+
committed = {path: (REPO_ROOT / path).read_text(encoding='utf-8') for path in GENERATED_FILES}
30+
drifted: list[Path] = []
31+
32+
try:
33+
subprocess.run(['uv', 'run', 'poe', 'generate-models'], check=True, cwd=REPO_ROOT) # noqa: S607
34+
35+
for path in GENERATED_FILES:
36+
regenerated = (REPO_ROOT / path).read_text(encoding='utf-8')
37+
if regenerated == committed[path]:
38+
continue
39+
40+
drifted.append(path)
41+
sys.stdout.writelines(
42+
difflib.unified_diff(
43+
committed[path].splitlines(keepends=True),
44+
regenerated.splitlines(keepends=True),
45+
fromfile=f'{path} (on disk)',
46+
tofile=f'{path} (regenerated)',
47+
)
48+
)
49+
finally:
50+
for path, content in committed.items():
51+
(REPO_ROOT / path).write_text(content, encoding='utf-8', newline='\n')
52+
53+
if drifted:
54+
print(
55+
f'\n{len(drifted)} generated file(s) do not match `spec/openapi.json`: '
56+
f'{", ".join(str(path) for path in drifted)}.\n'
57+
'Run `uv run poe generate-models` and commit the result.',
58+
file=sys.stderr,
59+
)
60+
sys.exit(1)
61+
62+
print('Generated models match the committed OpenAPI specification snapshot.')
63+
64+
65+
if __name__ == '__main__':
66+
main()

scripts/update_openapi_spec.py

Lines changed: 50 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,61 +12,93 @@
1212
from __future__ import annotations
1313

1414
import json
15-
import os
1615
import sys
16+
import time
17+
from pathlib import Path
1718

1819
import impit
1920

21+
REPO_ROOT = Path(__file__).resolve().parent.parent
22+
2023
# The published, bundled specification. It is built and deployed from the `apify/apify-docs` repository.
2124
SPEC_URL = 'https://docs.apify.com/api/openapi.json'
2225

23-
# Path of the committed snapshot, relative to the repository root.
24-
SPEC_PATH = os.path.join('spec', 'openapi.json')
26+
SPEC_PATH = REPO_ROOT / 'spec' / 'openapi.json'
2527

2628
# A truncated response or an error page must never overwrite the snapshot. The real specification is roughly
2729
# 1 MB, so anything remotely this small is broken.
2830
MIN_SPEC_SIZE_BYTES = 100_000
2931

30-
# Top-level members every specification we can generate models from has to contain.
31-
REQUIRED_SPEC_KEYS = ('openapi', 'paths', 'components')
32+
# Top-level members every specification we can generate models from has to contain. `info` is included because
33+
# the version stamp reported below is read from it.
34+
REQUIRED_SPEC_KEYS = ('openapi', 'info', 'paths', 'components')
3235

3336
REQUEST_TIMEOUT_SECS = 60
3437

38+
# The nightly workflow alerts the team when this fails, so a single network blip shouldn't be worth a ping.
39+
DOWNLOAD_ATTEMPTS = 3
40+
RETRY_DELAY_SECS = 5
41+
42+
43+
def download_spec() -> bytes:
44+
"""Download the published specification, retrying transient failures."""
45+
last_error = ''
3546

36-
def main() -> None:
3747
with impit.Client(follow_redirects=True) as client:
38-
response = client.request('GET', SPEC_URL, timeout=REQUEST_TIMEOUT_SECS)
48+
for attempt in range(1, DOWNLOAD_ATTEMPTS + 1):
49+
try:
50+
response = client.request('GET', SPEC_URL, timeout=REQUEST_TIMEOUT_SECS)
51+
except Exception as exc:
52+
last_error = f'{type(exc).__name__}: {exc}'
53+
else:
54+
if response.status_code == 200:
55+
return response.content
56+
last_error = f'HTTP {response.status_code}'
3957

40-
if response.status_code != 200:
41-
print(f'Failed to download {SPEC_URL}: HTTP {response.status_code}.')
42-
sys.exit(1)
58+
print(f'Attempt {attempt}/{DOWNLOAD_ATTEMPTS} to download {SPEC_URL} failed ({last_error}).')
59+
if attempt < DOWNLOAD_ATTEMPTS:
60+
time.sleep(RETRY_DELAY_SECS)
61+
62+
print(f'Failed to download {SPEC_URL} after {DOWNLOAD_ATTEMPTS} attempts: {last_error}.', file=sys.stderr)
63+
sys.exit(1)
64+
65+
66+
def main() -> None:
67+
payload = download_spec()
4368

44-
payload = response.content
4569
if len(payload) < MIN_SPEC_SIZE_BYTES:
46-
print(f'Downloaded specification is only {len(payload)} bytes, which cannot be the real one - aborting.')
70+
print(
71+
f'Downloaded specification is only {len(payload)} bytes, which cannot be the real one - aborting.',
72+
file=sys.stderr,
73+
)
4774
sys.exit(1)
4875

4976
try:
5077
spec = json.loads(payload)
5178
except json.JSONDecodeError as exc:
52-
print(f'Downloaded specification is not valid JSON: {exc}.')
79+
print(f'Downloaded specification is not valid JSON: {exc}.', file=sys.stderr)
5380
sys.exit(1)
5481

5582
missing_keys = [key for key in REQUIRED_SPEC_KEYS if key not in spec]
5683
if missing_keys:
57-
print(f'Downloaded specification is missing top-level {", ".join(missing_keys)} - aborting.')
84+
print(f'Downloaded specification is missing top-level {", ".join(missing_keys)} - aborting.', file=sys.stderr)
85+
sys.exit(1)
86+
87+
# Read the version stamp before writing anything, so a malformed `info` aborts with the snapshot intact.
88+
version = spec['info'].get('version')
89+
if not version:
90+
print('Downloaded specification has no `info.version` - aborting.', file=sys.stderr)
5891
sys.exit(1)
5992

6093
# Normalize the formatting so the committed diffs stay readable no matter how the published bundle is
6194
# formatted. Key order is deliberately preserved: `keep_model_order` makes the generated model order follow
6295
# the specification, so sorting keys here would reshuffle `_models.py` on the next run.
6396
normalized = json.dumps(spec, indent=2, ensure_ascii=False) + '\n'
6497

65-
os.makedirs(os.path.dirname(SPEC_PATH), exist_ok=True)
66-
with open(SPEC_PATH, 'w', encoding='utf-8') as spec_file:
67-
spec_file.write(normalized)
98+
SPEC_PATH.parent.mkdir(parents=True, exist_ok=True)
99+
SPEC_PATH.write_text(normalized, encoding='utf-8', newline='\n')
68100

69-
print(f'Wrote {SPEC_PATH} (version {spec["info"]["version"]}, {len(normalized.encode())} bytes).')
101+
print(f'Wrote {SPEC_PATH.relative_to(REPO_ROOT)} (version {version}, {len(normalized.encode())} bytes).')
70102

71103

72104
if __name__ == '__main__':

0 commit comments

Comments
 (0)