search

chernovk / practix_auth

0 stars 1 forks source link

Code review #38

Open BigDeepBlue opened 2 years ago

BigDeepBlue commented 2 years ago

Сильно :) Работа очень хорошая

  1. Все еще читабельнее использовать http.HTTPStatus вместо явного указания кодов.
  2. Местами в коде есть неиспользуемые импорты и импорты сгруппированы не по pep8. Попробуйте интересный инструмент pre-commit, туда же можно прикрутить и isort(https://github.com/PyCQA/isort) и black, и при каждом коммите он будет выполнять всю эту рутину по форматированию и прочему.
  3. Не нашел в FastApi репозитории реализации изящной деградации, выдача разного контента в зависимости от статуса пользователя. Например, авторизованному - все, неавторизованному - только фильмы старше 10 лет. Может и пропустил: тогда прошу описать.
  4. И у вас не очень с типизацией, где-то есть, где-то нет, это создает определенные сложности при чтении кода. Мелочь конечно, но имейте в виду.
ghost commented 2 years ago

Из-за довольно ограниченного (на данный момент) функционала нашего онлайн кинотеатра, не смогли никаких нормальных ограничений для деградации сервисов придумать. Решили сделать так, чтобы точки, в которых предусмотрена деградация, были помечены флагом degrading, который ставится в True при помощи декоратора, который проверяет права обращающегося юзера. Для примера, установили деградацию для сервиса листинга фильмов, который, в случае отказа от сервиса авторизации, выдает только фильмы с рейтингом ниже 7. Остальное подправили

BigDeepBlue commented 2 years ago

LGTM