Run auto-reconcile as a scan phase inside the running window
The post-scan reconcile previously ran AFTER the worker's 'finished'
signal, which flips db_update_state status to 'finished'. Automations
wait for a scan by polling that status, so they stopped waiting before
the reconcile ran — and the dashboard/Tools card showed "Completed" then
flipped to "Reading file tags…". For incremental scans this was
invisible (sub-second); for a full refresh it was a real gap (a chained
automation would fire minutes before the IDs were filled).
Fix: the worker now routes completion through _emit_finished(), which
runs self.post_scan_hook (the reconcile) FIRST, then emits 'finished'.
The hook is injected by the web layer (it owns path resolution). So:
- status stays 'running' through the reconcile,
- the reconcile pushes its phase ("Reading file tags for N new tracks…")
and per-track progress through the SAME db_update_state callbacks the
scan already uses — so automations, the dashboard card, and the Tools
page all see it for free and wait for it,
- 'finished' is emitted exactly once, AFTER the reconcile — race-free, no
status blip a poll could catch,
- best-effort: a hook exception never blocks 'finished', so a scan can't
get stranded as perpetually 'running'.
Both scan entry points (_run_database_update_task, _run_deep_scan_task)
set the hook before run()/run_deep_scan(); the redundant post-run calls
are removed.
5 ordering tests pin the contract (hook-before-finished, finished still
fires without a hook, hook exception doesn't block finished, hook gets
the worker). Full suite clean (only pre-existing soundcloud /app env
failures remain).
This commit is contained in:
parent
83c1cd92aa
commit
79a30c055a
3 changed files with 106 additions and 14 deletions
|
|
@ -43,11 +43,17 @@ class DatabaseUpdateWorker:
|
|||
self.should_stop = False
|
||||
|
||||
# Track ids of rows newly INSERTED this run (not updates). The web
|
||||
# layer reads this after the scan to gap-fill embedded provider IDs
|
||||
# for the new files (auto-reconcile), so newly-added music contributes
|
||||
# its Spotify/MusicBrainz/etc. ids without a manual backfill.
|
||||
# layer reads this to gap-fill embedded provider IDs for the new files
|
||||
# (auto-reconcile), so newly-added music contributes its
|
||||
# Spotify/MusicBrainz/etc. ids without a manual backfill.
|
||||
self._new_track_ids = set()
|
||||
|
||||
# Optional callback(worker) run as the FINAL scan phase, immediately
|
||||
# before the 'finished' signal — so the auto-reconcile is inside the
|
||||
# scan's running window (automations/UI treat it as a normal phase and
|
||||
# wait for it). Injected by the web layer (which owns path resolution).
|
||||
self.post_scan_hook = None
|
||||
|
||||
# Statistics tracking
|
||||
self.processed_artists = 0
|
||||
self.processed_albums = 0
|
||||
|
|
@ -86,6 +92,25 @@ class DatabaseUpdateWorker:
|
|||
except Exception as e:
|
||||
logger.error(f"Error in callback for {signal_name}: {e}")
|
||||
|
||||
def _emit_finished(self, *args):
|
||||
"""Run the post-scan hook (auto-reconcile) as the final phase, THEN
|
||||
emit 'finished'.
|
||||
|
||||
Running the hook before 'finished' keeps the scan's status at
|
||||
'running' through the reconcile, so every caller (automations that
|
||||
poll for completion, the dashboard card, the Tools page) treats it as
|
||||
a normal scan phase and waits for it — rather than seeing 'finished'
|
||||
and missing the tail. Best-effort: a hook failure never blocks the
|
||||
completion signal.
|
||||
"""
|
||||
if self.post_scan_hook:
|
||||
try:
|
||||
self.post_scan_hook(self)
|
||||
except Exception as e:
|
||||
logger.warning(f"post-scan hook failed (non-fatal): {e}")
|
||||
self._emit_signal('finished', *args)
|
||||
|
||||
|
||||
def connect_callback(self, signal_name: str, callback: Callable):
|
||||
"""Connect a callback for progress notifications."""
|
||||
self.callbacks.setdefault(signal_name, []).append(callback)
|
||||
|
|
@ -152,7 +177,7 @@ class DatabaseUpdateWorker:
|
|||
logger.info(f"Merged {merged} duplicate artists")
|
||||
except Exception as e:
|
||||
logger.warning(f"Could not merge duplicate artists: {e}")
|
||||
self._emit_signal('finished', 0, 0, 0, 0, 0)
|
||||
self._emit_finished(0, 0, 0, 0, 0)
|
||||
return
|
||||
logger.info(f"Incremental update: Found {len(artists_to_process)} artists to process")
|
||||
|
||||
|
|
@ -236,7 +261,7 @@ class DatabaseUpdateWorker:
|
|||
self.removed_tracks = removal.get('tracks_removed', 0) if removal else 0
|
||||
|
||||
# Emit final results
|
||||
self._emit_signal('finished',
|
||||
self._emit_finished(
|
||||
self.processed_artists,
|
||||
self.processed_albums,
|
||||
self.processed_tracks,
|
||||
|
|
@ -337,7 +362,7 @@ class DatabaseUpdateWorker:
|
|||
f"{self.processed_albums} albums, {self.processed_tracks} new tracks, "
|
||||
f"{stale_removed} stale tracks removed")
|
||||
|
||||
self._emit_signal('finished',
|
||||
self._emit_finished(
|
||||
self.processed_artists,
|
||||
self.processed_albums,
|
||||
self.processed_tracks,
|
||||
|
|
|
|||
66
tests/test_database_update_reconcile_hook.py
Normal file
66
tests/test_database_update_reconcile_hook.py
Normal file
|
|
@ -0,0 +1,66 @@
|
|||
"""The auto-reconcile must run as the FINAL scan phase — inside the worker's
|
||||
completion, BEFORE the 'finished' signal — so the scan's status stays
|
||||
'running' through it. That ordering is what makes automations (which poll for
|
||||
completion), the dashboard card, and the Tools page all treat the reconcile as
|
||||
part of the scan and wait for it, rather than seeing 'finished' early and
|
||||
missing the tail. These pin that contract on DatabaseUpdateWorker._emit_finished.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from core.database_update_worker import DatabaseUpdateWorker
|
||||
|
||||
|
||||
def _bare_worker():
|
||||
# __new__ avoids the full media-client/config init; _emit_finished only
|
||||
# touches self.callbacks + self.post_scan_hook.
|
||||
w = DatabaseUpdateWorker.__new__(DatabaseUpdateWorker)
|
||||
w.callbacks = {'finished': [], 'error': [], 'progress_updated': [],
|
||||
'phase_changed': [], 'artist_processed': []}
|
||||
w.post_scan_hook = None
|
||||
return w
|
||||
|
||||
|
||||
def test_post_scan_hook_runs_before_finished():
|
||||
w = _bare_worker()
|
||||
order = []
|
||||
w.post_scan_hook = lambda worker: order.append('hook')
|
||||
w.callbacks['finished'].append(lambda *a: order.append('finished'))
|
||||
w._emit_finished(1, 2, 3, 4, 5)
|
||||
assert order == ['hook', 'finished'] # reconcile happens inside the running window
|
||||
|
||||
|
||||
def test_finished_receives_original_args():
|
||||
w = _bare_worker()
|
||||
got = []
|
||||
w.callbacks['finished'].append(lambda *a: got.append(a))
|
||||
w._emit_finished(1, 2, 3, 4, 5)
|
||||
assert got == [(1, 2, 3, 4, 5)]
|
||||
|
||||
|
||||
def test_no_hook_still_emits_finished():
|
||||
# Backward-compatible: a worker with no hook signals finished exactly as before.
|
||||
w = _bare_worker()
|
||||
got = []
|
||||
w.callbacks['finished'].append(lambda *a: got.append(a))
|
||||
w._emit_finished(0, 0, 0, 0, 0)
|
||||
assert got == [(0, 0, 0, 0, 0)]
|
||||
|
||||
|
||||
def test_hook_exception_never_blocks_finished():
|
||||
# A reconcile failure must not strand the scan as perpetually 'running'.
|
||||
w = _bare_worker()
|
||||
fired = []
|
||||
w.post_scan_hook = lambda worker: (_ for _ in ()).throw(RuntimeError("boom"))
|
||||
w.callbacks['finished'].append(lambda *a: fired.append(a))
|
||||
w._emit_finished(1, 1, 1, 1, 1)
|
||||
assert fired == [(1, 1, 1, 1, 1)]
|
||||
|
||||
|
||||
def test_hook_receives_the_worker():
|
||||
w = _bare_worker()
|
||||
seen = []
|
||||
w.post_scan_hook = lambda worker: seen.append(worker)
|
||||
w.callbacks['finished'].append(lambda *a: None)
|
||||
w._emit_finished(0, 0, 0, 0, 0)
|
||||
assert seen == [w]
|
||||
|
|
@ -15216,13 +15216,14 @@ def _run_db_update_task(full_refresh, server_type):
|
|||
db_update_worker.connect_callback('finished', _db_update_finished_callback)
|
||||
db_update_worker.connect_callback('error', _db_update_error_callback)
|
||||
|
||||
# Auto-reconcile runs as the FINAL scan phase (inside run(), before the
|
||||
# 'finished' signal) so status stays 'running' through it — automations,
|
||||
# the dashboard card, and the Tools page all treat it as part of the scan.
|
||||
db_update_worker.post_scan_hook = _reconcile_after_scan
|
||||
|
||||
# This is a blocking call that runs the worker logic
|
||||
db_update_worker.run()
|
||||
|
||||
# Auto-reconcile: pull embedded provider IDs from newly-added files into
|
||||
# the DB so enrichment workers skip those API lookups (#tag-id-backfill).
|
||||
_reconcile_after_scan(db_update_worker)
|
||||
|
||||
|
||||
def _run_deep_scan_task(server_type):
|
||||
"""Run a deep library scan in the background thread."""
|
||||
|
|
@ -15268,12 +15269,12 @@ def _run_deep_scan_task(server_type):
|
|||
db_update_worker.connect_callback('finished', _db_update_finished_callback)
|
||||
db_update_worker.connect_callback('error', _db_update_error_callback)
|
||||
|
||||
# Auto-reconcile runs as the final scan phase (see _run_database_update_task).
|
||||
db_update_worker.post_scan_hook = _reconcile_after_scan
|
||||
|
||||
# Run deep scan instead of normal run()
|
||||
db_update_worker.run_deep_scan()
|
||||
|
||||
# Auto-reconcile newly-inserted files' embedded provider IDs into the DB.
|
||||
_reconcile_after_scan(db_update_worker)
|
||||
|
||||
|
||||
@app.route('/api/database/stats', methods=['GET'])
|
||||
def get_database_stats():
|
||||
|
|
|
|||
Loading…
Reference in a new issue