Skip to content

feat: Textual TUI + реструктуризація пакету olap_tool/ - #5

Closed
starychenko wants to merge 28 commits into
mainfrom
feature/tui-restructure
Closed

feat: Textual TUI + реструктуризація пакету olap_tool/#5
starychenko wants to merge 28 commits into
mainfrom
feature/tui-restructure

Conversation

@starychenko

Copy link
Copy Markdown
Owner

Summary

  • Реструктуризація пакету: плаский olap_tool/ розбито на підпакети core/, connection/, data/, sinks/ — чіткі межі відповідальності, оновлені всі відносні імпорти
  • Повноцінний Textual TUI: python olap.py без аргументів запускає інтерактивне меню з екранами для OLAP-експорту та XLSX-імпорту; прогрес виводиться прямо у TUI через TUIStream
  • Об'єднаний batch-скрипт: import_xlsx_to_clickhouse.py + import_xlsx_to_duckdb.pyscripts/import_xlsx.py --target ch|duck|pg; ClickHouse та PostgreSQL — thread-local sinks

Changes

Нові файли

  • olap_tool/sinks/base.pyAnalyticsSink ABC + sanitize_df()
  • olap_tool/sinks/clickhouse.py — поглинає clickhouse_export.py
  • olap_tool/sinks/duckdb.py, postgresql.py
  • olap_tool/core/, olap_tool/connection/, olap_tool/data/ — переміщені модулі
  • olap_tool/tui/app.py + screens/main_menu.py, olap_export.py, xlsx_import.py
  • scripts/import_xlsx.py

