119 lines
5.8 KiB
Markdown
119 lines
5.8 KiB
Markdown
# Анализ проекта IntradBot — Senior Review
|
||
|
||
## Общая оценка
|
||
|
||
Проект написан чисто, логика разбита по модулям правильно, читается легко.
|
||
Для своего масштаба архитектура адекватная. Ниже — конкретные проблемы
|
||
от критических до косметических.
|
||
|
||
---
|
||
|
||
## SMART применим?
|
||
|
||
SMART — это фреймворк для постановки целей (Specific / Measurable / Achievable /
|
||
Relevant / Time-bound), не для архитектуры. Скорее всего вы имели в виду
|
||
**SOLID** + **DRY** + **KISS**. Вот где они нарушены:
|
||
|
||
---
|
||
|
||
## Критические проблемы
|
||
|
||
### 1. Flask dev-сервер в продакшене
|
||
**Файл:** `web/server.py` → `app.run(...)`
|
||
|
||
Flask's built-in server однопоточный, не рассчитан на нагрузку и явно
|
||
написано в документации: *"Do not use the development server in production"*.
|
||
При одновременных запросах (webhook + кто-то открыл miniapp) будут задержки.
|
||
|
||
**Решение:** Gunicorn — одна строка в Dockerfile.
|
||
|
||
### 2. Пароль захардкожен в коде
|
||
**Файл:** `config.py`
|
||
```python
|
||
DASHBOARD_PASSWORD = os.environ.get("DASHBOARD_PASSWORD", "capsspac")
|
||
```
|
||
Дефолтное значение пароля прямо в репозитории — плохая практика.
|
||
Если этот код когда-либо окажется на GitHub (даже приватном) — пароль скомпрометирован.
|
||
|
||
**Решение:** дефолт должен быть пустой строкой `""`.
|
||
|
||
---
|
||
|
||
## Серьёзные проблемы
|
||
|
||
### 3. Дублирование ~150 строк в formatter.py (нарушение DRY)
|
||
`parse_lifetime()` и `format_lifetime_api_events()` — это одна и та же логика
|
||
разбора событий, написанная дважды. Любое изменение (новый тип события,
|
||
новый формат файла) нужно делать в двух местах — классический источник багов.
|
||
|
||
**Решение:** вынести разбор одного события в `_parse_single_event()`,
|
||
обе функции используют её.
|
||
|
||
### 4. Hardcoded ID сотрудников и группы (нарушение SOLID — OCP)
|
||
**Файл:** `bot/telegram.py`
|
||
```python
|
||
EXECUTOR_USER_IDS = [2227907, 631481, 525111]
|
||
```
|
||
**Файл:** `web/server.py`
|
||
```python
|
||
"(status ne 68045) and (executorgroup eq 243025)"
|
||
```
|
||
Чтобы добавить сотрудника или сменить группу — нужно лезть в код и делать rebuild.
|
||
|
||
**Решение:** вынести в `.env` / `config.py`.
|
||
|
||
### 5. 17 новых DB-соединений на каждый запрос
|
||
**Файл:** `database.py` — каждая функция вызывает `_connect()`.
|
||
SQLite при частых соединениях создаёт overhead на открытие/закрытие файла.
|
||
|
||
**Решение:** использовать `threading.local()` для переиспользования соединения
|
||
в рамках одного потока.
|
||
|
||
---
|
||
|
||
## Средние проблемы
|
||
|
||
### 6. Незапиненные зависимости
|
||
**Файл:** `requirements.txt`
|
||
```
|
||
requests ← без версии
|
||
flask ← без версии
|
||
schedule ← без версии
|
||
```
|
||
При следующем `docker build` может поставиться breaking версия.
|
||
|
||
**Решение:** зафиксировать все версии.
|
||
|
||
### 7. Нет `.dockerignore`
|
||
При `COPY . .` в контейнер попадают: `ssl/`, `data/`, `tests/`, `.env`,
|
||
`__pycache__`, `.git` если будет. Это раздувает образ и создаёт риск
|
||
утечки секретов.
|
||
|
||
### 8. Нет валидации конфига при старте
|
||
Если `TELEGRAM_TOKEN` не задан — бот упадёт не сразу, а в момент первого
|
||
обращения к Telegram с непонятной ошибкой.
|
||
|
||
**Решение:** проверять обязательные переменные в `config.py` на старте.
|
||
|
||
---
|
||
|
||
## Что НЕ нужно трогать
|
||
|
||
- **SQLite → PostgreSQL**: при текущей нагрузке SQLite достаточно.
|
||
- **Разбить на микросервисы**: оверкилл, один процесс с потоками — нормально.
|
||
- **ORM (SQLAlchemy)**: текущие raw SQL запросы читаются лучше.
|
||
- **Celery/Redis**: webhook обрабатывается в отдельном треде, этого хватает.
|
||
|
||
---
|
||
|
||
## Итог изменений
|
||
|
||
| # | Файл | Изменение | Приоритет |
|
||
|---|------|-----------|-----------|
|
||
| 1 | `Dockerfile` + `requirements.txt` | Gunicorn вместо dev-сервера | 🔴 Критично |
|
||
| 2 | `config.py` | Убрать захардкоженный пароль, добавить валидацию, EXECUTOR_USER_IDS, DEFAULT_FILTER | 🔴 Критично |
|
||
| 3 | `intradesk/formatter.py` | Вынести `_parse_single_event()`, убрать дублирование | 🟡 Важно |
|
||
| 4 | `database.py` | Thread-local соединение | 🟡 Важно |
|
||
| 5 | `requirements.txt` | Зафиксировать версии | 🟡 Важно |
|
||
| 6 | `.dockerignore` | Добавить | 🟢 Хорошая практика |
|
||
| 7 | `web/server.py` | Убрать `app.run()`, добавить `create_app()` | 🟢 Хорошая практика |
|