fix: address code review feedback
- Add inline safety comment for noqa: S608 (table_name from inspect) - Fix HTTPException detail to be a string (not dict) - Add aria-label to migration progress bar - Rename _noop to _NoOpContextManager in tests - Add explanatory comment for zip(strict=False) Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
+2
-1
@@ -162,8 +162,9 @@ async def execute_migration(body: MigrateRequest, request: Request) -> dict:
|
||||
|
||||
result = migrate_data(body.source_url, body.target_url)
|
||||
if not result["success"]:
|
||||
error_summary = "; ".join(result.get("errors", ["Unknown error"]))
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail={"message": "Migration completed with errors", **result},
|
||||
detail=f"Migration completed with errors: {error_summary}",
|
||||
)
|
||||
return result
|
||||
|
||||
@@ -82,6 +82,7 @@ def preview_migration(source_url: str) -> dict[str, Any]:
|
||||
total = 0
|
||||
with src_engine.connect() as conn:
|
||||
for table_name in tables:
|
||||
# table_name is safe — sourced from inspect().get_table_names(), not user input
|
||||
row = conn.execute(text(f'SELECT COUNT(*) FROM "{table_name}"')).fetchone() # noqa: S608
|
||||
count = row[0] if row else 0
|
||||
result.append({"name": table_name, "row_count": count})
|
||||
@@ -182,6 +183,8 @@ def migrate_data(
|
||||
total_for_table = len(rows)
|
||||
for i in range(0, total_for_table, batch_size):
|
||||
batch = rows[i : i + batch_size]
|
||||
# strict=False: column count should always match, but tolerate
|
||||
# minor schema drift (e.g. extra columns) to avoid crashing mid-migration.
|
||||
insert_data = [dict(zip(column_names, row, strict=False)) for row in batch]
|
||||
tgt_session.execute(tgt_table.insert(), insert_data)
|
||||
tgt_session.commit()
|
||||
|
||||
@@ -411,7 +411,7 @@
|
||||
|
||||
{# Migration progress / result #}
|
||||
<div x-show="migrateLoading" class="mt-4" role="status" aria-live="polite">
|
||||
<div class="w-full bg-gray-200 rounded-full h-3">
|
||||
<div class="w-full bg-gray-200 rounded-full h-3" role="progressbar" aria-label="Migration in progress" aria-valuetext="Migrating data">
|
||||
<div class="bg-indigo-600 h-3 rounded-full animate-pulse-bar" style="width:100%"></div>
|
||||
</div>
|
||||
<p class="text-sm text-gray-600 mt-2">Migration in progress — please do not close this page…</p>
|
||||
|
||||
@@ -31,7 +31,7 @@ class TestDatabaseApiEndpoints:
|
||||
def test_build_url_sqlite(self, client):
|
||||
"""Test building a SQLite URL as admin."""
|
||||
# Simulate admin session
|
||||
with client.session_transaction() if hasattr(client, "session_transaction") else _noop():
|
||||
with client.session_transaction() if hasattr(client, "session_transaction") else _NoOpContextManager():
|
||||
pass
|
||||
# Use the session cookie approach
|
||||
client.cookies.set("session", "test")
|
||||
@@ -148,7 +148,9 @@ class TestDatabaseWizardView:
|
||||
|
||||
|
||||
# Context manager helper for tests that don't need session_transaction
|
||||
class _noop:
|
||||
class _NoOpContextManager:
|
||||
"""Dummy context manager for tests that don't need session_transaction."""
|
||||
|
||||
def __enter__(self):
|
||||
return None
|
||||
|
||||
|
||||
Reference in New Issue
Block a user