-
Notifications
You must be signed in to change notification settings - Fork 2
Feature/bot refactor simple commands #153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Шаблон не должен содержать бизнес-логику.
После удаления, вернусь к ревью
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Все что касается redis repo посмотри в email-smartapp/-/merge_requests/242 - там был разбор этой темы.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- важно держать "тяжёлые" подключения и пуллы соединений в единственном экземпляре на процесс (AsyncEngine SQLAlchemy, async_sessionmaker и redis.Redis). SQLAlchemy engine стоит сдлеать как Resource или Singleton, в session_factory стоит сделать bind на engine(не збадуь), ну и клиента через Pool. Плюс добавь сюда TODO для поддержки работы с redis sentinel.
- всё что "лёгкое" и/или должно создаваться на запрос/единицу работы - поднимать фабриками. Repository на конкретную ASyncSession, UseCase на конкретную репу, Uow. Это позволит иметь "чистые" границы транзакции, повысит потокобезоспаность и тестируемость.
- всё что "реестр команд/маршрутов", лучше собирать на старте и передавать уже готовым списком в Bot/Router, а не отдавать "ленивые провайдеры" дальше по стеку. Это даст тебе возможность детерменированного старта и выявления ошибок при запуске, а не использовании; не будут течь провайдеры в сторонние фреймворки
return self | ||
|
||
@abstractmethod | ||
@abstractmethod |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 раза??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Чтоб наверняка)
Changes in this Pull Request
state
andctx
with explicit injection using dependency-injectorCollector Handler → Command Handler → Unit of Work → Use Case → Repository → Database