code review // outages-ge-bot
В целом, всё более-менее =) Далее разбил по модулям, чтоб было более конструктивно и понятно. Проект небольшой, но дам советы, которые пригодятся в будущем.
db -> models.py
На каждую модель должен быть отдельный файл. Так и проще для ориентирования и для масштабирования. В файле с моделью у тебя могу лежать и функции и инструменты, которые непосредственно связаны с данной моделью.
В твоём случае должно быть так:
model
chat.py
address.py
sent_outage.py
Заметь, название файла отображает название модели в единственном числе. Название таблицы в множественном - как у тебя и есть.
def default_full_address - у тебя используется в одном месте (дефолт для поля full_address у addresses) и нужна она исключительно для данного поля. Так добавь её в данный класс и объяви приватной (_default_full_address).
Так же начни использовать индексы. В данном случае они вообще не критичны, но для понимания, опыта и ускорения выполнения операций, хоть и на чуть, будет полезно. Подумай сам, по каким уникальным полям будет происходить поиск чаще всего?
Про удаление на примере "Chat"
Вообще, лучше никогда, ничего не удалять с базы. Юзай статусы.
Добавь колону "state" и меняй у неё нужные тебе статусы.
state = Column(Integer, nullable=False, default=2)
# далее внизу пишешь так:
State = _ChatState()
# и отдельно обьявляешь допустимые статусы:
class _ChatState(Enum):
def __init__(self, **kwargs):
super(_ChatState, self).__init__(**kwargs)
self.active = 1
""" активный """
self.inactive = 2
"""нет активный"""
self.deleted = 3
""" удаленный """
self.banned = 4
"""заблокированный"""
Это к примеру, главное понять суть, что так больше функциональности и гибче код
db -> functions.py
По поводу функций insert_chat, delete_chat, is_address_num_exceeded
А что если тебе нужно добавить или удалить несколько чатов? Допустим в коде тебе надо будет удалить 100 чатов. Ты будешь 100 раз открывать сессию, удалять чат, комитеть там и закрывать сессию?
Лучше добавить функциональность по добавлению и т.д. как одного, так и множества сущностей.
По коду у тебя не возникает такий ситуаций. Но так как ты, допустим, не будешь удалять, а будешь проставлять статус deleted, то можно реализовать следующим образом:
def delete_chat(chats: [str]): """Delete chats from database""" with Session(engine) as session: session.begin() result_ids = [] for chat_id in chats: chat = session.query(Chat).filter_by(tg_chat_id=chat_id).one_or_none() if chat: chat.state = Chat.State.deleted result_ids.append(chat.id) else: logging.info({chat_id} not found) try: session.commit() logging.info(Total chat was deleted: {len(result_ids)}. Chat: ({result_ids}) was deleted from database.') except Exception as err: logging.error(f'Chats ({chats}) wasn\'t deleted, {err}') return None
scrapers -> gwp.py and telasi.py
На данный момент ни а каком расширении тут и речи идти и не может =)
Заметил, что не используешь классы. Как раз вот тут они нужны.
Давай с очевидного not DRY
collect_outages, request_soup -> полностью дублируются. Даже не применяя классы их можно вынести отделно и импортировать
scrap_notifications, parse_notifications_info -> по большей части копируют друг друга, их вот как раз и нужно юзать в классе.
Тут у нас есть 2 пути:
1) Берём один файл и переписываем там всё под класс меняя вещи, которые будут меняться на атрибуты класса и методы. Далее уже мы наследуемся от этого класса и меняем атрибуты и методы под свой случай. Так же тут добавляем методы, которые именно здесь нужны.
2) Создаём абстрактный класс, там описываем методы, которые в общей случае должны быть, минимальную логику, атрибуты. Далее уже наследуемся от этого класса в конкретном случае. А когда нужно будет добавить новы скраппер, то если сильно прям он отличается, то так же наследуемся от абстрактного, если нет, то от уже существующего класса с логикой и заменяем что нужно.
То есть выглядеть класс будет примерно так:
Class Gwp:
def __init__(
self,
type: str = 'water'',
root_url: str = "",
urls: list[dict] | None = None):
self.type = 'water'
self.root_url = 'https://www.gwp.ge'
self.urls = [{"url": urljoin(ROOT_URL, '/ka/dagegmili'), "emergency": False},
{"url": urljoin(ROOT_URL, '/ka/gadaudebeli'),"emergency":True}]
И тут уже перечисляешь методы. Например, collect_outages ты укажешь один раз в абстрактном классе и её вообще писать не придётся. Если не хочешь передавать urls как-то в класс, то просто делаешь данный метод @staticmethod
get_info - ты просто создашь новый метод у класса, так как используется только в одном из 2
Вообще, это самый основной момент. Во-первых, самая важная часть. Во-вторых, самая перспективно маштабируемая.
Так же момент по поводу парсинга. Что будет если где-то, что-то на страннице поменяют? Парсер сломается и не всегда ты это сможешь заметить. Поэтому здесь если ты парсишь, что-то связанной со стилями или расположением инфы, то оборачивай или в try/except или обрабатывай результат, который не ожидаешь получить.
То есть если пришло, то что не должно прийти или упала ошибка, то логируй, чтоб ты мог оперативно увидеть это и поправить.
Общее
Ты писал про асинхронность. Я не совсем понял о чём именно ты имел в виду. На созвоне пояснишь тогда, а я расскажу почему так и что можно сделать.
По деплоую так же лучше обсудить. Ну в твоём случае прост через докер запускаешь и всё. Нет необходимости поднимать там сервер или кубы.
В целом с докером, гитигноре и проектом всё норм. Проект небольшой, выполняет свою функцию, написан нормально. Все хорошо =)
Каких-то явных ошибок и косяков нет. Я больше указал по архитектуре, чтоб ты мог масштабировать и сразу понимал в каком направлении думать при создании приложения.
Если используешь аннотацию типов, то и используй при указании возвращаемого значение (def test() -> bool)