Fix LastFM rate limiting dropping scrobbles (f91fdd53 follow-up)
This commit is contained in:
@ -35,6 +35,15 @@ Two real bugs were found and fixed:
|
|||||||
Tests added in `tests/scrobbles_tests/test_lastfm.py` (5 tests) covering
|
Tests added in `tests/scrobbles_tests/test_lastfm.py` (5 tests) covering
|
||||||
dedup-by-timestamp, creation, and dispatch resume/no-resume behavior.
|
dedup-by-timestamp, creation, and dispatch resume/no-resume behavior.
|
||||||
|
|
||||||
|
A follow-up fix on 2026-08-08: the per-scrobble `track.getDuration`/
|
||||||
|
`getMbid` API calls in `get_last_scrobbles` exhausted Last.fm's rate limit
|
||||||
|
during large imports, raising `WSError` for almost every track and dropping
|
||||||
|
the whole scrobble (the artist lookup lived in the same `try`, so any failure
|
||||||
|
left `artist` unset). Artist now comes straight from the recent-tracks
|
||||||
|
response (no API call), duration/MBID enrichment is best-effort, and
|
||||||
|
enrichment calls are throttled to ~0.2s each. Test added for the
|
||||||
|
artist-survives-enrichment-failure path.
|
||||||
|
|
||||||
** DONE [#B] Fix LookupError on scrobble start/long-plays views :bug:scrobbles:videos:
|
** DONE [#B] Fix LookupError on scrobble start/long-plays views :bug:scrobbles:videos:
|
||||||
:PROPERTIES:
|
:PROPERTIES:
|
||||||
:ID: 868e2a91-c974-4656-a8ae-b90a1f94ae74
|
:ID: 868e2a91-c974-4656-a8ae-b90a1f94ae74
|
||||||
|
|||||||
@ -1,6 +1,7 @@
|
|||||||
from datetime import datetime, timedelta
|
from datetime import datetime, timedelta
|
||||||
from unittest.mock import MagicMock, patch
|
from unittest.mock import MagicMock, patch
|
||||||
|
|
||||||
|
import pylast
|
||||||
import pytest
|
import pytest
|
||||||
import pytz
|
import pytz
|
||||||
from django.contrib.auth import get_user_model
|
from django.contrib.auth import get_user_model
|
||||||
@ -62,6 +63,44 @@ def mock_track_find_or_create():
|
|||||||
yield mock_find
|
yield mock_find
|
||||||
|
|
||||||
|
|
||||||
|
def make_pylast_scrobble(artist, title, timestamp, album="Emotion"):
|
||||||
|
track = MagicMock()
|
||||||
|
artist_mock = MagicMock()
|
||||||
|
artist_mock.name = artist
|
||||||
|
track.artist = artist_mock
|
||||||
|
track.title = title
|
||||||
|
scrobble = MagicMock()
|
||||||
|
scrobble.track = track
|
||||||
|
scrobble.album = album
|
||||||
|
scrobble.timestamp = str(int(timestamp.timestamp()))
|
||||||
|
return scrobble
|
||||||
|
|
||||||
|
|
||||||
|
class TestGetLastScrobbles:
|
||||||
|
@pytest.mark.django_db
|
||||||
|
def test_artist_used_when_enrichment_fails(self, lfm_user, mock_lastfm_network):
|
||||||
|
timestamp = datetime(2023, 2, 15, 12, 0, 0, tzinfo=UTC)
|
||||||
|
scrobble = make_pylast_scrobble("Carly Rae Jepsen", "Emotion", timestamp)
|
||||||
|
# Enrichment (duration/mbid) hits Last.fm's rate limit, but the
|
||||||
|
# artist from the recent-tracks response must still be used.
|
||||||
|
scrobble.track.get_duration.side_effect = pylast.WSError(
|
||||||
|
None, 429, "rate limited"
|
||||||
|
)
|
||||||
|
scrobble.track.get_mbid.side_effect = pylast.WSError(None, 429, "rate limited")
|
||||||
|
|
||||||
|
lastfm = LastFM(lfm_user)
|
||||||
|
lastfm.user.get_recent_tracks.return_value = [scrobble]
|
||||||
|
with patch("scrobbles.importers.lastfm.time.sleep"):
|
||||||
|
parsed = lastfm.get_last_scrobbles()
|
||||||
|
|
||||||
|
assert len(parsed) == 1
|
||||||
|
assert parsed[0]["artist"] == "Carly Rae Jepsen"
|
||||||
|
assert parsed[0]["title"] == "Emotion"
|
||||||
|
assert parsed[0]["run_time_seconds"] is None
|
||||||
|
assert parsed[0]["mbid"] is None
|
||||||
|
assert parsed[0]["timestamp"] == timestamp
|
||||||
|
|
||||||
|
|
||||||
class TestImportFromLastfm:
|
class TestImportFromLastfm:
|
||||||
@pytest.mark.django_db
|
@pytest.mark.django_db
|
||||||
def test_import_creates_scrobble(
|
def test_import_creates_scrobble(
|
||||||
|
|||||||
@ -1,5 +1,6 @@
|
|||||||
import calendar
|
import calendar
|
||||||
import logging
|
import logging
|
||||||
|
import time
|
||||||
from datetime import datetime, timedelta
|
from datetime import datetime, timedelta
|
||||||
|
|
||||||
import pylast
|
import pylast
|
||||||
@ -138,30 +139,38 @@ class LastFM:
|
|||||||
|
|
||||||
for scrobble in found_scrobbles:
|
for scrobble in found_scrobbles:
|
||||||
logger.info(f"Processing {scrobble}")
|
logger.info(f"Processing {scrobble}")
|
||||||
|
log_dict = {"scrobble": scrobble}
|
||||||
|
|
||||||
|
# Artist/title/album come from the recent-tracks response itself,
|
||||||
|
# no extra API call required.
|
||||||
|
artist = scrobble.track.artist.name if scrobble.track.artist else None
|
||||||
|
log_dict["artist"] = artist
|
||||||
|
|
||||||
|
# Duration and MBID require a per-track `getInfo` API call, which
|
||||||
|
# quickly exhausts Last.fm's rate limit for large historical
|
||||||
|
# imports. Treat them as best-effort so one failure doesn't drop
|
||||||
|
# the whole scrobble.
|
||||||
run_time = None
|
run_time = None
|
||||||
mbid = None
|
mbid = None
|
||||||
artist = None
|
|
||||||
|
|
||||||
log_dict = {"scrobble": scrobble}
|
|
||||||
try:
|
try:
|
||||||
run_time = int(scrobble.track.get_duration() / 1000)
|
run_time = int(scrobble.track.get_duration() / 1000)
|
||||||
mbid = scrobble.track.get_mbid()
|
mbid = scrobble.track.get_mbid()
|
||||||
artist = scrobble.track.get_artist().name
|
|
||||||
log_dict["artist"] = artist
|
|
||||||
log_dict["mbid"] = mbid
|
log_dict["mbid"] = mbid
|
||||||
log_dict["run_time"] = run_time
|
log_dict["run_time"] = run_time
|
||||||
except pylast.MalformedResponseError as e:
|
except pylast.MalformedResponseError as e:
|
||||||
logger.warning(e)
|
logger.warning(e)
|
||||||
except pylast.WSError as e:
|
except pylast.WSError as e:
|
||||||
logger.info(
|
logger.info(
|
||||||
"LastFM barfed trying to get the track for {scrobble.track}",
|
f"LastFM barfed trying to enrich {scrobble.track}", extra=log_dict
|
||||||
extra=log_dict,
|
|
||||||
)
|
)
|
||||||
except pylast.NetworkError as e:
|
except pylast.NetworkError as e:
|
||||||
logger.info(
|
logger.info(
|
||||||
"LastFM barfed trying to get the track for {scrobble.track}",
|
f"LastFM barfed trying to enrich {scrobble.track}", extra=log_dict
|
||||||
extra=log_dict,
|
|
||||||
)
|
)
|
||||||
|
finally:
|
||||||
|
# Last.fm rate limit is ~5 req/sec; keep per-track enrichment
|
||||||
|
# calls spaced out so large imports don't trip it.
|
||||||
|
time.sleep(0.2)
|
||||||
|
|
||||||
if not artist:
|
if not artist:
|
||||||
logger.info(
|
logger.info(
|
||||||
|
|||||||
Reference in New Issue
Block a user