Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DI-2709] Adds CI worflow for the generated project #89

Merged
merged 4 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,9 @@ Run application:
```bash
make compose-up
```

We are using docker for running the app, so you don't have to keep your python env locally.
But we also use pre-commit hooks, and the `safety` hook requires `poetry` to be in your `PATH`.

> [!TIP]
> Ensure the poetry installed with the same python version as in `pyproject.toml`
bulya marked this conversation as resolved.
Show resolved Hide resolved
53 changes: 53 additions & 0 deletions {{ cookiecutter.project_slug }}/.github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
name: CI
on: [push]

concurrency:
group: ${{ '{{' }} github.workflow {{ '}}' }}-${{ '{{' }} github.ref {{ '}}' }}
cancel-in-progress: true


jobs:
linter:
runs-on: ubuntu-latest
steps:
- name: Checkout Code Repository
uses: actions/checkout@v4

- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: "{{ cookiecutter.python_version.lower() }}"

# We need `poetry` to run safety pre-commit hook
- name: Install Poetry
uses: snok/install-poetry@v1

- name: Run pre-commit
uses: pre-commit/[email protected]

# Tests & migrations checks
checks:
runs-on: ubuntu-latest
steps:

- name: Checkout Code Repository
uses: actions/checkout@v4

- name: Create .env file
run: |
cp ./api/.env.example ./api/.env

- name: Build the Stack
run: make compose-build

- name: Check missing DB migrations
run: make api-check-migrations

- name: Run DB Migrations
run: make api-migrate

- name: Run `api` Tests
run: make api-test

- name: Tear down the Stack
run: make compose-down
29 changes: 11 additions & 18 deletions {{ cookiecutter.project_slug }}/.pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ default_language_version:
repos:
# General
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.4.0
rev: v5.0.0
hooks:
- id: check-added-large-files
args: ['--maxkb=10000']
Expand All @@ -19,29 +19,22 @@ repos:
- id: pretty-format-json
args: ['--autofix', '--indent 2', ]
- repo: https://github.com/codespell-project/codespell
rev: v2.2.4
rev: v2.3.0
hooks:
- id: codespell
args: [ "--toml=api/pyproject.toml" ]
additional_dependencies:
- tomli

# API
- repo: https://github.com/charliermarsh/ruff-pre-commit
rev: v0.0.265
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.6.9
hooks:
- id: ruff
args: ['--fix', '--exit-non-zero-on-fix']
args: [ '--fix', '--exit-non-zero-on-fix' ]
- id: ruff-format
- repo: https://github.com/Lucas-C/pre-commit-hooks-safety
rev: v1.3.1
rev: v1.3.3
hooks:
- id: python-safety-dependencies-check
entry: safety
args: [check]
files: ./api/requirements-build.txt
# TODO: Run this hook from docker container.
# - repo: local
# hooks:
# - id: migrations-check
# language: system
# name: Check for uncreated migrations.
# entry: sh -c "make api-check-migrations"
# files: models/*.*\.py$
# stages: [commit]
Comment on lines -39 to -47
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will run this on CI, so I think it's OK to remove it from the pre-commit

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that pre-commit hook & CI are for the same purpose. In my point of view pre-commit hook is a helper for developer to fix and notify some stupid issues like formatting, unused imports & not created migrations.

Pre-commit hook is not required, while CI is. So, pre-commit hook helps to avoid unnecessary CI runs in case of some issues. It's better to get a notification about these issues when you make a commit instead of waiting for a CI run.

I would like to keep this hook.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexryabtsev
I don't know why it was "commented" before, I can uncomment it and see if it works fine with Docker, though. If not, I can keep it commented and we can look at this later in another task/PR.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know either why it's commented out.

@SergeyKubrak do you remember the reason why it was commented here?

@bulya Please try to uncomment and test it, if you have a capacity. If it won't work, let's try to fix it in another task. Thank you.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bulya It's not a strong requirement. You can remove checking migrations from pre-commit hooks, if it produces additional complexity in CI workflow. It's more important to have this check in CI rather than in pre-commit hooks.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am leaving this for visibility.
We have nevertheless decided to remove this from the pre-commit hooks, as we run all pre-commit hooks in the linter CI job, and we don't want either to run it there via the docker-compose or explicitly exclude it from the hooks list.

files: pyproject.toml
3 changes: 3 additions & 0 deletions {{ cookiecutter.project_slug }}/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,6 @@ compose-down:

compose-build:
cd docker/ && docker compose build

api-migrate:
cd docker/ && docker compose run --rm api make migrate
bulya marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 3 additions & 0 deletions {{ cookiecutter.project_slug }}/api/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -125,3 +125,6 @@ exclude_lines =[
]
show_missing = true

[tool.codespell]
skip = "*.lock"
ignore-words = "codespell-ignore-words.txt"