From 2cbacb5deabbe39e2cd59bbb11d84aa5006a7e8f Mon Sep 17 00:00:00 2001 From: Adrian Ehrsam Date: Thu, 27 Aug 2026 15:00:08 +0200 Subject: [PATCH] Fix code-review findings from the migrate apply queueing change - Lock tqdm bar.update()/bar.write() calls: both the worker thread and the --ask review loop call into the same tqdm instance, which isn't guaranteed thread-safe without an explicit lock. - Warn (instead of silently treating every file as pending) when the tracking table is missing on apply, matching migrate check's behavior. - Fix the CREATE TABLE regex fallback to stop at '(' as well as whitespace, so `CREATE TABLE foo(id int)` parses as `foo` instead of `foo(id`, which was making apply wrongly report a successful migration as failed verification. Bump version to 0.3.4. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01MsW4bjEgHHQKvmCReYzyNg --- pgdevkit/cli.py | 36 +++++++++++++++++++++++++----------- pgdevkit/migrate.py | 2 +- pyproject.toml | 2 +- uv.lock | 2 +- 4 files changed, 28 insertions(+), 14 deletions(-) diff --git a/pgdevkit/cli.py b/pgdevkit/cli.py index e5906ad..64ca5a1 100644 --- a/pgdevkit/cli.py +++ b/pgdevkit/cli.py @@ -310,6 +310,9 @@ def migrate_apply( try: targets = migrate.pending_migrations(migrations_dir, conninfo, tracking_table) except migrate.TrackingTableMissing: + err_console.print( + f"[yellow]⚠[/yellow] {tracking_table} not found — treating every migration as pending" + ) targets = migrate.list_migration_files(migrations_dir) if not targets: @@ -324,6 +327,17 @@ def migrate_apply( stop = threading.Event() outcomes: list[tuple[str, str]] = [] bar = tqdm(total=len(targets), unit="migration", desc="Applying") + # tqdm isn't guaranteed thread-safe without an explicit lock, and both the worker + # thread and this (the main/--ask) thread call bar.update()/bar.write(). + bar_lock = threading.Lock() + + def bar_write(msg: str) -> None: + with bar_lock: + bar.write(msg) + + def bar_step() -> None: + with bar_lock: + bar.update(1) def worker() -> None: nonlocal failure @@ -334,18 +348,18 @@ def worker() -> None: except Exception as e: # noqa: BLE001 failure = e stop.set() - bar.write(f"FAILED {path.name}: {e}") + bar_write(f"FAILED {path.name}: {e}") outcomes.append((path.name, "failed")) else: if result.executed: for tbl in result.verified_tables: - bar.write(f" table {tbl} exists") - bar.write(f"Applied {path.name}") + bar_write(f" table {tbl} exists") + bar_write(f"Applied {path.name}") outcomes.append((path.name, "applied")) else: - bar.write(f"Recorded {path.name} as already applied (not executed)") + bar_write(f"Recorded {path.name} as already applied (not executed)") outcomes.append((path.name, "recorded")) - bar.update(1) + bar_step() work_q.task_done() thread = threading.Thread(target=worker, daemon=True) @@ -357,23 +371,23 @@ def worker() -> None: break already_done = False if ask: - bar.write(f"\n=== {path.name} ===") - bar.write(path.read_text(encoding="utf-8")) + bar_write(f"\n=== {path.name} ===") + bar_write(path.read_text(encoding="utf-8")) answer = typer.prompt("[Y]es execute / [n]o skip / [a]lready done / [q]uit", default="y").strip().lower() if answer in ("q", "quit"): quit_requested = True break if answer in ("n", "no"): - bar.write(f"Skipped {path.name}") + bar_write(f"Skipped {path.name}") outcomes.append((path.name, "skipped")) - bar.update(1) + bar_step() continue if answer in ("a", "already", "already done"): already_done = True elif answer not in ("", "y", "yes"): - bar.write(f"Skipped {path.name}") + bar_write(f"Skipped {path.name}") outcomes.append((path.name, "skipped")) - bar.update(1) + bar_step() continue work_q.put((path, already_done)) diff --git a/pgdevkit/migrate.py b/pgdevkit/migrate.py index a088238..8ffa181 100644 --- a/pgdevkit/migrate.py +++ b/pgdevkit/migrate.py @@ -191,7 +191,7 @@ def _created_table_names(stmts: list[str]) -> list[str]: names.extend( m.group(1) for m in re.finditer( - r"CREATE\s+TABLE\s+(?:IF\s+NOT\s+EXISTS\s+)?(\S+)", stripped, re.IGNORECASE + r'CREATE\s+TABLE\s+(?:IF\s+NOT\s+EXISTS\s+)?([\w."]+)', stripped, re.IGNORECASE ) ) return names diff --git a/pyproject.toml b/pyproject.toml index f120603..74b126a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -11,7 +11,7 @@ packages = ["pgdevkit"] [project] name = "pgdevkit" -version = "0.3.3" +version = "0.3.4" description = "A helper for developing with Postgres" readme = "README.md" requires-python = ">=3.14" diff --git a/uv.lock b/uv.lock index b113c74..37034a2 100644 --- a/uv.lock +++ b/uv.lock @@ -313,7 +313,7 @@ wheels = [ [[package]] name = "pgdevkit" -version = "0.3.3" +version = "0.3.4" source = { editable = "." } dependencies = [ { name = "docker" },