From b59e3cd9fece582253a8aa308bd51292cd5caa00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B5=85=E5=BF=B5?= Date: Fri, 7 Aug 2026 16:51:10 +0800 Subject: [PATCH] =?UTF-8?q?chore(sync):=20Task=203=20review=20fixes?= =?UTF-8?q?=E2=80=94=E2=80=94=E5=88=A0=E6=9C=AA=E7=94=A8=20import=20+=20mo?= =?UTF-8?q?dule=20docstring=20=E8=AF=9A=E5=AE=9E=E5=8C=96=20+=20repr(exc)?= =?UTF-8?q?=20=E4=B8=80=E8=87=B4=20+=20=E6=89=B9=E5=A4=84=E7=90=86=20guard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude --- yunke-server/app/sync/orchestrator.py | 15 +++++++++++---- yunke-server/tests/test_sync_call_recordings.py | 1 - 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/yunke-server/app/sync/orchestrator.py b/yunke-server/app/sync/orchestrator.py index affcf00..303a735 100644 --- a/yunke-server/app/sync/orchestrator.py +++ b/yunke-server/app/sync/orchestrator.py @@ -32,7 +32,10 @@ sync continues (the message is still upserted; the watermark still advances). Transactions: the orchestrator never commits. Every write is flushed into the caller's ``AsyncSession``; the caller owns the commit / rollback. This keeps a multi-entity batch atomic at the caller's discretion and lets a test roll a -failed advance back by simply not committing. +failed advance back by simply not committing. Exception: +:func:`sync_call_recordings` commits its own ``SyncRun`` because +:func:`download_call_recordings` commits per batch internally, so the run +bookkeeping must land in the same transaction boundary. """ from __future__ import annotations @@ -609,7 +612,11 @@ async def sync_call_recordings( caller), this one commits its own ``SyncRun`` row: the underlying :func:`download_call_recordings` commits per batch as it advances, so the run bookkeeping must land in the same transaction boundary to stay - consistent with what was actually persisted. + consistent with what was actually persisted. Because it commits internally, + it must NOT be added to a batched runner (:func:`_run_all` / + :func:`sync_all_wechat`) without reconsidering per-batch atomicity — a + batched wrapper can no longer treat the whole entity set as one rollback + unit once this func has already persisted its own rows. Failure policy mirrors every other ``sync_*``: a downloader exception is recorded as a ``status='failed'`` ``SyncRun`` and a failed @@ -634,7 +641,7 @@ async def sync_call_recordings( except Exception as exc: logger.exception("sync_call_recordings failed") await _finish_run( - session, run, status="failed", fetched=0, upserted=0, error=str(exc) + session, run, status="failed", fetched=0, upserted=0, error=repr(exc) ) await session.commit() return SyncResult( @@ -642,7 +649,7 @@ async def sync_call_recordings( status="failed", fetched=0, upserted=0, - error=str(exc), + error=repr(exc), ) diff --git a/yunke-server/tests/test_sync_call_recordings.py b/yunke-server/tests/test_sync_call_recordings.py index 9c59d52..214be26 100644 --- a/yunke-server/tests/test_sync_call_recordings.py +++ b/yunke-server/tests/test_sync_call_recordings.py @@ -29,7 +29,6 @@ from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine from app.db.base import Base from app.db.models import PhoneCall, SyncRun -from app.scraper.media import download_call_recordings from app.session.yunke_client import YunkeClient from app.storage.object_store import ObjectStore from app.sync import orchestrator