TRUEVORO / Auth_sprint_2

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

Code review #6

Open BigDeepBlue opened 1 year ago

BigDeepBlue commented 1 year ago

Обратите внимание на следующее:

  1. Давайте трейсер занесем под if settings.enable_tracer, чтобы была возможность его отключить в тех же тестах
  2. Вот сюда рекомендую добавить параметр resource, чтобы сервис был не unknown_service в интерфейсе JaegerUI:

      resource = Resource(attributes={
           SERVICE_NAME: 'auth-service'
      })
      provider = TracerProvider(resource=resource)
  3. Ограничение количества запросов сделано достаточно хорошо. Но, вдруг будет интересно, https://flask-limiter.readthedocs.io/en/stable/
  4. При применении миграций не создаются дополнительные таблицы. Дело в том, что Alembic этого не умеет и приходится редактировать миграции вручную.
  5. Необходимо подготовить код к расширению списка провайдеров авторизации. Сейчас вы очень жестко привязались к google. Попробуйте еще какой-нибудь добавить и увидите, что там будет очень много общего. Можно будет создать базовый класс с общим функционалом и от него наследоваться.
  6. В продолжение п.5 у вас нет модели для связи пользователя и соцсети, т.е. в архитектуру приложения даже не заложена возможность добавления других провайдеров авторизации, и пользователь не сможет добавить себе несколько соц. сетей.
  7. Одним из заданий было: Создайте интеграцию Auth-сервиса и AsyncAPI-сервиса, используя контракт, который вы сделали в прошлом задании. Т.е. добавить middleware, например, которая будет разбирать токен или ходить с ним в сервис авторизации и в зависимости от прав давать тот или иной контент. Например, авторизованным - все, гостям - только списком фильмы без подробностей. Укажите ссылку на сервис выдачи контента, где у вас это реализовано. Посмотрите очень подробный и интересный пример реализации https://testdriven.io/blog/fastapi-jwt-auth/ , самое интересное в разделе Securing Routes
BigDeepBlue commented 1 year ago

LGTM