Bot dev/helpers to merge - #4
Conversation
…tbot-jeopardy into bot-dev/helpers-to-merge
# Conflicts: # app/game/models.py # app/store/database/accessor.py # app/store/database/models.py # rest/alembic/README # rest/alembic/env.py # rest/alembic/script.py.mako # rest/alembic/versions/b6f6e5cefec4_first_migration.py
…tbot-jeopardy into bot-dev/helpers-to-merge
…tbot-jeopardy into bot-dev/helpers-to-merge
artbakulev
left a comment
artbakulev
left a comment
There was a problem hiding this comment.
посмотрел код - основное замечание что очень все хаотично - функции не там где они должны быть, аксессуары не используются, а просто существуют во многих экземплярах, gino пытается создать модели при каждом запросе... В идеале, каждый должен делать свою работу, например GameHelper не должен исполнять запросы к базе, это не его зона ответственности. И еще большое замечание - код очень разобщен и частично из-за этого и возникает первое замечание. Игровые функции не имеют доступа к app, а следовательно и к конфигу, и к PostgresAccessor, и к логгеру, и поэтому им приходится инстанцировать объекты каждый раз, а конфиг вообще обходить стороной - сделайте хотя бы так, чтобы в GameHelper пробрасывался экземпляр app (к которому должны быть привязаны все вышеперечисленные сущности) и из app уже доставайте все необходимое. В остальном хорошо, молодцы что научились работать с aiogram и поняли как обрабатывать игровые сессии
| from bot.middlewares.db import PostgressMiddleware | ||
|
|
||
|
|
||
| bot = Bot(token=TOKEN) |
There was a problem hiding this comment.
мне не очень нравится, что бот и диспетчер создаются тут, сделайте отдельный accessor для бота как в репе-примере - возможно в будущем нужно будет сделать что-то on_connect, on_disconnect (кстати, у бота есть метод close) + надо привязать бота к приложению, чтобы к нему был доступ по app.bot или хотя бы app['bot']
| dp = Dispatcher(bot, storage=MemoryStorage()) | ||
|
|
||
|
|
||
| async def main(): |
There was a problem hiding this comment.
в одной функции - один смысл.
у вас эта функция разрастется до невозможности при дальнейшем написании кода. Лучше переделать на setup_configs, setup_handlers и т.д., а тут вызывать уже просто по порядку. К примеру о неразберихе в коде: почему мидлвары при запуске добавляются? логичнее было бы их вынести в функцию main и только main запускать
| async def create_session(self) -> None: | ||
| from app.store.database.models import db | ||
| await db.set_bind( | ||
| "postgresql://postgres:1111@localhost:5433/postgres" |
There was a problem hiding this comment.
надо из конфигов это тягать
| from aiogram import types | ||
|
|
||
|
|
||
| class PostgressMiddleware(BaseMiddleware): |
|
|
||
|
|
||
| class PostgressMiddleware(BaseMiddleware): | ||
| def __init__(self): |
There was a problem hiding this comment.
Очень страшная мидлвара - не надо открывать соединение на каждый запрос, это очень не эффективно - надо переиспользовать соединение или, если есть необходимость, иметь пул соединений и из него уже брать. Представьте ситуацию - к вам пришло 100 медленных клиентов одновременно == вы открыли 100 соединений к базе, что ухудшит ее производительность. Открытие/закрытие соединений тоже дает overhead, это не мгновенная операция - база должна проверить, что ничего не выполняется и она может закрыть соединение безопасно. Убирайте мидлвару, создайте одно соединение, привяжите к PostgresAccessor и пользуйтесь им
|
|
||
| class GameHelper: | ||
|
|
||
| # Config |
There was a problem hiding this comment.
вынесите все в нормальный конфиг и устанавливайте в init
| error = 4 | ||
|
|
||
|
|
||
| class GameHelper: |
There was a problem hiding this comment.
Все операции с базой должны происходить в PostgresAccessor - нельзя раскидывать чистые вызовы по коду
|
|
||
| async def game_status(message: types.Message): | ||
| game = GameHelper(message.chat.id, db) | ||
| state = await game.GetState() |
There was a problem hiding this comment.
это все надо переносить в GameHelper, не надо часть логики по игре оставлять во view, а часть в классе. в идеале View должен принять запрос, вызвать какую-то функцию и отправить ответ
|
|
||
| from app.store.database.accessor import PostgresAccessor | ||
|
|
||
| pg = PostgresAccessor() |
There was a problem hiding this comment.
один accessor на все приложение, практически из каждой функции и класса должен быть доступ к app (не к глобальному только)
| @@ -0,0 +1,3 @@ | |||
| from bot.settings import cfg | |||
|
|
|||
| TOKEN = cfg["bot"]["token"] | |||
There was a problem hiding this comment.
грустно... сделайте dataclass с конфигурацией бота, у вас там не только token будет
…tbot-jeopardy into bot-dev/helpers-to-merge
3 participants
No description provided.