AndreiUkladchikov / YandexPracticumTeam

0 stars 1 forks source link

Code review #164

Open BigDeepBlue opened 1 year ago

BigDeepBlue commented 1 year ago

Добрый день.

Отличный у вас диплом получился!!! Видно, что был проделан большой объем работы. У меня совсем немного замечаний.

  1. Запуск приложения лучше делать непосредственно в описании Dockerfile через ENTRYPOINT или CMD, а в docker-compose описывать разворачивание сервисов. Так контейнер уже получается самостоятельной боевой единицей, если его просто поднять и добавить переменные окружения.
  2. В docker-compose.yml много лишних портов выставлено наружу, по идее должно быть достаточно только nginx. Для безопасности приложения это плохо.
  3. [Можно лучше] Джанго админку лучше отдавать не по /admin, а по какому нибудь неочевидному слову. Например /promo_my_admin. Так можно немного защититься от роботов, сканеров которые по ключевым словам перебирают url и обычно они всегда смотрят /admin в поисках админки чтобы ее взломать :)
  4. [Можно лучше] докстринги к функциям в исходном коде лучше все таки стараться полностью на английском оформлять
  5. Подумайте над тем, чтобы добавить пагинацию к ручке get_user_history. По идее в какой либо копящейся статистике она нужна.
  6. debug_toolbar лучше включать в INSTALLED_APPS и MIDDLEWARE только когда DEBUG=true
  7. Неплохо было бы добавить в nginx location регулярку по которой в джанго будут проходить запросы имеющие "admin", "/api/v1" , а все остальные по типу /ihackyoursserver отклонять на уровне nginx. Это также для безопасности и отказоустойчивости.
    location <здесьнапиширегулярку> {
    proxy_pass http://django_app;
    }
  8. Давайте добавим логов по коду уровня info, debug для того чтобы понимать, что происходит при работе вашего приложения в продакшене. Представьте, что вы свой проект выводите в production, и реальные клиенты массово начинают этим пользоваться. При первом релизе проекта очень удобно когда он покрыт логами info и debug для понимания того, что происходит. Практически всегда бывает, что разработчик не все случаи оттестировал, и когда реальные клиенты начинают пользоваться сервисом, что-то обязательно всплывает. Представим ситуацию. Клиент жалуется, что в период с 2 до 4 дня не мог применить свой промокод в сервисе. Разработчик фронтенда говорит, что проблема в бэкенде. Что будете делать при отсутствии логов? Скорее всего будете долго разбираться как такое может быть, и есть шанс что так и не поймете. Проблема повторится - бизнес пострадает еще раз, и доверие клиента снизится. А если у вас все тщательно покрыто логами, вы увидите, что с 2 до 4 дня фронтенд присылал promo_value с каким нибудь кривым символом из-за его ошибки в коде. Соответственно передадите проблему ему и устраните быстрее.
BigDeepBlue commented 1 year ago

В логгировании не стоит использовать f-string,.format() , а делать через printf-style примерно так - logging.info("message %s", param) printf-style строка не форматируется до самого последнего момента. То есть если использовать f-string, то все вычисления по формированию строки происходят сразу, а если использовать %s и параметры, то только непосредственно перед выводом данных. А если например level handler будет в конфиге поставлен в critical, то этот лог никуда не отправится, а значит опять же строка форматироваться не будет.

https://okomestudio.net/biboroku/2020/04/on-lazy-logging-evaluation/