fix: address Devin review round 3 — retry exhaustion, discover path, WAL snapshot

- CalVer retry loop now exits with error if all 5 attempts fail
  (prevents pushing Docker image with unclaimed version tag)
- discover_tables endpoint reads data_source.keboola.url (consistent
  with configure_instance and _discover_and_register_tables)
- Pre-migration snapshot flushes WAL via CHECKPOINT before copying
  and copies .wal file if it still exists after flush

663 tests pass.
This commit is contained in:
ZdenekSrotyr 2026-04-10 14:11:17 +02:00
parent c79d85f87c
commit dc8a9275e6
3 changed files with 22 additions and 4 deletions

View file

@ -58,6 +58,7 @@ jobs:
# Claim a unique version by pushing a git tag BEFORE building. # Claim a unique version by pushing a git tag BEFORE building.
# Retry up to 5 times if another CI run took our N. # Retry up to 5 times if another CI run took our N.
TAG_CLAIMED=false
for ATTEMPT in 1 2 3 4 5; do for ATTEMPT in 1 2 3 4 5; do
git fetch --tags --force git fetch --tags --force
EXISTING=$(git tag -l "*-${YEAR_MONTH}.*" | wc -l | tr -d ' ') EXISTING=$(git tag -l "*-${YEAR_MONTH}.*" | wc -l | tr -d ' ')
@ -68,14 +69,20 @@ jobs:
git tag -a "$TAG" -m "Release $TAG" git tag -a "$TAG" -m "Release $TAG"
if git push origin "$TAG" 2>/dev/null; then if git push origin "$TAG" 2>/dev/null; then
echo "Claimed tag $TAG (attempt $ATTEMPT)" echo "Claimed tag $TAG (attempt $ATTEMPT)"
TAG_CLAIMED=true
break break
else else
echo "Tag $TAG already exists, retrying... (attempt $ATTEMPT)" echo "Tag $TAG already exists, retrying... (attempt $ATTEMPT)"
git tag -d "$TAG" git tag -d "$TAG"
sleep 1 sleep 2
fi fi
done done
if [ "$TAG_CLAIMED" != "true" ]; then
echo "::error::Failed to claim a unique version tag after 5 attempts"
exit 1
fi
echo "channel=${CHANNEL}" >> "$GITHUB_OUTPUT" echo "channel=${CHANNEL}" >> "$GITHUB_OUTPUT"
echo "version=${VERSION}" >> "$GITHUB_OUTPUT" echo "version=${VERSION}" >> "$GITHUB_OUTPUT"
echo "versioned_tag=${TAG}" >> "$GITHUB_OUTPUT" echo "versioned_tag=${TAG}" >> "$GITHUB_OUTPUT"

View file

@ -65,10 +65,12 @@ async def discover_tables(
if source_type == "keboola": if source_type == "keboola":
from connectors.keboola.client import KeboolaClient from connectors.keboola.client import KeboolaClient
import os
from app.instance_config import get_value from app.instance_config import get_value
url = get_value("keboola", "url", default="") url = get_value("data_source", "keboola", "url", default="")
token = os.environ.get(get_value("keboola", "token_env", default="KEBOOLA_STORAGE_TOKEN"), "") token_env = get_value("data_source", "keboola", "token_env", default="KEBOOLA_STORAGE_TOKEN")
token = os.environ.get(token_env, "") if token_env else ""
if not token:
token = os.environ.get("KEBOOLA_STORAGE_TOKEN", "")
client = KeboolaClient(token=token, url=url) client = KeboolaClient(token=token, url=url)
tables = client.discover_all_tables() tables = client.discover_all_tables()
return {"tables": tables, "count": len(tables), "source": "keboola"} return {"tables": tables, "count": len(tables), "source": "keboola"}

View file

@ -269,8 +269,17 @@ def _ensure_schema(conn: duckdb.DuckDBPyConnection) -> None:
try: try:
db_path = Path(os.environ.get("DATA_DIR", "./data")) / "state" / "system.duckdb" db_path = Path(os.environ.get("DATA_DIR", "./data")) / "state" / "system.duckdb"
if db_path.exists(): if db_path.exists():
# Flush WAL to main DB file before copying
try:
conn.execute("CHECKPOINT")
except Exception:
pass # CHECKPOINT may fail on read-only or in-memory DBs
snapshot = db_path.parent / "system.duckdb.pre-migrate" snapshot = db_path.parent / "system.duckdb.pre-migrate"
shutil.copy2(str(db_path), str(snapshot)) shutil.copy2(str(db_path), str(snapshot))
# Also copy WAL if it still exists (belt and suspenders)
wal_path = Path(str(db_path) + ".wal")
if wal_path.exists():
shutil.copy2(str(wal_path), str(snapshot) + ".wal")
logger.info("Pre-migration snapshot saved: %s", snapshot) logger.info("Pre-migration snapshot saved: %s", snapshot)
except Exception as e: except Exception as e:
logger.warning("Could not create pre-migration snapshot: %s", e) logger.warning("Could not create pre-migration snapshot: %s", e)