Add scripts for OpenAPI generation and post-generation and more convenience - #100
Add scripts for OpenAPI generation and post-generation and more convenience#100antalszava wants to merge 7 commits into
Conversation
antalszava
changed the title
|
|
||
| def main() -> None: | ||
| client_file = PACKAGE_DIR / "client.py" | ||
| client_file.write_text( |
There was a problem hiding this comment.
write_text emits CRLF on Windows, so regen output there isn't byte-identical (and the staleness gate is Linux-only). Add newline="\n" to both calls.
| --overwrite | ||
| run: uv run --group regen python scripts/regenerate_models.py | ||
| - name: Check for uncommitted changes | ||
| if: runner.os == 'Linux' |
There was a problem hiding this comment.
Could run this check on macOS too - right now the mac/windows jobs only prove the script runs.
| post_hooks: | ||
| - "perl -pi -e 's/token: str\\K$/ = field(repr=False)/' client.py" | ||
| - "perl -0777 -pi -e '$y=(gmtime)[5]+1900;s/\\A(?!# SPDX-FileCopyrightText)/# SPDX-FileCopyrightText: $y IonQ, Inc.\\n# SPDX-License-Identifier: Apache-2.0\\n# \\@generated\\n\\n/' $(find . -name '*.py')" | ||
| - "python ../scripts/post_generate.py" |
hodgestar-ionq
left a comment
hodgestar-ionq
left a comment
There was a problem hiding this comment.
Looks good to me. Thank you for adding the Makefile. I left a couple of questions.
| enable-cache: ${{ github.event_name == 'push' }} | ||
| - run: uv sync | ||
| - run: uv run pytest ${{ matrix.python-version != '3.11' && '--no-cov' || '' }} | ||
| - run: uv run pytest ${{ (matrix.python-version != '3.11' || matrix.os != 'ubuntu-latest') && '--no-cov' || '' }} |
There was a problem hiding this comment.
Ideally we'd want to run coverage on all targets, and then combine the output so that we can see coverage of target-specific branches. No for this PR, of course, just a big picture nice to have.
| --overwrite | ||
| run: uv run --group regen python scripts/regenerate_models.py | ||
| - name: Check for uncommitted changes | ||
| if: runner.os == 'Linux' |
There was a problem hiding this comment.
I'm wondering why this isn't matrix.os like elsewhere? Is the intention here to only fail one version of the check but test that generation works on all platforms? If so, I feel it's worth a comment.
| ) | ||
|
|
||
| year = datetime.datetime.now(datetime.UTC).year | ||
| header = f"# SPDX-FileCopyrightText: {year} IonQ, Inc.\n# SPDX-License-Identifier: Apache-2.0\n# @generated\n\n" |
There was a problem hiding this comment.
| header = f"# SPDX-FileCopyrightText: {year} IonQ, Inc.\n# SPDX-License-Identifier: Apache-2.0\n# @generated\n\n" | |
| header = f"# SPDX-FileCopyrightText: YYYY-{year} IonQ, Inc.\n# SPDX-License-Identifier: Apache-2.0\n# @generated\n\n" |
Should we include the year the generated file was first published in the copyright? Not sure which year that would be, so I just put YYYY in the suggestion.
3 participants
Summary
scripts/regenerate_models.pyas the canonical, OS-independent way to regenerate the client: stdlib-only (urllib,shutil,tempfile,subprocess), appliesopenapi-overlay.yamlwhen present, and supports--sync-specto fetch the latest upstream spec first.post_hooksinopenapi-python-client-config.yamlwithscripts/post_generate.py(same behavior: SPDX/@generatedheaders,tokenhidden fromrepr) — the perl quoting was Unix-only and broke generation on Windows.Makefilewith shorthand targets (regen,sync-spec,check-generated,lint,test, ...) that wrap the script and the existinguv runcommands.generated.ymlnow runs the script on a Linux/macOS/Windows matrix (staleness gate on Linux), so the regeneration command is defined in one place and exercised on all supported platforms.ci.ymltest matrix gains macOS and Windows entries (Python 3.14,--no-cov).CONTRIBUTING.mdto document the script as the regeneration workflow.Test plan
uv run --group regen python scripts/regenerate_models.pyreproduces the committed generated code with zero diff (i.e. the Python post-hooks are byte-identical to the perl ones).ruff check,ruff format --check, andty checkclean.generated.ymlstaleness jobs pass on ubuntu, macos, and windows on this PR.Note: Used the help of Claude Code for this PR. 🤖