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

Move pytest to pyproject.toml and other small changes #535

Merged
merged 11 commits into from
Feb 4, 2023
Merged

Move pytest to pyproject.toml and other small changes #535

merged 11 commits into from
Feb 4, 2023

Conversation

nkiryanov
Copy link
Contributor

  1. Настройки pytest перевезли в общий pyproject.toml
  2. Мелкие обновления таргетов в Makefile:
    1. deps и dev-deps: зависимости собираются с -resolver=backtracking как рекомендует pip-tools
    2. Новый таргет: fmt-check — проверяет сортировку импортов и форматирование black. Ну чтоб в lint всё не собирать
    3. Таргет lint:
      1. Зависимость от fmt-check: что-то у меня с сортировкой импортов было не ок, но ошибок не было. Теперь будут
      2. Проверяем django-проект манаджмент командой check для "inspect the entire Django project for common problems"
    4. Таргет test
      1. Подправил создание папки для статики
      2. Проверяем отсутствие миграций для изменений в моделях
  3. Зависимости:
    1. Обновлены до последних совместимых версий
    2. Удалил дев-зависимости flake-aaa и flake8-multiline-containers: первую не используем, вторая конфликтует с black
    3. Добавил зависимость psycopg2-binary: как минимум в ci запускаем тесты с postgres
  4. Обновлён BaseService: теперь возвращает Any
  5. Обновлён github.ci
    1. Версию python берём из .python-version файла
    2. Кэширование с github.ci через экшн cache: так можем явно указать какие папки кэшировать (нам нужен venv).
  6. Обновлён Dockerfile: версию python нужно передавать в build-args. Так удобнее.
  7. В README подправил, что документация теперь через drf-spectacular

1. Drop pytest.ini and move settings to toml
2. Drop redundant `flake-aaa` and `flake8-multiline-containers`
1. Check `isort` and `black` format on lint make target
2. Check created migrations on `test` make target
3. Check project on `lint` target
4. Deps with `resolver=backtracking` as pip-tools recommendation
1. Add `psycopg2-binary` cause CI test assumes postgres as DB
2. Update all existed deps to the latest supported versions
1. Use python version from `.python-version` file
2. Add `PYTHON_VERSION` dockerfile var arg
@nkiryanov
Copy link
Contributor Author

@f213 посмотри 🙏
Чёт описание большое, но изменения мелкие.

Проверил что в таком виде проект собирается и нормально github ci проходит.

cookiecutter.json Outdated Show resolved Hide resolved
hooks/post_gen_project.sh Outdated Show resolved Hide resolved
@@ -5,22 +5,28 @@ install-deps: deps
pip-sync requirements.txt

deps:
pip-compile --output-file=requirements.txt pyproject.toml
pip-compile --resolver=backtracking --output-file=requirements.txt pyproject.toml
Copy link
Member

Choose a reason for hiding this comment

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

На всякий случай оставлю это здесь: я поковырял возможность конфигурировать pip-tools через pyproject.toml и нашёл jazzband/pip-tools#604, в которой пока ничего не сделали. Надеюсь, сделают :-)

{{cookiecutter.project_slug}}/pyproject.toml Show resolved Hide resolved
@nkiryanov
Copy link
Contributor Author

@f213 подправил замечания + обновил описание PR. Если всё норм, можно мерджить.

Что поменялось:

  • убрал таргет fmt-check в Makefile — вообще лишний
  • убрал Circle CI stub
  • добавил пункт про postgres в README
  • подправил переменную USE_I18N (правка из PR Update i18n.py #534)

@kazqvaizer
Copy link
Contributor

@nkiryanov а не хочешь найти тулзу, которая выпиливает неиспользованные импорты? Я тут понял, что сейчас за меня это делает pycharm, а хотелось бы, чтобы форматер

@nkiryanov
Copy link
Contributor Author

@nkiryanov а не хочешь найти тулзу, которая выпиливает неиспользованные импорты? Я тут понял, что сейчас за меня это делает pycharm, а хотелось бы, чтобы форматер

О, этого отличная идея. Посмотрю.
Только не хочу в этот PR затягивать, т.к. много всего получается.

@nkiryanov
Copy link
Contributor Author

@f213 если что не могу сам смерджить: нет прав.

@f213
Copy link
Member

f213 commented Feb 4, 2023

А теперь?

@nkiryanov nkiryanov merged commit ff6cacd into fandsdev:master Feb 4, 2023
This was referenced Feb 4, 2023
@@ -5,4 +5,4 @@

class AppPagination(PageNumberPagination):
page_size_query_param = "page_size"
max_page_size = settings.MAX_PAGE_SIZE
max_page_size = settings.MAX_PAGE_SIZE # type: ignore
Copy link
Contributor

@nvo87 nvo87 Feb 8, 2023

Choose a reason for hiding this comment

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

@nkiryanov @f213 @kazqvaizer вот такие тайп-игноры придется теперь ставить везде где есть settings.
Это происходит, т.к. в django-stubs есть какой-то баг. Последний коммент верный. Чтобы этого не было - надо ограничивать версию django-stubs до ==1.12.0.

По мере роста проекта писать везде тайп-игнор напрягает. Но и стопорить версию по идее не правильно. К тому же она за собой стопорит версию drf-stubs, т.к. требует django-stubs>=1.13.0

Что лучше, ставим в новых проектах везде тайп-игнор пока не пофиксят?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ну я бы попридержал версию django-stubs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Думал об этом и склонялся к другому варианту: лучше type: ignore для настроек.

Почему:

  1. В стабах тоже фиксят баги. Так у нас будет type: ignore для одного конкретного сценария, а если не обновлять то могут быть type: ignore в соврешенно разных местах (сталкивался с багом с тайпингом менеджера кверисета который появлялся с обновлением django и который чинился обновлением стабов)
  2. Настройки не так часто вызываем и игноров будет не оч. много. А если пофиксят, нам mypy сообщит, что больше не нужны
  3. Вроде новый LTS релиз не за горами и для него будет нужна новая версия стабов. Всё равно столкнёмся (если не пофиксят)

Ну, а если долго не будут фиксить, то подумать что можно сделать с настройками.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants