What is wrong with maxapi (imho)

[K1rL3s]

Что?

По просьбе @love-apples постарался собрать здесь список того, что меня не устраивает в махапи.
По приоритетам не сортировал, шёл по коду, отмечал

Отталкиваемся от того, что в ридми написано

Dispatcher, Router, фильтры, F и middleware в стиле aiogram.

поэтому далее будут апелляции к аиограму

TL;DR

• Странная реализация роутеров
• Странная реализация методов
• Спорные типы и их разделение
• Странные директории
• God Dispatcher
• Странная реализация ошибок и их обработки
• Странная реализация евент-обсерверов
• Почему всё не сделано, не скопировано, не взято из аиограма?

Увиденное

• Кто такой maxapi.client, если там нет клиента, а только DefaultConnectionProperties, создание ssl контекста и коннектра?
  DefaultConnectionProperties используется только в bot.py, функции из ssl.py только в bot.py и maxapi.connection.base.BaseConnection. Почему не перенести весь maxapi.client в maxapi.connection, или наоборот, maxapi.connection в maxapi.client? Две отдельные директории для одного и того же, в первой - настройки, во второй - базовый клиент.
• Почему maxapi.context, который в аиограме aiogram.fsm.storage? Почему не скопировать из аиограма?
  Также к State, StateGroup, ContextManager и всему в maxapi.context - почему не скопировать из аиограма?
• Зачем луа скрипт в редис в питоне для стейт даты? Почему не скопировать реализацию RedisStorage из аиограма?
  

  maxapi/maxapi/context/context.py

  Lines 156 to 172 in 3da4311

  	lua_script = """
  	local data = redis.call('get', KEYS[1])
  	local decoded = {}
  	if data then
  	decoded = cjson.decode(data)
  	end
  	local updates = cjson.decode(ARGV[1])
  	for k, v in pairs(updates) do
  	decoded[k] = v
  	end
  	if ARGV[2] ~= "" then
  	redis.call('set', KEYS[1], cjson.encode(decoded), 'PX', ARGV[2])
  	else
  	redis.call('set', KEYS[1], cjson.encode(decoded))
  	end
  	return cjson.encode(decoded)
  	"""

• Дальше чуть выпал. Зашёл в maxapi.enums, вижу ApiPath. Думаю: "А зачем выносить пути в енум?". Ткнул использования, и вижу ВОТ ЭТО.
  

  maxapi/maxapi/methods/delete_bot_from_chat.py

  Lines 44 to 48 in d312003

  	path=ApiPath.CHATS
  	+ "/"
  	+ str(self.chat_id)
  	+ ApiPath.MEMBERS
  	+ ApiPath.ME,

  
  

  maxapi/maxapi/methods/remove_admin.py

  Lines 42 to 48 in d312003

  	path=ApiPath.CHATS
  	+ "/"
  	+ str(self.chat_id)
  	+ ApiPath.MEMBERS
  	+ ApiPath.ADMINS
  	+ "/"
  	+ str(self.user_id),

  
  Почему надо было выносить ЧАСТИ ручек в енум, чтобы потом склеивать их?
  Почему не просто f"/some/path/{self.user_id}/etc", "/another/{chat_id}/path".format(chat_id=self.chat_id)?
• Куда-то в maxapi.enums.parse_mode или maxapi.enums.format можно варнинг про депрекейт или коммент почему существуют алиасы
• Не думаю, что этот енум нужен в паблик апи
  

  maxapi/maxapi/enums/http_method.py

  Line 7 in 025493a

  class HTTPMethod(StrEnum):

  
  Каюсь, сам грешил подобным в алисии, щас считаю это не лучшим решением
• Раз у нас есть MaxApiError (не уверен, это ошибка maxapi, или ошибка от API Макса), то можно отнаследовать от неё все остальные ошибки либы. Сейчас нет возможности повесить куда-то except MaxApiError, чтобы ловить все ошибки либы
  

  maxapi/maxapi/exceptions/max.py

  Lines 19 to 25 in 6756d39

  	@dataclass(slots=True)
  	class MaxApiError(Exception):
  	code: int
  	raw: str | dict[str, Any]
  	def __str__(self) -> str:
  	return f"Ошибка от API: {self.code=} {self.raw=}"

• Само существование maxapi.exceptions.dispatcher с HandlerException и MiddlewareException уже необъяснимо. Почему не скопировать подход аиограма?
• Почему в одном maxapi/dispatcher.py и Dispatcher (НА 1700 СТРОК), и Router, и ErrorEventObserver, и Event?
  • Почему диспетчер сам отвечает за (свои И ЧУЖИЕ) мидлвари, почему нет MiddlewareManager? Относится ко всем атрибутам и методам *_middleware* в диспетчере
  • Почему диспетчер это CallableObject из аиограма?
  • Почему паттерн "цепочка ответственности" на роутерах сделан через _iter_routers (и ещё 10 методов после него), а не вызов метода aiogram::Router.propagate_event и подобные?
  • Почему в диспетчере _get_middleware_title???
  • Почему в диспетчере ещё и отдельно методы для еррор ивента, почему еррор ивент не идёт как обычный ивент?
  • Почему диспетчер и запускает поллинг, и запускает вебхук? Почему не разделить ответственность?
  • Почему Router наследуется от Dispatcher, а не наоборот?
  • Почему ErrorEventObserver не наследник Event? Почему нет какого-то базового EventObserver, от которого будут MaxEventObserver, и ErrorEventObserver, и ещё какой-нибудь Observer?
  • Почему не скопировать подход и код из аиограма???
• Почему Handler и ErrorHandler (второй не наследуется от первого) находятся в maxapi/filters/handler.py?
• Если maxapi.filters.command.Command реализует only_with_bot_username: bool, который явно взят из аиограма, почему остальное не взято из аиограма?
  И что это?
  

  maxapi/maxapi/filters/command.py

  Lines 199 to 202 in d312003

  	async def __call__(
  	self, event: UpdateUnion
  	) -> dict[str, list[str]] | bool:
  	return await super().__call__(event)

• Это вообще что? Если пидантик, почему не взять подход из аиограма с context?
  https://github.com/love-apples/maxapi/blob/97c5d349a387388a729617625e8e6ab199434b73/maxapi/utils/runtime.py#L44-L67
• Если есть default_connection с настройками соединения, почему не собрать остальные параметры в BotSettings/BotConfig/BotDefaults?
  

  maxapi/maxapi/bot.py

  Lines 90 to 114 in 5c80ddf

  	class Bot(BaseConnection):
  	"""
  	Основной класс для работы с API бота.
  	Предоставляет методы для взаимодействия с чатами, сообщениями,
  	пользователями и другими функциями бота.
  	"""
  	def __init__(
  	self,
  	token: str | None = None,
  	*,
  	format: TextFormat | None = None,
  	parse_mode: ParseMode | None = None,
  	notify: bool | None = None,
  	disable_link_preview: bool | None = None,
  	auto_requests: bool = True,
  	default_connection: DefaultConnectionProperties | None = None,
  	after_input_media_delay: float | None = None,
  	after_upload_attempts: int | None = None,
  	after_upload_retry_delay: float | None = None,
  	after_upload_give_up_timeout: float | None = None,
  	auto_check_subscriptions: bool = True,
  	marker_updates: int | None = None,
  	):

• Какой толк делать Bot._me приватным?
  

  maxapi/maxapi/bot.py

  Lines 236 to 249 in 5c80ddf

  	@property
  	def me(self) -> User | None:
  	"""
  	Возвращает объект пользователя (бота).
  	Returns:
  	User | None: Объект пользователя или None.
  	"""
  	return self._me
  	@me.setter
  	def me(self, value: User | None) -> None:
  	self._me = value

• И дальше все методы бота, которые вызывают методы API, просто копируют сигнатуру инитов API методов, создают методы и вызываю .fetch(). В аиограме такая копипаста из-за автогенерации, тут - зачем?
• Импорт с четырёх точек, почему не просто maxapi.enums? Это же и к импортам с тремя и иногда с двумя точками
  

  maxapi/maxapi/types/attachments/buttons/attachment_button.py

  Line 3 in 046798b

  from ....enums.attachment import AttachmentType

• Все типы наследуются только от pydantic.BaseModel, как понять, что в них ещё есть тот самый .bot из bind_bot?
• Зачем разделять типы на maxapi.methods.types и maxapi.types? И потом добавлять костыль под обратную совместимость.
  

  maxapi/maxapi/types/__init__.py

  Lines 130 to 141 in db78fde

  	if TYPE_CHECKING:
  	from ..methods.types.sended_message import SendedMessage
  	def __getattr__(name: str) -> Any:
  	if name == "SendedMessage":
  	from ..methods.types.sended_message import ( # noqa: PLC0415
  	SendedMessage,
  	)
  	return SendedMessage
  	raise AttributeError(f"module {__name__!r} has no attribute {name!r}")

  
  Можно понять "разделение на модели запросов и модели ответов", но в аиограме не так, и хз насколько удобно юзеру импортировать часть моделей из одного места, часть из другого
• Про maxapi.methods:
  • Почему все методы наследуются от BaseConnection, который шлёт запросы? Почему методы просто не хранят квери-хедеры-бади-путь, а отвечают ещё и за отправку запросов? Почему это не отдельный класс как aiogram.client.base.session.BaseSession?
  • Про формирование path было выше
  • Логика Nullable и optional реализована через ручной перебор каждого параметра, проверкой его на None, и сбором тела и квери руками. Буквально в каждом методе одни и те же if-else в перемешку с валидацией значений и дампом вложенных моделей.
    Если в API встретится с момент, где None - одно, а "не передано" - другое, придётся добавлять NOT_SET и делать условия по нему. Из хорошего, в апи я такого пока не нашёл

  In objects, a nullable property is not the same as an optional property, but some tools may choose to map an optional property to the null value

• Мелочь. setuptools вместо uv или хотя бы poetry

Что дальше?

Я думаю, что буду дополнять этот ишак комментами, если буду углубляться в реализацию махапи. Спасибо!

[K1rL3s]

И если вы хотите указать на бревно в моём глазу - https://github.com/K1rL3s/maxo
Я открыт к обсуждению махапи, махо, аиограма, телеграма и макса

[goduni]

наследование методов от BaseConnection выглядит ну крайне странно

к слову о том, как у maxo выглядят методы через unihttp:

from maxo.bot.methods.base import MaxoMethod
from maxo.bot.methods.markers import Body, Path
from maxo.types.modify_members_result import ModifyMembersResult


class AddMembers(MaxoMethod[ModifyMembersResult]):
    """
    Добавление участников в групповой чат или канал

    Добавляет участников в групповой чат или канал 

     Бот, чей токен `access_token` используется для авторизации, должен быть администратором этого чата или канала с соответствующим правом `add_remove_members`. Чтобы получить информацию о правах бота, используйте [GET /chats/-chatId-/members/admins](https://dev.max.ru/docs-api/methods/GET/chats/-chatId-/members/admins). Подробнее о правах - в описании объекта [`Chat`](https://dev.max.ru/docs-api/objects/Chat)

    Пример запроса:
    ```bash
    curl -X POST "https://platform-api.max.ru/chats/{chatId}/members" \
      -H "Authorization: {access_token}" \
      -H "Content-Type: application/json" \
      -d '{
      "user_ids": ["{user_id_1}", "{user_id_2}"]
    }'
    ```

    Args:
        chat_id: ID группового чата или канала
        user_ids: 

    Источник: https://dev.max.ru/docs-api/methods/POST/chats/-chatId-/members
    """

    __url__ = "chats/{chat_id}/members"
    __method__ = "post"

    chat_id: Path[int]
    """ID группового чата или канала"""

    user_ids: Body[list[int]]