bogatovad / notifications_sprint_1

Репозиторий для сервиса нотификаций
0 stars 0 forks source link

Code review #8

Open BigDeepBlue opened 1 year ago

BigDeepBlue commented 1 year ago

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

  1. Было бы очень хорошо нарисовать схему отношений компонентов сервиса, добавить чуть больше документации о том что было сделано как и почему.
  2. Тут я бы советовал отделить установку apt зависимостей куда то повыше, отдельно от pip. Чтоб не ждать каждый раз долгую установку apt при измнении чего то в requirements.
  3. Тут нужно добавить пробельчик.
  4. Тут можно попробовать новую фишку FastAPI - https://fastapi.tiangolo.com/advanced/events/?h=life#lifespan-events (в теории курса это еще не обновили, FastAPI развивается достаточно быстро) В документации этот способ даже рекомендован
    The recommended way to handle the startup and shutdown is using the lifespan parameter of the FastAPI app as described above.

    https://ngwaifoong92.medium.com/introduction-to-lifespan-events-in-fastapi-49ff39c55651

  5. Тут советовал бы использовать gunicorn для запуска https://fastapi.tiangolo.com/deployment/server-workers/#server-workers-gunicorn-with-uvicorn и вот об этом самой документации uvicorn - https://www.uvicorn.org/deployment/#gunicorn
  6. Тут вы использовали синхронный вариант Sqlalchemy, но вы использовали асинхронный фреймворк - потому Вам нужно использовать возможность работы Sqlalchemy асинхронно - https://docs.sqlalchemy.org/en/20/orm/extensions/asyncio.html
  7. Здесь используйте асинхронную версию https://aio-pika.readthedocs.io/en/latest/index.html , иначе вызовы к реббиту будут блокирующими.
  8. Тут выглядит как дубликат.
  9. Тут синтаксич ошибка, message неопределена.
  10. Для отправки массовых рассылок я бы рекомендовал специализированный сервис, что то вроде Sendgrid или его аналоги. Отправлять письма поштучно, когда вам нужно отправить 10 тыс писем - задача непростая.
  11. Можно добавить аннотаций типов в методы тут и по коду в общем.
  12. Тут лучше указать конкретные версии.
  13. Тут в реальных проектах не забывайте менять пароли по дефолту.
BigDeepBlue commented 1 year ago

Молодцы ! быстро подправили !