Code review (sprint 9) - Githubissues

BigDeepBlue commented 1 year ago

Здравствуйте ! Хорошо поработали ! Есть несколько рекомендаций:

Тут Вы создали клиент для работы с монго. Очевидно нужно его закрыть здесь.
Здесь и по коду пересмотрите организацию импортов, она несколько отличается от рекомендаций PEP8 https://peps.python.org/pep-0008/#imports
Тут я бы советовал Вам использовать printf-style
```
logger.debug("some value = %s", value)
```
почитайте, в этом есть смысл в том, что если у вас допустим уровень логирования не debug - строка форматироваться не будет (не будет подставляться value в шаблон). Хоть минимальная оптимизация, но все же.
Тут на самом деле не очень хорошая практика передавать через query параметры данные для создания. Я бы рекомендовал все же передавать данные в теле (body) POST запроса.
Здесь пытаюсь понять заложенную логику. Т.е. тут в операторах сравнения Вы сравниваете передал ли пользователь оценку 10 - пишете в БД лайк, поставил 0 - категорический дизлайк ). А разве нельзя поставить фильму допустим оценку 5 или 8 ? Как считаете, может хранить рейтинги фильмов ? А лайки - дизлайки просто считать запросом ? Тогда средняя оценка фильма обретет смысл - у вас будут значения рейтинга по мнению пользователя.

Тут аналогично, советую оперировать с Body как то так:

film_id: str = Body(description=....),
user_id: str = Body(description=....),
text: str = Body(min_length=1, max_length=7000),
rating: Rating = Body(description="Like: 10, dislike: 0"),

Тут используйте вместо числовых кодов https://docs.python.org/3/library/http.html
Тут конфиг logstash который запустится отсюда. А кто будет слать ему что то ? В теории к спринту у Вас был пример с filebeat - который следит за логами (файликами) и шлет в logstash, а как у Вас, как и какой сервис вы логируете ? :) Хорошей практикой является разделить логи разных сервисов по разным индексам, например logstash.conf может выглядеть примерно как то так:
```
input {
beats {
    port => "5045"
    host => "0.0.0.0"
    tags => ["nginx"]
    codec => "json"
    ssl => false
  }

gelf {
type => "docker"
port => "5044"
}
}
output {
if "nginx" in [tags] {
  elasticsearch {
    hosts => '${ES_HOST}'
    index => "nginx-%{+YYYY.MM.dd}"
  }
} else if [tag] == "backend-app" {
  elasticsearch {
    hosts => '${ES_HOST}'
    index => "backend-app-%{+YYYY.MM}"
  }
} else if [tag] == "auth-service" {
    elasticsearch {
    hosts => '${ES_HOST}'
    index => "auth-service-%{+YYYY.MM}"
  }
}
stdout {
codec => rubydebug
}
}
```
посмотрите по gelf https://www.elastic.co/guide/en/logstash/current/plugins-inputs-gelf.html и тут статья https://ridwanfajar.medium.com/send-your-container-logs-to-elk-elasticsearch-logstash-and-kibana-with-gelf-driver-7995714fbbad получается Вы логи докер контейнеров так можете собрать. Ну либо пользуйтесь filebeat, в теории к спринту пример с nginx должен быть.

mikeGot commented 1 year ago

По-поводу 8 комментария:

https://github.com/AndreiUkladchikov/YandexPracticumTeam/blob/main/ugc_backend/core/custom_log.py В этом файле мы в Loguru добавляем возможность отправлять логи в logstash

AndreiUkladchikov commented 1 year ago

Поправили: https://github.com/AndreiUkladchikov/YandexPracticumTeam/pull/131 https://github.com/AndreiUkladchikov/YandexPracticumTeam/pull/133 https://github.com/AndreiUkladchikov/YandexPracticumTeam/pull/134

BigDeepBlue commented 1 year ago

@AndreiUkladchikov @mikeGot @rostsul @andrewqaa Молодцы ! Быстро поправили, принято.

AndreiUkladchikov / YandexPracticumTeam

Code review (sprint 9) #129