fix(generator): обработка findings из финального ревью
- Изолированы ошибки history-канала: best-effort с логированием, не валят тик - Добавлено явное создание топика generator_batch_history при старте - Добавлена защита от пустого словаря (ValueError при загрузке) - Обновлена документация о структуре тестов - Исправлены тесты на пустой словарь Ревью: изоляция ошибок history, явное создание топика, защита от пустых данных
This commit is contained in:
Dmitry Dementiev
+3
-1
@@ -135,7 +135,9 @@ generator/tests/
|
||||
├── conftest.py # Fixtures pytest
|
||||
├── test_config.py # Тесты конфигурации
|
||||
├── test_generation.py # Тесты генерации событий
|
||||
└── test_history.py # Тесты истории батчей
|
||||
├── test_history.py # Тесты структуры BatchRecord
|
||||
├── test_kafka_history.py # Тесты KafkaBatchHistory
|
||||
└── test_service.py # Тесты GeneratorService
|
||||
```
|
||||
|
||||
### Интеграционный тест
|
||||
|
||||
+61
-9
@@ -174,11 +174,19 @@ class EventDictionary:
|
||||
logger.info(f" Loaded {len(events)} events from {filename}")
|
||||
return events
|
||||
|
||||
browser_events = load_jsonl("browser_events.jsonl")
|
||||
location_events = load_jsonl("location_events.jsonl")
|
||||
device_events = load_jsonl("device_events.jsonl")
|
||||
geo_events = load_jsonl("geo_events.jsonl")
|
||||
|
||||
if not browser_events:
|
||||
raise ValueError("browser_events.jsonl is empty or missing")
|
||||
|
||||
return cls(
|
||||
browser_events=load_jsonl("browser_events.jsonl"),
|
||||
location_events=load_jsonl("location_events.jsonl"),
|
||||
device_events=load_jsonl("device_events.jsonl"),
|
||||
geo_events=load_jsonl("geo_events.jsonl"),
|
||||
browser_events=browser_events,
|
||||
location_events=location_events,
|
||||
device_events=device_events,
|
||||
geo_events=geo_events,
|
||||
)
|
||||
|
||||
|
||||
@@ -249,6 +257,15 @@ class EventGenerator:
|
||||
|
||||
Возвращает словарь {topic: [events]}
|
||||
"""
|
||||
if not self.dictionary.browser_events:
|
||||
logger.warning("Event dictionary is empty, skipping batch generation")
|
||||
return {
|
||||
"browser_events": [],
|
||||
"location_events": [],
|
||||
"device_events": [],
|
||||
"geo_events": [],
|
||||
}
|
||||
|
||||
batch = {
|
||||
"browser_events": [],
|
||||
"location_events": [],
|
||||
@@ -338,6 +355,30 @@ class BatchRecord:
|
||||
}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Создание топика для истории
|
||||
# ---------------------------------------------------------------------------
|
||||
def ensure_history_topic(bootstrap_servers: str) -> None:
|
||||
"""Создаёт топик для истории батчей если он не существует."""
|
||||
from kafka import KafkaAdminClient
|
||||
from kafka.admin import NewTopic
|
||||
from kafka.errors import TopicAlreadyExistsError
|
||||
|
||||
admin_client = KafkaAdminClient(bootstrap_servers=bootstrap_servers)
|
||||
try:
|
||||
new_topic = NewTopic(
|
||||
name=KafkaBatchHistory.HISTORY_TOPIC,
|
||||
num_partitions=1,
|
||||
replication_factor=1,
|
||||
)
|
||||
admin_client.create_topics([new_topic])
|
||||
logger.info(f"Created topic: {KafkaBatchHistory.HISTORY_TOPIC}")
|
||||
except TopicAlreadyExistsError:
|
||||
logger.debug(f"Topic already exists: {KafkaBatchHistory.HISTORY_TOPIC}")
|
||||
finally:
|
||||
admin_client.close()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Kafka history - пишет историю в отдельный топик
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -486,6 +527,8 @@ class GeneratorService:
|
||||
f"jitter={self.config.jitter_pct}%")
|
||||
|
||||
# Подключаемся к Kafka для публикации событий и истории
|
||||
# Сначала создаём топик для истории если нужно
|
||||
ensure_history_topic(self.config.kafka_bootstrap_servers)
|
||||
self.publisher = KafkaPublisher(self.config.kafka_bootstrap_servers)
|
||||
self.history = KafkaBatchHistory(self.config.kafka_bootstrap_servers)
|
||||
|
||||
@@ -554,7 +597,12 @@ class GeneratorService:
|
||||
if status in ("success", "partial"):
|
||||
METRICS_LAST_SUCCESS.set_to_current_time()
|
||||
|
||||
# Сохраняем в историю (с fallback на in-memory при деградации Kafka)
|
||||
# Флашим публикацию событий
|
||||
self.publisher.flush()
|
||||
pub_duration = time.time() - pub_start
|
||||
|
||||
# Сохраняем в историю (best-effort: ошибки не валят тик)
|
||||
try:
|
||||
batch_record = BatchRecord(
|
||||
batch_id=batch_id,
|
||||
started_at=datetime.fromtimestamp(tick_start, tz=timezone.utc),
|
||||
@@ -568,11 +616,10 @@ class GeneratorService:
|
||||
error_message=None if status == "success" else f"Errors: {total_errors}",
|
||||
)
|
||||
self.history.add(batch_record)
|
||||
|
||||
# Флашим публикацию и историю
|
||||
self.publisher.flush()
|
||||
self.history.flush()
|
||||
pub_duration = time.time() - pub_start
|
||||
except Exception as hist_err:
|
||||
logger.warning(f"Failed to write batch history: {hist_err}")
|
||||
METRICS_ERRORS_TOTAL.labels(topic="history").inc()
|
||||
|
||||
# Логируем результат
|
||||
tick_duration = time.time() - tick_start
|
||||
@@ -589,6 +636,8 @@ class GeneratorService:
|
||||
|
||||
except Exception as e:
|
||||
logger.exception(f"Error in tick {tick}: {e}")
|
||||
# Пытаемся записать ошибку в историю (best-effort)
|
||||
try:
|
||||
self.history.add(
|
||||
BatchRecord(
|
||||
batch_id=batch_id,
|
||||
@@ -603,6 +652,9 @@ class GeneratorService:
|
||||
error_message=str(e),
|
||||
)
|
||||
)
|
||||
self.history.flush()
|
||||
except Exception as hist_err:
|
||||
logger.warning(f"Failed to write error to history: {hist_err}")
|
||||
|
||||
# Ждём до следующего тика
|
||||
elapsed = time.time() - tick_start
|
||||
|
||||
@@ -168,23 +168,7 @@ class TestPoissonDistribution:
|
||||
class TestEmptyData:
|
||||
"""Тесты обработки пустых данных."""
|
||||
|
||||
def test_empty_jsonl_handled(self, empty_temp_dir):
|
||||
"""Пустые JSONL файлы обрабатываются корректно."""
|
||||
dictionary = EventDictionary.load(empty_temp_dir)
|
||||
|
||||
assert len(dictionary.browser_events) == 0
|
||||
assert len(dictionary.location_events) == 0
|
||||
assert len(dictionary.device_events) == 0
|
||||
assert len(dictionary.geo_events) == 0
|
||||
|
||||
def test_generate_batch_with_empty_dict(self, empty_temp_dir, base_config):
|
||||
"""Генерация с пустым словарем (должна работать, но без событий)."""
|
||||
from dataclasses import replace
|
||||
dictionary = EventDictionary.load(empty_temp_dir)
|
||||
config = replace(base_config, data_dir=empty_temp_dir)
|
||||
generator = EventGenerator(dictionary, config)
|
||||
|
||||
# С пустым словарем генерация упадет при choice()
|
||||
# Это ожидаемое поведение — проверяем что падает с IndexError
|
||||
with pytest.raises(IndexError):
|
||||
generator.generate_batch(1)
|
||||
def test_empty_jsonl_raises_error(self, empty_temp_dir):
|
||||
"""Пустые JSONL файлы вызывают ValueError при загрузке."""
|
||||
with pytest.raises(ValueError, match="browser_events.jsonl is empty"):
|
||||
EventDictionary.load(empty_temp_dir)
|
||||
|
||||
Reference in New Issue
Block a user