Similar Artists worker: guarantee every stored similar has a source id
Verified against live data: 1312/1313 stored similars carry a metadata source id, but 1 slipped through name-only (a match on a source with no id column, e.g. discogs). Enforce the standard: process_artist now SKIPS any similar whose match doesn't map to a storable id column (spotify/itunes/deezer/musicbrainz) instead of writing a useless name-only row. Regression test covers discogs-match + no-id cases. Now 100% of newly-stored similars are actionable.
This commit is contained in:
parent
9d308638f0
commit
843de8a45e
2 changed files with 23 additions and 0 deletions
|
|
@ -108,6 +108,13 @@ def process_artist(
|
||||||
if not name:
|
if not name:
|
||||||
continue
|
continue
|
||||||
kwargs = map_payload_to_store_kwargs(s)
|
kwargs = map_payload_to_store_kwargs(s)
|
||||||
|
if not kwargs:
|
||||||
|
# The match resolved to a source with no id column in similar_artists
|
||||||
|
# (e.g. discogs). Storing it name-only would be useless — you can't
|
||||||
|
# navigate/explore/download it. Enforce the standard: every stored
|
||||||
|
# similar carries a metadata source id, or we skip it.
|
||||||
|
logger.debug("Skipping similar '%s' (matched %s — no storable source id)", name, s.get('source'))
|
||||||
|
continue
|
||||||
try:
|
try:
|
||||||
ok = store_similar(
|
ok = store_similar(
|
||||||
source_artist_id=source_artist_id,
|
source_artist_id=source_artist_id,
|
||||||
|
|
|
||||||
|
|
@ -115,6 +115,22 @@ def test_process_artist_error_when_fetch_raises_carries_detail():
|
||||||
assert status == 'error' and count == 0 and 'musicmap down' in detail
|
assert status == 'error' and count == 0 and 'musicmap down' in detail
|
||||||
|
|
||||||
|
|
||||||
|
def test_process_artist_skips_similars_without_a_storable_source_id():
|
||||||
|
# Every stored similar MUST carry a metadata source id (spotify/itunes/deezer/
|
||||||
|
# musicbrainz) — otherwise it's not actionable. A match on a source with no id
|
||||||
|
# column (e.g. discogs) is skipped, never stored name-only.
|
||||||
|
store, calls = _capture_store()
|
||||||
|
payload = {'success': True, 'similar_artists': [
|
||||||
|
{'name': 'KeepMe', 'id': 'sp1', 'source': 'spotify'},
|
||||||
|
{'name': 'DropMe', 'id': 'dg1', 'source': 'discogs'}, # no id column → must skip
|
||||||
|
{'name': 'NoId', 'source': 'spotify'}, # no id at all → must skip
|
||||||
|
]}
|
||||||
|
status, count, _ = w.process_artist('S', 'A', lambda n, l: payload, store)
|
||||||
|
assert count == 1 and len(calls) == 1
|
||||||
|
assert calls[0]['similar_artist_name'] == 'KeepMe'
|
||||||
|
assert calls[0]['similar_artist_spotify_id'] == 'sp1'
|
||||||
|
|
||||||
|
|
||||||
def test_process_artist_skips_unstorable_but_counts_real():
|
def test_process_artist_skips_unstorable_but_counts_real():
|
||||||
# A store() that fails for one row shouldn't abort the rest.
|
# A store() that fails for one row shouldn't abort the rest.
|
||||||
calls = []
|
calls = []
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue