search

Gaidunov / team_notifications_sprint_1

0 stars 0 forks source link

Code review #1

Open BigDeepBlue opened 1 year ago

BigDeepBlue commented 1 year ago

Интересная реализация 🔥

  1. Разработка Websocket сервера была опциональной. В этой связи просто к сведению: websocket как средство уведомления в рамках сервиса уведомлений должен быть интегрировано в этот сервис во всем вытекающим - авторизация, отправка уведомлений по запросу. А не только чат для всех.
  2. Моделям 1_project/notifier/models.py в качестве id лучше сделать UUID, в микросервисном приложении использовать просто целое число будет проблематично.
  3. Запуск приложения не должен быть запуском питонячего скрипта - запуск через uwsgi, gunicorn, uvicorn и т.д.
  4. Для работы с почтой в dev окружении рекомендую https://github.com/mailhog/MailHog. Очень удобный инструмент выступающий в качестве SMTP сервера и web интерфейсом для просмотра всех отправленных сообщений. Вот еще пример https://akrabat.com/using-mailhog-via-docker-for-testing-email/. Сейчас же заглушка смотрится не очень хорошо.
  5. Этот адрес необходимо отправить в настройки и переменную окружения.
  6. А у вас в сервисе авторизации эта ручка как-нибудь защищена от доступа снаружи? Это, конечно же, опционально, но для межсервисного взаимодействия я бы рекомендовал gRPC.
  7. Вот это Flask приложение 1_project/notifier/app/app.py нужно как-нибудь отделить от Django.
BigDeepBlue commented 1 year ago

LGTM