Added custom http client, update webhook, new download methods - #167
Added custom http client, update webhook, new download methods#167m-xim wants to merge 113 commits into
Conversation
|
Important Review skippedToo many files! This PR contains 213 files, which is 113 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (213)
You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
K1rL3s
added
enhancement
…essage_callback.py
…mprove error handling
…nced request handling
m-xim
changed the title
Разрешение конфликтов: - сохранена архитектура webhook3 с общим retort и фасадами в maxo.types.facades - comment API из master перенесен в новый слой; обновлены warming up, defaults, serialization и настройки butcher - удаленный webhook routing не восстановлен; skill-пути сведены к .agents Проверки: - ruff, codespell, slotscheck, bandit и mypy - 1736 passed, 1 rerun - butcher: 63 passed
chore: merge master into webhook3
# Conflicts: # src/maxo/serialization.py # tests/maxo/bot/test_api_client.py
|
P1: нормализовать сетевые ошибки при разрешении бота При ленивом разрешении |
| def warm_up( | ||
| *, | ||
| loaded: Iterable[type] | None = None, | ||
| dumped: Iterable[type] | None = None, | ||
| ) -> None: | ||
| from maxo.serialization import get_retort # noqa: PLC0415 - avoids import cycle | ||
|
|
||
| for tp in types: | ||
| retort_method(tp) # type: ignore[arg-type] | ||
| retort = get_retort() | ||
|
|
||
| return retort | ||
| for type_ in _LOADED_ROOTS if loaded is None else loaded: | ||
| retort.get_loader(type_) | ||
| for type_ in _DUMPED_ROOTS if dumped is None else dumped: | ||
| retort.get_dumper(type_) |
There was a problem hiding this comment.
С варм_апом точно что-то не так. Нельзя прогреть реторту, полученную из create_retort. Если у нас реторта одна и глобальная, то надо сделать и create_retort кэширующим одну реторту, как делает get_retort. Либо оставить один метод получения реторты, либо передавать реторту в warm_up
There was a problem hiding this comment.
У нас есть create_retort, которая создаёт реторту. Есть get_retort, которая вызывает create_retort и кэширует результат. В warm_up использует строго get_retort, то есть юзер, создавший реторту через create_retort, не сможет прогреть её
| @@ -1,8 +1,16 @@ | |||
| from __future__ import annotations | |||
There was a problem hiding this comment.
Увидел здесь, прокомментирую здесь. Запрещаю from __future__ import annotations, убрать отовсюду
| json_dumps: Callable[[Any], str] = json.dumps, | ||
| json_loads: Callable[[str | bytes | bytearray], Any] = json.loads, | ||
| client: BaseAsyncClient | None = None, | ||
| middlewares: Sequence[AsyncMiddleware] = (), |
There was a problem hiding this comment.
Я передумал, надо обратно назвать его middleware, потому что он везде middleware =(
| class SingleBotEngine( | ||
| BaseWebhookEngine[AppT, RawRequestT, FrameworkResponseT], | ||
| Generic[AppT, RawRequestT, FrameworkResponseT], | ||
| ): |
There was a problem hiding this comment.
В чате была мысль, что Single и Simple легко перепутать, поэтому надо добавить SimpleBotEngine: TypeAlias = SingeBotEngine и импортировать его в те же __init__.py, что и SingeBotEngine
| async def start(self) -> None: | ||
| if self.state.started: | ||
| return | ||
|
|
||
| api_client = MaxApiClient( | ||
| token=self._token, | ||
| request_dumper=self._retort, | ||
| response_loader=self._retort, | ||
| middleware=self._middleware, | ||
| upload_config=self._upload_config, | ||
| json_dumps=self._json_dumps, | ||
| json_loads=self._json_loads, | ||
| ) | ||
| self._state = ConnectingBotState(api_client=api_client) | ||
|
|
||
| info = await self.get_my_info() | ||
| self._state = RunningBotState(info=info, api_client=api_client) |
There was a problem hiding this comment.
Щас метод start пропал вообще, и логика started=True -> self._info is not None неочевидна. Можно добавить алиас start = get_my_info, чтобы человекам было понятнее и была минимальная обратная совместимость
| class TokenEngine( | ||
| BaseMultiBotEngine[AppT, RawRequestT, FrameworkResponseT], | ||
| Generic[AppT, RawRequestT, FrameworkResponseT], | ||
| ): |
There was a problem hiding this comment.
Если SingleBotEngine и TokenEngine, то надо либо SingleEngine, либо TokenBotEngine
| async def silent_call_method(self, method: MaxoMethod[_MethodResultT]) -> None: | ||
| try: | ||
| await self.call_method(method) | ||
| except MaxBotApiError as e: | ||
| # Webhook-ответ не позволяет вернуть ошибку вызывающему коду. | ||
| loggers.bot.error("Failed to make answer: %s: %s", e.__class__.__name__, e) |
There was a problem hiding this comment.
Удалён
Bot.silent_call_method().Webhookбольше не интерпретирует возвращённый хендлеромMaxoMethodкак отложенный ответ
Почему?
| both synchronous and background processing. | ||
| """ | ||
|
|
||
| class BaseWebhookEngine(ABC, Generic[AppT, RawRequestT, FrameworkResponseT]): |
There was a problem hiding this comment.
handle_in_backgroundудалён, Webhook updates всегда обрабатываются в фоне
А обязательно всегда в фоне? Нет флага на последовательную обработку?
Подсократил тесты кукодексом
|
Я склонировал репозиторий, изучил дифф (214 файлов, +5937/−4289 относительно master) и прогнал тесты локально. Вот моё ревью. Общее впечатлениеPR добротный: архитектура вебхуков в стиле aiogram-webhook3 (разделение на Блокирующие замечания1. Даунгрейд версии. В master 2. Утечка токена при скачивании произвольных URL. 3. Утечка сессии в 4. DoS-вектор в авторегистрации Некритичные замечания
В В Мелочь: в По процессу104 коммита и 214 файлов — CodeRabbit даже отказался ревьюить из-за лимита в 100 файлов. Кастомный HTTP-клиент, download-методы и переписывание вебхуков — три логически независимых блока, которые стоило разнести по отдельным PR; на будущее это сильно ускорит ревью. И в чек-листе не отмечены «самопроверка кода» и «изменения в документации», а два пункта про авторство (нейросети/человек) оставлены оба пустыми — стоит привести в порядок перед мержем. Ответ автора про Если хочешь, могу глубже разобрать какой-то конкретный модуль — например, |
2 participants
Описание
Introduced a custom HTTP transport and updated the webhook to the latest version of aiogram-webhook 3.
Closes #74
Closes #121
Тип изменения
Контрольный список: