Gaidunov / team_Auth_sprint_1

1 stars 0 forks source link

Code review #28

Closed BigDeepBlue closed 2 years ago

BigDeepBlue commented 2 years ago
  1. Не получилось запустить проект. Т.к. в docker-compose.yaml нет нужных сервисов (postgers, nginx).
  2. Сервис авторизации как и остальные сервисы проекта, взаимодействующие с пользователем, должен отдаваться через nginx. Это постоянное правило для высоконагруженных проектов.
  3. .env из корня проекта нужно указать в .gitignore
  4. Соберите все настройки в одно месте в src/config.py, сейчас у вас в src/app.py встречаются os.environ
  5. Вот такой способ инициализации и обновления таблиц БД не самый лучший, а в реальных проектах - недопустимый. Только именованные миграции. Попробуйте https://alembic.sqlalchemy.org/en/latest/, к тому же, как я вижу, шаги в этом направлении у вас уже сделаны в виде файла alembic.ini, явно пробовали сделать, но отказались.
  6. Лишние скобки.
  7. Какое назначение у файла src/api/v1/movies.py?
  8. Судя по всему у вас управление ролями доступно всем. Нужно предусмотреть создание суперпользователя и разрешать это только ему. Flask предоставляет механизм создания консольных команд: https://flask.palletsprojects.com/en/2.1.x/cli/#custom-commands
  9. А для проверки прав пользователя при управлении ролями можно попробовать вот это https://flask-jwt-extended.readthedocs.io/en/stable/custom_decorators/
  10. Любое API нужно документировать. Обратите внимание на https://github.com/flasgger/flasgger
  11. Вот такие были требования к работе:

    • Все ключевые методы покрыты тестами: каждый ответ каждой ручки API и важная бизнес-логика тщательно проверены;
    • У тестов есть понятное описание, что именно проверяется внутри. Используйте pep257{target="_blank"};
    • Заполните README.md так, чтобы по нему можно было легко познакомиться с вашим проектом. Добавьте короткое, но ёмкое описание проекта. По пунктам опишите как запустить приложения с нуля, перечислив полезные команды. Упомяните людей, которые занимаются проектом и их роли. Ведите changelog: описывайте, что именно из задания модуля уже реализовано в вашем сервисе и пополняйте список по мере развития.
Gaidunov commented 2 years ago

Семен: 1, 2 Вася: 3, 5, 8, 10 Кирилл: 4, 7, 9

BigDeepBlue commented 2 years ago

Папка migrations есть, а самих миграций нет 😟

BigDeepBlue commented 2 years ago

LGTM