search

SmirnovaT / Auth_sprint_2

Спринт 7
0 stars 0 forks source link

Code review #34

Open BigDeepBlue opened 1 week ago

BigDeepBlue commented 1 week ago

Правильные подходы 👍 Немного толстоваты эндпоинты (логику можно в сервисный слой отправить), но это мелочи и допустимо, если в команде принято такое решение.

  1. Давайте трейсер занесем под if settings.enable_tracer, чтобы была возможность его отключить в тех же тестах
  2. Имена провайдеров (yandex, google и т.д.) лучше поместить в enum. Работа с литералами - всегда не очень хорошая практика. Далее с развитием проекта может возникнуть необходимость их использовать в коде. Работа с enum, как минимум, избавит нас от опасности опечатки.
  3. [можно лучше] Вот тут и в следующем эндпоинте я бы попробовал "Абстрактную фабрику" или еще какой-нибудь паттерн.
  4. Вот это синхронная функция, и в ней идет синхронная работа с Redis. Давайте ее сделаем асинхронной (с редисом работа в асинхронном режиме и т.д., он это умеет) или отправим в тредпул https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.run_in_executor
  5. Этот метод бросается исключениями если не может достучаться до хоста назначения. Исключение нужно поймать и обработать.
BigDeepBlue commented 1 week ago

LGTM