search

oruchkin / Async_API_sprint_3

AsyncAPI Спринт 3
1 stars 1 forks source link

Code review #122

Closed BigDeepBlue closed 4 days ago

BigDeepBlue commented 4 days ago

Мелочи:

  1. Крайне рекомендую, это как минимум, крайне желательно, указывать конкретные версии зависимостей. Сейчас у некоторых из них версия не указана в notifications/requirements.txt, а это значит, что при очередном релизе, скачаются последние версии, а там могут быть изменены интерфейсы и тогда у вас все перестанет работать.
  2. Для работы с почтой в dev окружении рекомендую https://github.com/mailhog/MailHog. Очень удобный инструмент выступающий в качестве SMTP сервера и web интерфейсом для просмотра всех отправленных сообщений. Вот еще пример https://akrabat.com/using-mailhog-via-docker-for-testing-email/
  3. Имя файла немного сбивает с толку notifications/src/db/mogno.py
  4. Вот это лучше отправить в класс с настройками в notifications/src/core/settings.py
  5. Названия очередей в notifications/src/db/rabbitmq.py, тоже лучше не хардкодить, а держать в настройках и переменных окружения.
  6. А вот это что? Уточните пожалуйста.
  7. Наверное, забыли вот это убрать
  8. Можно было в этом месте использовать https://docs.python.org/3/library/http.html#http.HTTPMethod
GDreyV commented 4 days ago

Большое спасибо! Все поправили:

  1. Да, это два противоборствующих лагеря - одни за стабильные сборки, другие против глубокого legacy :) Версии проставили.
  2. Прикольный сервис, спасибо
  3. Хаха, вот почему у меня автокомплит странно работал ))))
  4. Логично, перенесли в настройки
  5. Да, тоже перенесли в настройки
  6. Добавили, чтобы было в 3х кавычках, так попривычнее. Но то ли pylint. то ли mypy ругались на такое, что, если одна строка, то кавычки должны быть одинарные. В общем как писать docstring к аттрибутам класса все равно неясно: https://peps.python.org/pep-0224/ (оставлю тут для истории)
  7. Да, подправили
  8. С HTTPMethod определенно удобнее, спасибо!