SmirnovaT / notifications_sprint_1

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

Code review #41

Open BigDeepBlue opened 2 months ago

BigDeepBlue commented 2 months ago

Немного критики😀

  1. Такие вещи нужно держать в переменных окружения.
  2. Для чего у вас notification_service/src/schemas. Там пусто, может быть есть смысл удалить?
  3. Не забывайте указывать типы принимаемых параметров и возвращаемых значений в методах. Например, scheduler_worker/src/services/send_notification_to_queue.py
  4. Тут мне даже IDE подсказывает, что: Instance attribute summary_data defined outside __init__ Это не очень хорошо - инициализировать поля класса за пределами метода init не только потому-что их сложно отследить в коде, но и по причине экономии памяти, если поля класса инициализируются в init, то при создании первого экземпляра класса создается shared хэш таблица общая для всех инстансов. А если за пределами, то создается для каждого объекта своя.
  5. Вызов синхронного метода в асинхронном. Это плохо и заблокирует цикл событий. Либо сделайте все асинхронным, либо обратитесь к https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.run_in_executor
GreyGreyman commented 2 months ago

Для 5ого пункта использовали метода asyncio.to_thread который под капотом передает контекст и вызывает run_in_executor с ThreadPoolExecutor'ом