chore: add checks for schema changes#1992
Conversation
Signed-off-by: Luiz Georg <luiz.georg@profusion.mobi>
This removes code duplication and makes the CI checks consistent with the development environment. Signed-off-by: Luiz Georg <luiz.georg@profusion.mobi>
|
@mentonin are we dropping the removal of the schema.yml from versioning? |
|
how come the CI checks are empty? |
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | ||
| - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0 |
There was a problem hiding this comment.
wont we need to setup node (and pnpm) to be able to run the frontend linters?
There was a problem hiding this comment.
Nice catch. Pre-commit is able to setup isolated environments, including for node, but we are not using that for the frontend checks
| path: ./dashboard/node_modules | ||
|
|
||
| - name: Install pnpm | ||
| uses: pnpm/action-setup@v4 |
There was a problem hiding this comment.
might be needed on pre-commit run?
There was a problem hiding this comment.
we are using the old task name here
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | ||
| - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0 |
There was a problem hiding this comment.
might be better to reuse setup-backend
uses: ./.github/actions/setup-backend
There was a problem hiding this comment.
We don't need poetry and dependencies, as pre-commit should use its own environment for running tests
| CI: true | ||
|
|
||
| jobs: | ||
| pre-commit-checks: |
There was a problem hiding this comment.
might be worth to include timeout just in case
Signed-off-by: Luiz Georg <luiz.georg@profusion.mobi>
Signed-off-by: Luiz Georg <luiz.georg@profusion.mobi>
7ee4186 to
edd0c86
Compare
| - id: api-schema | ||
| name: API schema is up-to-date | ||
| entry: | | ||
| python -c ' | ||
| import os, subprocess, sys; | ||
| os.chdir("backend") | ||
| subprocess.run(["poetry", "install"], capture_output=True) | ||
| spectacular = subprocess.run(["poetry", "run", "python", "manage.py", "spectacular"], capture_output=True, text=True) | ||
| new_schema = spectacular.stdout | ||
| with open("schema.yml") as f: | ||
| old_schema = f.read() | ||
| if new_schema != old_schema: | ||
| print("Schema is not up-to-date. Run \"python manage.py spectacular --file schema.yml\" to regenerate it") | ||
| sys.exit(1) | ||
| ' | ||
| files: 'backend/(.*\.py|schema.yml)' | ||
| pass_filenames: false | ||
| language: python | ||
| language_version: '3.12' | ||
| additional_dependencies: [poetry] | ||
| stages: [pre-commit] |
There was a problem hiding this comment.
Any reason to use python+subprocess instead of sh?
| - id: api-schema | |
| name: API schema is up-to-date | |
| entry: | | |
| python -c ' | |
| import os, subprocess, sys; | |
| os.chdir("backend") | |
| subprocess.run(["poetry", "install"], capture_output=True) | |
| spectacular = subprocess.run(["poetry", "run", "python", "manage.py", "spectacular"], capture_output=True, text=True) | |
| new_schema = spectacular.stdout | |
| with open("schema.yml") as f: | |
| old_schema = f.read() | |
| if new_schema != old_schema: | |
| print("Schema is not up-to-date. Run \"python manage.py spectacular --file schema.yml\" to regenerate it") | |
| sys.exit(1) | |
| ' | |
| files: 'backend/(.*\.py|schema.yml)' | |
| pass_filenames: false | |
| language: python | |
| language_version: '3.12' | |
| additional_dependencies: [poetry] | |
| stages: [pre-commit] | |
| - id: api-schema | |
| name: API schema is up-to-date | |
| entry: | | |
| bash -c ' | |
| cd backend || exit 1 | |
| poetry install --quiet | |
| NEW_SCHEMA=$(poetry run python manage.py spectacular) | |
| OLD_SCHEMA=$(cat schema.yml 2>/dev/null) | |
| if [ "$NEW_SCHEMA" != "$OLD_SCHEMA" ]; then | |
| echo "Schema is not up-to-date. Run \"python manage.py spectacular --file schema.yml\" to regenerate it" | |
| exit 1 | |
| fi | |
| ' | |
| files: '^backend/(.*\.py|schema\.yml)$' | |
| pass_filenames: false | |
| language: system | |
| stages: [pre-commit] |
-- Assisted-by: Gemini
There was a problem hiding this comment.
It also warned about poetry install --quiet causing commits to become slow, but I doubt it's a problem
There was a problem hiding this comment.
Finally, it suggested using --validate --fail-on-warn instead of diffing the schema
Summary
This PR adds a pre-commit hook to check that
backend/schema.ymlis up-to-date withpython manage.py spectacular.It also adds all pre-commit checks to the CI workflow, replacing duplicated linting/format specifications.
Related Issues