Видалені файли

  • olap_tool/sinks.py, olap_tool/clickhouse_export.py (поглинуті)
  • import_xlsx_to_clickhouse.py, import_xlsx_to_duckdb.py (замінені)
  • Всі пласкі модулі olap_tool/*.py (переміщені в підпакети)

Test Plan

  • python -c "from olap_tool import main; from olap_tool.tui.app import OlapApp; print('OK')" — всі імпорти
  • python olap.py --help — CLI режим без змін
  • python olap.py — запускається Textual TUI (q для виходу)
  • python scripts/import_xlsx.py --help — виводить usage з --target ch|duck|pg

🤖 Generated with Claude Code

starychenko and others added 11 commits March 10, 2026 11:07
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Об'єднує ClickHouseSink з sinks.py та всю допоміжну логіку з
clickhouse_export.py в один модуль olap_tool/sinks/clickhouse.py.
Імпортує AnalyticsSink та sanitize_df з .base, ClickHouseConfig з ..config.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…it__.py

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Старі плоскі файли замінені пакетом sinks/. Імпорт у
import_xlsx_to_clickhouse.py оновлено на olap_tool.sinks.clickhouse.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Переміщено плоскі модулі в тематичні субпакети:
- core/: config, cli, runner, periods, profiles, scheduler, compression, progress, utils
- connection/: connection, auth, security, prompt
- data/: queries, exporter

Оновлено крос-пакетні імпорти та sinks/*.py (..config → ..core.config,
..utils → ..core.utils). Оновлено olap_tool/__init__.py і кореневий olap.py.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Замінює два окремі скрипти (import_xlsx_to_clickhouse.py,
import_xlsx_to_duckdb.py) єдиним scripts/import_xlsx.py з аргументом
--target ch|duck|pg. ClickHouse використовує thread-local sinks,
DuckDB і PostgreSQL — один спільний sink на весь запуск.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ити olap.py

- Додано textual>=0.70.0 до requirements.txt (виправлено також злиплий рядок rich+pyarrow)
- TUIStream у utils.py: перехоплює stdout, очищує ANSI, пише у RichLog потокобезпечно
- olap_tool/tui/: OlapApp + MainMenuScreen + OlapExportScreen + XlsxImportScreen
- olap.py: без аргументів → TUI, з аргументами → CLI (sys.exit(main()))

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- TUIStream.flush() тепер дренує _buf замість no-op, щоб не втрачати останній рядок
- PostgreSQLSink у scripts/import_xlsx.py переведено на thread-local патерн (_pg_local/_get_pg_sink/_process_pg), аналогічно ClickHouse; оновлено коментарі
- _list_profiles() прив'язано до Path(__file__) замість CWD, щоб profiles/ знаходились незалежно від робочої директорії

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ля reconfigure, assert для cfg narrowing

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@claude

claude Bot commented Mar 10, 2026

Copy link
Copy Markdown

Claude finished @starychenko's task in 4m 36s —— View job


Огляд PR #5 — feat: Textual TUI + реструктуризація пакету olap_tool/

  • Ознайомлення з контекстом PR
  • Читання ключових нових файлів (sinks/base.py, sinks/clickhouse.py, sinks/duckdb.py, sinks/postgresql.py)
  • Читання TUI файлів (tui/app.py, screens/*)
  • Читання scripts/import_xlsx.py
  • Читання змінених файлів (olap.py, runner.py, utils.py, config.py)
  • Аналіз архітектурних рішень
  • Публікація фінального ревʼю

Вердикт: ❌ Request Changes

PR реструктурує пакет коректно і TUI виглядає логічно, але є один критичний баг у thread-local ініціалізації sink-ів у scripts/import_xlsx.py — ClickHouse та PostgreSQL воркери впадуть з AttributeError на кожному файлі. Крім того, кілька важливих проблем потребують уваги.


[BLOCKER] Thread-local CH/PG sink-и ніколи не викликають setup() → crash на delete_period()

scripts/import_xlsx.py:78–88 та 100–110

def _get_ch_sink(cfg_kwargs: dict):
    if not hasattr(_ch_local, "sink") or _ch_local.sink is None:
        sink = ClickHouseSink(ClickHouseConfig(**cfg_kwargs))
        # setup вже викликаний у main() з першим df — тут не викликаємо  ← помилка
        _ch_local.sink = sink
    return _ch_local.sink

ClickHouseSink.__init__ встановлює self._client = None. setup() не викликається — отже self._client залишається None. Коли воркер-потік робить перший виклик:

sink.delete_period(year, week)
# → _delete_period(self._client=None, ...)
# → get_table_schema(None, database, table)
# → None.query(...)  ← AttributeError

Кожен ClickHouse і PostgreSQL воркер-потік впаде на першому файлі. Те саме для _get_pg_sink()PostgreSQLSink теж потребує _get_conn() який не ініціалізується без setup() або явного _get_conn().

Виправлення: після створення thread-local sink потрібно або викликати sink.setup(df) (CREATE TABLE IF NOT EXISTS — ідемпотентна операція), або ввести окремий метод connect() що не робить DDL, але ініціалізує _client.

Fix this →


[IMPORTANT] asyncio.get_event_loop() deprecated у Python 3.10+

olap_tool/tui/screens/olap_export.py:137 та xlsx_import.py:94

async def _do_export(self, argv: list[str]) -> None:
    loop = asyncio.get_event_loop()  # deprecated — DeprecationWarning у 3.10+
    await loop.run_in_executor(None, self._run_export_sync, argv)

У async-контексті треба asyncio.get_running_loop(), який повертає поточний event loop гарантовано.

Fix this →


[IMPORTANT] self.query_one() викликається всередині run_in_executor thread

olap_tool/tui/screens/olap_export.py:143 та xlsx_import.py:98

def _run_export_sync(self, argv: list[str]) -> None:
    log = self.query_one("#export-log", RichLog)  # ← виклик DOM з executor thread!
    stream = TUIStream(self.app, log)
    ...

_run_export_sync запускається через loop.run_in_executor(None, ...) — це звичайний ThreadPoolExecutor, не Textual worker. Textual гарантує безпеку DOM-операцій лише з основного потоку або через call_from_thread. Читання через query_one з executor thread може призвести до гонки.

Виправлення: отримати log у _start_export() (на основному потоці) і передати посилання до _do_export:

def _start_export(self) -> None:
    log = self.query_one("#export-log", RichLog)  # отримуємо на main thread
    ...
    self._worker = self.run_worker(self._do_export(argv, log), ...)

async def _do_export(self, argv, log) -> None:
    loop = asyncio.get_running_loop()
    await loop.run_in_executor(None, self._run_export_sync, argv, log)

Fix this →


[IMPORTANT] DuckDBSink._refresh_schema() без захисту від невалідної відповіді API

olap_tool/sinks/duckdb.py:237–242

def _refresh_schema(self) -> None:
    result = self._query(f'DESCRIBE "{self._config.table}"')
    col_idx = result["columns"].index("column_name")  # KeyError якщо API повернув інше
    type_idx = result["columns"].index("column_type")  # ValueError якщо ключа немає
    with self._schema_lock:
        self._schema = {row[col_idx]: row[type_idx] for row in result["rows"]}

Якщо REST API поверне відповідь у нестандартному форматі або зміниться формат DESCRIBE, буде KeyError/ValueError без жодного контексту. Треба додати try/except з інформативним повідомленням.


[NIT] _duck_value() визначена але не використовується

olap_tool/sinks/duckdb.py:107–141

Функція _duck_value() серіалізує Python-значення до SQL-літералів, але вся вставка відбувається через Parquet (_upload_parquet). Мертвий код. Якщо _duck_value — залишок старої реалізації через VALUES-рядки, її варто видалити.


Що зроблено добре

  • AnalyticsSink контракт повністю реалізований у всіх трьох нових sink-ах (setup, delete_period, insert, close) ✓
  • sanitize_df в одному місці (sinks/base.py), імпортується всюди — жодного дублювання ✓
  • DELETE before INSERT у всіх sink-ах забезпечує ідемпотентність ✓
  • З'єднання закриваються в finally — і в runner.py, і в кожному sink ✓
  • Lightweight DELETE у ClickHouse (22.8+) замість мутацій — правильне рішення ✓
  • Thread-local pattern для CH/PG і shared singleton для DuckDB — правильна архітектура (лише ініціалізація зламана) ✓
  • TYPE_CHECKING для уникнення циклічних імпортів — акуратно ✓

- BLOCKER: thread-local CH/PG sinks тепер викликають setup() при створенні
  (зберігаємо _ch_setup_df / _pg_setup_df під час ініціалізації)
- asyncio.get_event_loop() → get_running_loop() в TUI screens
- query_one() виноситься на головний потік, передається як аргумент
- DuckDBSink._refresh_schema(): try/except з інформативним RuntimeError
- _duck_value() видалено (мертвий код — вставка через Parquet, не VALUES)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude

claude Bot commented Mar 10, 2026

Copy link
Copy Markdown

Claude finished @starychenko's task in 3m 8s —— View job


Повторний огляд PR #5 — всі виправлення перевірено

  • Читання оновлених файлів після виправлень
  • Перевірка [BLOCKER]: ініціалізація thread-local sink-ів
  • Перевірка [IMPORTANT]: asyncio.get_event_loop()
  • Перевірка [IMPORTANT]: query_one() з executor thread
  • Перевірка [IMPORTANT]: DuckDBSink._refresh_schema()
  • Перевірка [NIT]: _duck_value() мертвий код
  • Публікація фінального ревʼю

Вердикт: ✅ Approve

Всі 5 проблем з попереднього ревʼю усунуто. Один залишковий edge-case задокументовано нижче — не є blocker-ом.


Статус виправлень

Попередня проблема Статус
[BLOCKER] Thread-local CH/PG sink не викликав setup()AttributeError ✅ Виправлено — _ch_setup_df/_pg_setup_df зберігається в main() і передається у _get_ch_sink()/_get_pg_sink()
[IMPORTANT] asyncio.get_event_loop() deprecated у 3.10+ ✅ Виправлено — get_running_loop() в olap_export.py:138 та xlsx_import.py:95
[IMPORTANT] query_one() з executor thread ✅ Виправлено — log отримується на головному потоці у _start_export()/_start_import() та передається як аргумент
[IMPORTANT] DuckDBSink._refresh_schema() без захисту від невалідної відповіді ✅ Виправлено — try/except (KeyError, ValueError) з інформативним RuntimeError (duckdb.py:202-209)
[NIT] _duck_value() — мертвий код ✅ Видалено

[IMPORTANT] Залишковий edge-case: перший файл порожній → thread-local CH sink ініціалізується без setup()

scripts/import_xlsx.py:391-401 та scripts/import_xlsx.py:85-94

Якщо перший XLSX файл (files[0]) має порожній DataFrame, блок ініціалізації пропускає setup() і _ch_setup_df залишається None:

if not df_init.empty:          # ← якщо порожній — пропускаємо setup()
    ...
    init_sink.setup(df_init_clean)
    global _ch_setup_df
    _ch_setup_df = df_init_clean   # ← None якщо df порожній

Тоді _get_ch_sink() (рядок 89) не викликає setup() і self._client залишається None. Виклик delete_period() у воркер-потоці → _delete_period(None, ...)None.query(...)AttributeError. Ця помилка перехоплюється except Exception у _process_ch (рядок 194), тому програма не падає, але всі файли цього потоку повертають (0, False) — тихий збій.

Для PostgreSQL наслідки менш критичні: _get_conn() у PostgreSQLSink є lazy (рядок 66-78), тому delete_period() підключиться, знайде порожню схему (таблиця не існує) і поверне достроково. COPY потім впаде з "table does not exist", але це вже явна помилка.

Виправлення для ClickHouse: можна або викликати _get_ch_sink() у головному потоці для першого файлу (гарантуючи, що перший воркер правильно ініціалізований), або у ClickHouseSink.delete_period() перевіряти self._client is None і кидати зрозуміле повідомлення.

Fix this →


Що зроблено добре

  • Підхід _ch_setup_df/_pg_setup_df елегантний — ідемпотентний CREATE TABLE IF NOT EXISTS в головному потоці, thread-local sinks отримують готову схему без зайвих DDL-запитів ✓
  • PostgreSQLSink._get_conn() — lazy init — навіть без setup() не падає з AttributeError, а видає зрозумілу помилку на COPY ✓
  • Видалення _duck_value() — чисто, без залишків ✓

Шукаємо перший непорожній файл для ініціалізації sink замість завжди
брати files[0]. Без цього thread-local CH/PG sinks не отримували б setup_df
і _client залишався None → тихий AttributeError у воркер-потоці.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude

claude Bot commented Mar 10, 2026

Copy link
Copy Markdown

Claude finished @starychenko's task in 1m 5s —— View job


Claude finished @starychenko's task —— View job


Перевірка edge-case PR #5 — перший XLSX порожній

  • Ознайомлення з контекстом PR та останніми комітами
  • Читання оновленого scripts/import_xlsx.py — перевірка виправлення edge-case
  • Публікація фінального ревʼю

Вердикт: ✅ Approve

Виправлення edge-case коректне та повне.


Аналіз виправлення (scripts/import_xlsx.py:380–406)

Було (проблема): читався лише перший файл (files[0]); якщо він порожній — _ch_setup_df залишалась None.

Стало (виправлення):

df_init = pd.DataFrame()
init_file_idx = 0
for _i, (_fp, _y, _w) in enumerate(files):
    df_init = _read_excel(_fp, sheet)
    if not df_init.empty:
        init_file_idx = _i
        break

Ітерується по всіх файлах до першого непорожнього — підхід правильний.

Перевірка крайніх випадків:

Сценарій Поведінка
Перший файл порожній, решта — ні init_file_idx вказує на перший непорожній; _ch_setup_df встановлюється коректно ✓
Перші N файлів порожні Те ж саме — цикл знаходить перший непорожній ✓
Всі файли порожні _ch_setup_df = None, але воркери повертають достроково (рядок 181–182: if df.empty: return 0, True, ...), тому _get_ch_sink() ніколи не викликається → без краша ✓

[NIT] Файл, знайдений під час init-циклу, буде прочитаний вдруге у воркер-потоці. Для типового сценарію (мало порожніх файлів) це незначно, але варто знати при великій кількості порожніх файлів на початку.


Загальний статус всіх виправлень

Проблема Статус
[BLOCKER] Thread-local CH/PG sink без setup()AttributeError ✅ Виправлено
[IMPORTANT] asyncio.get_event_loop() deprecated ✅ Виправлено
[IMPORTANT] query_one() з executor thread ✅ Виправлено
[IMPORTANT] DuckDBSink._refresh_schema() без захисту ✅ Виправлено
[NIT] _duck_value() — мертвий код ✅ Видалено
[IMPORTANT] Edge-case: перший XLSX порожній ✅ Виправлено

@claude

claude Bot commented Mar 10, 2026

Copy link
Copy Markdown

Claude finished @starychenko's task in 1m 9s —— View job


Claude finished @starychenko's task ——


Перевірка нового коміту PR #5

  • Перегляд нових комітів після останнього ревʼю
  • Читання змінених файлів
  • Аналіз змін
  • Публікація фінального ревʼю

Вердикт: ✅ Approve

Новий коміт 11146ca — виключно косметичний.


Аналіз коміту

scripts/import_xlsx.py — прибрано невалідні return type annotations:

# Було:
def _get_ch_sink(cfg_kwargs: dict) -> "ClickHouseSink":
def _get_pg_sink(cfg_kwargs: dict) -> "PostgreSQLSink":

# Стало:
def _get_ch_sink(cfg_kwargs: dict):
def _get_pg_sink(cfg_kwargs: dict):

Виправлення коректне. ClickHouseSink та PostgreSQLSink імпортуються лінощено (всередині тіла функції), тому не доступні на рівні модуля навіть як forward reference у рядках. Прибрати анотацію — правильне рішення. Альтернативою було б додати TYPE_CHECKING імпорт на рівні модуля, але лінощий імпорт тут доцільніший (уникає кругових залежностей і зайвого часу завантаження).


Загальний статус всіх виправлень PR #5

Проблема Статус
[BLOCKER] Thread-local CH/PG sink без setup()AttributeError ✅ Виправлено
[IMPORTANT] asyncio.get_event_loop() deprecated ✅ Виправлено
[IMPORTANT] query_one() з executor thread ✅ Виправлено
[IMPORTANT] DuckDBSink._refresh_schema() без захисту ✅ Виправлено
[NIT] _duck_value() — мертвий код ✅ Видалено
[IMPORTANT] Edge-case: перший XLSX порожній ✅ Виправлено
[NIT] Невалідні return type annotations ✅ Виправлено (цей коміт)

PR готовий до мержу.

Textual Select очікує (label, value), а не (value, label).
Некоректний порядок призводив до InvalidSelectValueError при запуску.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude

claude Bot commented Mar 10, 2026

Copy link
Copy Markdown

Claude finished @starychenko's task in 55s —— View job


Claude finished @starychenko's task —— View job


Перевірка нового коміту PR #5

  • Перегляд нового коміту 8e6878e
  • Читання змінених файлів
  • Аналіз змін
  • Публікація фінального ревʼю

Вердикт: ✅ Approve

Новий коміт 8e6878e — коректний критичний bugfix.


Аналіз коміту

olap_tool/tui/screens/olap_export.py — виправлено порядок кортежів у всіх Select-опціях:

# Було (неправильно — Textual отримував value там де очікував label):
FORMAT_OPTIONS = [
    ("xlsx", "XLSX"),   # (value, label) ← InvalidSelectValueError
    ...
]

# Стало (правильно):
FORMAT_OPTIONS = [
    ("XLSX", "xlsx"),   # (label, value) ← відповідає Textual API
    ...
]

Виправлення коректне — Textual Select конструктор очікує Iterable[tuple[str, SelectType]] де перший елемент — людиночитаний label, другий — програмне значення. Некоректний порядок призводив до InvalidSelectValueError при спробі встановити value="xlsx", бо серед label-ів не було "xlsx".

Охоплені всі три константи (FORMAT_OPTIONS, PERIOD_OPTIONS, COMPRESS_OPTIONS) та fallback-опція для порожнього списку профілів.


Загальний статус всіх виправлень PR #5

Проблема Статус
[BLOCKER] Thread-local CH/PG sink без setup()AttributeError ✅ Виправлено
[IMPORTANT] asyncio.get_event_loop() deprecated ✅ Виправлено
[IMPORTANT] query_one() з executor thread ✅ Виправлено
[IMPORTANT] DuckDBSink._refresh_schema() без захисту ✅ Виправлено
[NIT] _duck_value() — мертвий код ✅ Видалено
[IMPORTANT] Edge-case: перший XLSX порожній ✅ Виправлено
[NIT] Невалідні return type annotations ✅ Виправлено
[BLOCKER] InvalidSelectValueError у TUI Select ✅ Виправлено (цей коміт)

PR готовий до мержу.

View job

…ls, alongside connection authentication and prompting utilities.
@claude

claude Bot commented Mar 11, 2026

Copy link
Copy Markdown

Claude finished @starychenko's task in 3m 13s —— View job


Огляд нового коміту PR #5

  • Ознайомлення з контекстом PR та останніми комітами
  • Читання нових файлів: credentials.py, prompt.py, auth.py
  • Читання оновлених olap_export.py, xlsx_import.py, app.py, utils.py, olap.py
  • Аналіз архітектурних рішень нового коміту
  • Публікація фінального ревʼю

Вердикт: ❌ Request Changes

Коміт 29633ce додає CredentialsDialog, prompt.py і оновлює auth.py. Є один важливий баг із зависанням потоку і продубльований код у auth.py.


[IMPORTANT] event.wait() без таймауту → потік зависає назавжди

olap_tool/connection/prompt.py:30 та olap_tool/connection/auth.py:104

app.call_from_thread(show_dialog)
event.wait()   # ← без таймауту

_run_export_sync виконується в ThreadPoolExecutor. Якщо TUI закривається під час очікування credentials (Ctrl+C, kill сигнал, або виняток у show_dialog до event.set()), event ніколи не буде встановлено — executor-поток зависне назавжди.

Зверніть увагу: _worker.cancel() скасовує Textual worker coroutine, але не перериває заблокований executor-поток. Процес не завершиться коректно.

Виправлення:

app.call_from_thread(show_dialog)
if not event.wait(timeout=60.0):    # або інший розумний таймаут
    raise RuntimeError("Таймаут очікування облікових даних")

Також у show_dialog потрібен event.set() у гілці except коли push_screen не викликається:

def show_dialog():
    try:
        ...
        app.push_screen(CredentialsDialog(...), cb)
    except Exception as e:
        print_info(f"Помилка виклику TUI діалогу: {e}")
        event.set()   # ← це вже є в prompt.py, але відсутнє в auth.py:103

У auth.py:102-103 є except Exception: event.set() — добре. Але відсутній таймаут в обох місцях.

Fix this →


[IMPORTANT] Дубльована TUI-логіка для майстер-пароля в auth.py

olap_tool/connection/auth.py:79–116

~30 рядків TUI-виявлення + call_from_thread + event.wait() скопійовано з prompt.py прямо всередину load_credentials(). Це порушує SRP і ускладнює підтримку: якщо логіка виявлення TUI зміниться, потрібно оновлювати два місця.

Функція prompt_credentials у prompt.py вже підтримує ask_login=False через CredentialsDialog. Можна перевикористати:

# Замість 30 рядків TUI-логіки в load_credentials():
from .prompt import prompt_credentials
_, mp_retry = prompt_credentials(with_domain=False)

Або винести визначення TUI-режиму в окрему функцію _get_tui_app() -> App | None в utils.py.

Fix this →


[NIT] hasattr(sys, "stdout") завжди True

olap_tool/connection/prompt.py:10

if hasattr(sys, "stdout") and hasattr(sys.stdout, "_app"):

sys.stdout завжди існує — перша перевірка зайва. Достатньо:

if sys.stdout is not None and hasattr(sys.stdout, "_app"):

[NIT] import sys всередині функції

olap_tool/connection/prompt.py:9

sys — стандартна бібліотека, завжди доступна. Краще виносити на рівень модуля як решта імпортів.


[NIT] Enter у формі CredentialsDialog не підтверджує

olap_tool/tui/screens/credentials.py

Зазвичай Enter у фінальному Input форми підтверджує введення. Можна додати:

def on_input_submitted(self, event: Input.Submitted) -> None:
    self.on_button_pressed(Button.Pressed(self.query_one("#ok-btn", Button)))

Що зроблено добре

  • TUIStream + call_from_thread — коректний потокобезпечний підхід для перехоплення print() у TUI-режимі ✓
  • CredentialsDialog як ModalScreen[tuple[str, str] | None] — типізований результат, акуратна реалізація dismiss ✓

Загальний статус виправлень PR #5

Проблема Статус
[BLOCKER] Thread-local CH/PG sink без setup() ✅ Виправлено
[IMPORTANT] asyncio.get_event_loop() deprecated ✅ Виправлено
[IMPORTANT] query_one() з executor thread ✅ Виправлено
[IMPORTANT] DuckDBSink._refresh_schema() без захисту ✅ Виправлено
[NIT] _duck_value() — мертвий код ✅ Видалено
[IMPORTANT] Edge-case: перший XLSX порожній ✅ Виправлено
[NIT] Невалідні return type annotations ✅ Виправлено
[BLOCKER] InvalidSelectValueError у TUI Select ✅ Виправлено
[IMPORTANT] event.wait() без таймауту → зависання ❌ Потребує виправлення
[IMPORTANT] Дубльована TUI-логіка у auth.py ❌ Потребує виправлення

…CSV/XLSX streaming, connection management, and progress display.
@claude

claude Bot commented Mar 11, 2026

Copy link
Copy Markdown

Claude finished @starychenko's task in 3m 23s —— View job


Claude finished @starychenko's task in 3m 13s —— View job


Огляд коміту a9a5b98 PR #5

  • Ознайомлення з контекстом PR та останніми комітами
  • Читання оновлених файлів (auth.py, prompt.py, utils.py, exporter.py, queries.py, connection.py, TUI screens)
  • Аналіз змін
  • Публікація фінального ревʼю

Вердикт: ❌ Request Changes

Коміт a9a5b98 вносить корисні покращення: TUI статус-рядок, LoadingIndicator, динамічне приховання поля введення, новий UX для CredentialsDialog, рефакторинг exporter.py з CsvStreamWriter/XlsxStreamWriter, чанковий стрімінг через fetchmany. Але 2 попередні [IMPORTANT] залишилися невиправленими, і додалась 1 нова [IMPORTANT].


Невиправлені проблеми з попереднього ревʼю

[IMPORTANT] event.wait() без таймауту — executor-поток зависає назавжди

olap_tool/connection/prompt.py:30 та olap_tool/connection/auth.py:114

# prompt.py:30
app.call_from_thread(show_dialog)
event.wait()   # ← без таймауту, як і раніше
# auth.py:114
app.call_from_thread(show_mp_dialog)
event.wait()   # ← без таймауту, як і раніше

Якщо TUI закривається (Ctrl+C, crash) поки executor-поток чекає на credentials — потік зависне назавжди і процес не завершиться.

Fix this →


[IMPORTANT] Дубльована TUI-логіка для майстер-пароля в auth.py

olap_tool/connection/auth.py:97–120

~25 рядків TUI-виявлення + call_from_thread + event.wait() залишаються продубльованими всередині load_credentials() — окремо від аналогічного механізму в prompt.py. Зміни до однієї гілки не поширяться на іншу.

Fix this →


Нова проблема в a9a5b98

[IMPORTANT] XlsxStreamWriter відкриває workbook у __init__, але не закривається при винятку → corrupt XLSX

olap_tool/data/exporter.py:54 та olap_tool/data/queries.py (блок except)

# exporter.py:54
self.workbook = xlsxwriter.Workbook(self.file_path_str, {"constant_memory": True})
# ↑ файл відкрито тут

# queries.py — блок обробки помилок:
except Exception as e:
    print_error(f"Помилка при виконанні запиту: {e}")
    return None
    # ↑ xlsx_writer.close() НІКОЛИ НЕ ВИКЛИКАЄТЬСЯ

xlsxwriter з constant_memory=True стрімить дані одразу на диск. Якщо cursor.fetchmany() або будь-який chunk-запис кидає виняток, close() не викликається — файл залишається незакінченим і corrupt.

Виправлення: обгорнути цикл у try/finally:

try:
    ...chunk loop...
finally:
    if xlsx_writer:
        try:
            xlsx_writer.close()
        except Exception:
            pass

Або XlsxStreamWriter повинен реалізувати __enter__/__exit__ і використовуватись як контекст-менеджер.

Fix this →


[NIT] hasattr(sys, "stdout") завжди True — невиправлено

olap_tool/connection/prompt.py:10

if hasattr(sys, "stdout") and hasattr(sys.stdout, "_app"):

sys.stdout завжди існує. Перша перевірка зайва. Достатньо hasattr(sys.stdout, "_app").


Що зроблено добре в цьому коміті

  • TUIStream з окремим status_widget — правильне розмежування "прогрес (\r)" і "журнал (\n)" ✓
  • _is_auth_error() — явна класифікація мережевих vs auth-помилок з безпечним дефолтом (return False) ✓
  • OleDbCursor.fetchmany() — нарешті є реалізація, потрібна для чанкового стрімінгу ✓
  • on_input_submitted в CredentialsDialog — NIT з попереднього ревʼю виправлено ✓
  • Динамічне приховання period-value-input залежно від типу — гарне UX-покращення ✓

Загальний статус виправлень PR #5

Проблема Статус
[BLOCKER] Thread-local CH/PG sink без setup() ✅ Виправлено
[IMPORTANT] asyncio.get_event_loop() deprecated ✅ Виправлено
[IMPORTANT] query_one() з executor thread ✅ Виправлено
[IMPORTANT] DuckDBSink._refresh_schema() без захисту ✅ Виправлено
[NIT] _duck_value() — мертвий код ✅ Видалено
[IMPORTANT] Edge-case: перший XLSX порожній ✅ Виправлено
[NIT] Невалідні return type annotations ✅ Виправлено
[BLOCKER] InvalidSelectValueError у TUI Select ✅ Виправлено
[NIT] Enter у формі CredentialsDialog не підтверджує ✅ Виправлено
[IMPORTANT] event.wait() без таймауту → зависання ❌ Не виправлено
[IMPORTANT] Дубльована TUI-логіка у auth.py ❌ Не виправлено
[IMPORTANT] XlsxStreamWriter не закривається при винятку ❌ Нова проблема

@claude

claude Bot commented Mar 16, 2026

Copy link
Copy Markdown

Claude finished @starychenko's task in 2m 13s —— View job


Огляд нових комітів PR #5

  • Ознайомлення з останніми комітами
  • Читання оновлених файлів (prompt.py, auth.py, queries.py, exporter.py, ui/)
  • Перевірка 3 невиправлених [IMPORTANT] з попереднього ревʼю
  • Публікація фінального ревʼю

Вердикт: ❌ Request Changes

Коміти b6e3dd81b14de5e267ddecc72227 замінюють Textual TUI на InquirerPy + rich wizards. Це усуває 2 з 3 невиправлених [IMPORTANT]. Але XlsxStreamWriter не закривається при винятку залишилося незакритим.


Статус виправлень з попереднього ревʼю

Проблема Статус
[IMPORTANT] event.wait() без таймауту → зависання executor-потоку ✅ Усунуто — Textual повністю видалено, TUI-логіка в auth.py/prompt.py прибрана
[IMPORTANT] Дубльована TUI-логіка у auth.py ✅ Усунуто — разом з видаленням Textual
[IMPORTANT] XlsxStreamWriter не закривається при винятку ❌ Не виправлено

[IMPORTANT] XlsxStreamWriter.close() не викликається при винятку → corrupt XLSX

olap_tool/data/queries.py:192–332

xlsx_writer відкриває workbook у __init__ (рядок 54 exporter.py). Виклики close() знаходяться всередині try-блоку (рядки 308–312). Блок finally (333–345) закриває лише cursor і spinner_thread — але не xlsx_writer і не csv_writer.

try:
    xlsx_writer = XlsxStreamWriter(...)   # rядок 198 — workbook відкрито

    for row in cursor.fetchone():         # рядки 253–303
        xlsx_writer.write_chunk(...)      # якщо тут виникне виняток ↓

    # рядки 308–312 — close() лише при нормальному завершенні
    _, file_size_bytes = xlsx_writer.close()

except Exception as e:
    print_error(...)
    return None                           # ← xlsx_writer.close() ніколи не викликається

finally:
    cursor.close()
    spinner_thread.join()
    # xlsx_writer — не закритий!

xlsxwriter з constant_memory=True стрімить дані на диск. Незакритий workbook залишає файл без EOCD-запису — Excel не може його відкрити.

Виправлення: додати до finally:

finally:
    if cursor is not None:
        try: cursor.close()
        except Exception: pass
    if xlsx_writer is not None:
        try: xlsx_writer.workbook.close()
        except Exception: pass
    if spinner_thread is not None:
        ...

Або XlsxStreamWriter.__enter__/__exit__ + with XlsxStreamWriter(...) as w: у run_dax_query.

Fix this →


[NIT] _show_summary() продубльована в двох wizard-файлах

olap_tool/ui/olap_export.py:54–62 та olap_tool/ui/xlsx_import.py:64–73

Ідентична функція _show_summary(params: dict[str, str]) визначена двічі. Варто винести в olap_tool/ui/__init__.py або окремий ui/utils.py.


[NIT] menu.py читає .env поза AppConfig-потоком

olap_tool/ui/menu.py:17–20

env = dotenv_values(Path(...) / ".env")
server = env.get("OLAP_SERVER") or env.get("SERVER") or "—"
auth = env.get("OLAP_AUTH_METHOD") or env.get("AUTH_METHOD") or "—"

Значення для відображення заголовку беруться напряму з .env, обходячи AppConfig. Це лише cosmetic — але якщо config.yaml колись перекриватиме ці ключі, заголовок буде показувати застарілі значення. Можна прийняти як є для display-only коду.


Що зроблено добре в нових комітах

  • Заміна Textual на InquirerPy — рішення усуває цілу категорію thread-safety проблем (executor threads, call_from_thread, event.wait()). Wizards виглядають чисто і читабельно.
  • sys.argv відновлюється через finally у обох wizards — коректне управління процесом.
  • scripts/import_xlsx.py має if __name__ == "__main__" guardimportlib-завантаження у xlsx_import.py не викликає main() двічі.

Загальний статус всіх виправлень PR #5

Проблема Статус
[BLOCKER] Thread-local CH/PG sink без setup() ✅ Виправлено
[IMPORTANT] asyncio.get_event_loop() deprecated ✅ Усунуто (Textual видалено)
[IMPORTANT] query_one() з executor thread ✅ Усунуто (Textual видалено)
[IMPORTANT] DuckDBSink._refresh_schema() без захисту ✅ Виправлено
[NIT] _duck_value() — мертвий код ✅ Видалено
[IMPORTANT] Edge-case: перший XLSX порожній ✅ Виправлено
[NIT] Невалідні return type annotations ✅ Виправлено
[BLOCKER] InvalidSelectValueError у TUI Select ✅ Усунуто (Textual видалено)
[NIT] Enter у формі CredentialsDialog не підтверджує ✅ Усунуто (Textual видалено)
[IMPORTANT] event.wait() без таймауту → зависання ✅ Усунуто (Textual видалено)
[IMPORTANT] Дубльована TUI-логіка у auth.py ✅ Усунуто (Textual видалено)
[IMPORTANT] XlsxStreamWriter не закривається при винятку ❌ Не виправлено

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant