From 58639c6fc11fbcc7f895c18350994dfddfd423b2 Mon Sep 17 00:00:00 2001 From: Colin Powell Date: Mon, 8 Jun 2026 10:58:02 -0400 Subject: [PATCH] [importers] Add error_log to surface import errors --- PROJECT.org | 66 ++++++- tests/birds_tests/test_importer.py | 50 +++++ vrobbler/apps/birds/admin.py | 2 +- vrobbler/apps/birds/importer.py | 10 +- .../0003_birdingcsvimport_error_log.py | 18 ++ vrobbler/apps/birds/models.py | 22 ++- vrobbler/apps/scrobbles/admin.py | 1 + ...ioscrobblertsvimport_error_log_and_more.py | 53 ++++++ vrobbler/apps/scrobbles/models.py | 171 +++++++++++------- 9 files changed, 324 insertions(+), 69 deletions(-) create mode 100644 vrobbler/apps/birds/migrations/0003_birdingcsvimport_error_log.py create mode 100644 vrobbler/apps/scrobbles/migrations/0090_audioscrobblertsvimport_error_log_and_more.py diff --git a/PROJECT.org b/PROJECT.org index 81086a4..f820b65 100644 --- a/PROJECT.org +++ b/PROJECT.org @@ -88,7 +88,7 @@ fetching and simple saving. *** Metadata sources **** Scraper -* Backlog [0/12] :vrobbler:project:personal: +* Backlog [1/16] :vrobbler:project:personal: ** TODO [#C] Create small utility to clean up tracks scrobbled with wonky playback times :vrobbler:personal:bug:music:scrobbles: :PROPERTIES: :ID: 702462cf-d54b-48c6-8a7c-78b8de751deb @@ -500,6 +500,70 @@ needed import celery task. This is how the WebDAV celery task currently works. This would also be an opporunity to clean up the code around WebDAV imports and make them more re-usable for other import services. +** TODO [#A] Generate a report of tracks with mistmatched metadata :music:tracks:metadata: +:PROPERTIES: +:ID: 684b8cd2-a3c1-4995-ba9e-7abdb02c37f2 +:END: + +*** Description + +We should have a management command that outputs a CSV file of track IDs where the log["raw_data"] artist +does not match the actual track artist. Specific cases are + +** TODO [#A] Add an exception list of artists as a constant that are exempted from splitting :music:artists:metadata: +:PROPERTIES: +:ID: fd86a11a-73ec-470d-b5e3-2d90ba9137c8 +:END: + +*** Description + +Certain artists like "Simon & Garfunkel" are actually one artist. While we don't want to mess with splitting up +tracks into featured artists, we should have a "LITERAL_ARTIST_TITLES" constant that can have exceptions like +this put into it and then we stop trying to pull the artist apart when we run into it. + +** TODO [#A] Before enriching anything, trust the POST data :feature:scrobbles:metadata: +:PROPERTIES: +:ID: db6b05f8-09f4-49f5-9838-fbacc9fe9cd0 +:END: + +*** Description + +Both Jellyfin and Mopidy provide a decent amount of metadata when they POST to our webhooks. + +In most cases, we should be able to trust this data to created music tracks or videos rather +than going to third-party services to enrich. Thus, for tracks and videos we should search in +the local database for imdb_id or musicbrainz_id for the specific content and, if found, not +enrich further. + +If not found, tracks and videos from mopidy and jellyfin should be created as completely as +possible using only the POST data from the webhooks, tagged the scrobble with "webhook-metadata-only" +and start the scrobble. A separate celery task should be kicked off to enrich the track or video +async with the POST data stored in the log["raw_data"] and used by the celery enrichment task +to go try to enrich the media instance. Should this enrichment fail, tag the scrobble as "enrichment-failed" +log a warning and move on. + +** DONE [#A] Date parsing failing in eBird imports :birds:ebird:importers: +:PROPERTIES: +:ID: 72e0254f-39d8-4843-9857-623e0362d77e +:END: + +*** Description + +On line 45 in the apps/birds/importer.py file, the import is thorowing this +error: + +#+begin_src python +ValueError: time data 'Jun 7, 2026, 5:15 PM' does not match format '%B %d, %Y %I:%M %p' +#+end_src + +Historically other files starting on May 24 worked, so I suspect this is a +problem of the date formatter expecting Long month names and zero-padded days +and the only sample we had was a three-letter month (May) and days with two +digites(24 through the 31) + +We should also add a "error_log" to the importers so that errors tha occur are +surfaced, even when 0 successful files were processed. And we should make sure +all importers do this as well. * Version 48.0 [2/2] ** DONE [#B] Show team or player images on sport detail and scrobble detail :sports:templates: diff --git a/tests/birds_tests/test_importer.py b/tests/birds_tests/test_importer.py index ff66cd4..464e899 100644 --- a/tests/birds_tests/test_importer.py +++ b/tests/birds_tests/test_importer.py @@ -1,3 +1,4 @@ +import tempfile from datetime import timedelta import pytest @@ -128,6 +129,23 @@ class TestBirdingCSVImportModel: assert imp.import_type == "Birding CSV" assert "Birding" in str(imp) + def test_record_error(self, db, user): + imp = BirdingCSVImport.objects.create(user=user) + assert imp.error_log is None + imp.record_error("test error") + imp.refresh_from_db() + assert imp.error_log is not None + assert "test error" in imp.error_log + + def test_record_error_appends(self, db, user): + imp = BirdingCSVImport.objects.create(user=user) + imp.record_error("first error") + imp.record_error("second error") + imp.refresh_from_db() + assert imp.error_log.count("\n") == 1 + assert "first error" in imp.error_log + assert "second error" in imp.error_log + @pytest.mark.django_db(transaction=True) def test_process_via_model(self, user, birding_csv_file): imp = BirdingCSVImport.objects.create(user=user) @@ -137,3 +155,35 @@ class TestBirdingCSVImportModel: imp.refresh_from_db() assert imp.process_count == 1 assert imp.processed_finished is not None + + def test_record_error_on_bad_csv(self, user, db): + content = """Species,Count,Location,Observation Type,Observation Date,Start Time,Duration,Distance,Area,Party Size,Complete Checklist,# of species,Details +Canada Goose,6,Test Park,Stationary,"Bad Date",4:15 PM,9 minute(s),,,4,true,4 species, +""" + with tempfile.NamedTemporaryFile( + mode="w", suffix=".csv", delete=False, encoding="utf-8" + ) as f: + f.write(content) + file_path = f.name + + errors = [] + scrobbles = import_birding_csv(file_path, user.id, record_error=errors.append) + assert len(scrobbles) == 0 + assert len(errors) == 1 + assert "Could not parse date/time" in errors[0] + + def test_record_error_on_bad_location(self, user, db): + content = """Species,Count,Location,Observation Type,Observation Date,Start Time,Duration,Distance,Area,Party Size,Complete Checklist,# of species,Details +Canada Goose,6,,Stationary,"May 10, 2026",4:15 PM,9 minute(s),,,4,true,4 species, +""" + with tempfile.NamedTemporaryFile( + mode="w", suffix=".csv", delete=False, encoding="utf-8" + ) as f: + f.write(content) + file_path = f.name + + errors = [] + scrobbles = import_birding_csv(file_path, user.id, record_error=errors.append) + assert len(scrobbles) == 0 + assert len(errors) == 1 + assert "Skipping rows with no location" in errors[0] diff --git a/vrobbler/apps/birds/admin.py b/vrobbler/apps/birds/admin.py index dcb474d..106de5d 100644 --- a/vrobbler/apps/birds/admin.py +++ b/vrobbler/apps/birds/admin.py @@ -26,5 +26,5 @@ class BirdingLocationAdmin(admin.ModelAdmin): @admin.register(BirdingCSVImport) class BirdingCSVImportAdmin(admin.ModelAdmin): date_hierarchy = "created" - list_display = ("uuid", "process_count", "processed_finished", "processing_started") + list_display = ("uuid", "process_count", "processed_finished", "processing_started", "error_log") ordering = ("-created",) diff --git a/vrobbler/apps/birds/importer.py b/vrobbler/apps/birds/importer.py index 1a8105d..ac8b657 100644 --- a/vrobbler/apps/birds/importer.py +++ b/vrobbler/apps/birds/importer.py @@ -64,7 +64,7 @@ def parse_int(value): return None -def import_birding_csv(file_path, user_id): +def import_birding_csv(file_path, user_id, record_error=None): user = User.objects.get(id=user_id) new_scrobbles = [] @@ -83,11 +83,17 @@ def import_birding_csv(file_path, user_id): for (location_str, date_str, time_str), sighting_rows in groups.items(): if not location_str: - logger.warning("Skipping rows with no location") + msg = "Skipping rows with no location" + logger.warning(msg) + if record_error: + record_error(msg) continue timestamp = parse_timestamp(date_str, time_str) if not timestamp: + msg = f"Could not parse date/time: {date_str} {time_str}" + if record_error: + record_error(msg) continue timestamp = user.profile.get_timestamp_with_tz(timestamp) diff --git a/vrobbler/apps/birds/migrations/0003_birdingcsvimport_error_log.py b/vrobbler/apps/birds/migrations/0003_birdingcsvimport_error_log.py new file mode 100644 index 0000000..3fbe5a0 --- /dev/null +++ b/vrobbler/apps/birds/migrations/0003_birdingcsvimport_error_log.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.29 on 2026-06-08 14:57 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("birds", "0002_birdingcsvimport"), + ] + + operations = [ + migrations.AddField( + model_name="birdingcsvimport", + name="error_log", + field=models.TextField(blank=True, null=True), + ), + ] diff --git a/vrobbler/apps/birds/models.py b/vrobbler/apps/birds/models.py index e74ef5a..c6545b5 100644 --- a/vrobbler/apps/birds/models.py +++ b/vrobbler/apps/birds/models.py @@ -216,6 +216,7 @@ class BirdingCSVImport(TimeStampedModel): processed_finished = models.DateTimeField(**BNULL) process_log = models.TextField(**BNULL) process_count = models.IntegerField(**BNULL) + error_log = models.TextField(**BNULL) csv_file = models.FileField(upload_to="birding-csv-uploads/", **BNULL) class Meta: @@ -269,6 +270,14 @@ class BirdingCSVImport(TimeStampedModel): self.process_count = len(scrobbles) self.save(update_fields=["process_log", "process_count"]) + def record_error(self, error_message): + log_line = f"{timezone.now().isoformat()}: {error_message}" + if self.error_log: + self.error_log += "\n" + log_line + else: + self.error_log = log_line + self.save(update_fields=["error_log"]) + def scrobbles(self): from scrobbles.models import Scrobble @@ -287,6 +296,13 @@ class BirdingCSVImport(TimeStampedModel): from birds.importer import import_birding_csv self.mark_started() - scrobbles = import_birding_csv(self.upload_file_path, self.user_id) - self.record_log(scrobbles) - self.mark_finished() + try: + scrobbles = import_birding_csv( + self.upload_file_path, self.user_id, record_error=self.record_error + ) + self.record_log(scrobbles) + except Exception as e: + self.record_error(f"Import failed: {e}") + logger.exception(f"Import failed for {self}") + finally: + self.mark_finished() diff --git a/vrobbler/apps/scrobbles/admin.py b/vrobbler/apps/scrobbles/admin.py index 453180c..bd75925 100644 --- a/vrobbler/apps/scrobbles/admin.py +++ b/vrobbler/apps/scrobbles/admin.py @@ -56,6 +56,7 @@ class ImportBaseAdmin(admin.ModelAdmin): "process_count", "processed_finished", "processing_started", + "error_log", ) ordering = ("-created",) diff --git a/vrobbler/apps/scrobbles/migrations/0090_audioscrobblertsvimport_error_log_and_more.py b/vrobbler/apps/scrobbles/migrations/0090_audioscrobblertsvimport_error_log_and_more.py new file mode 100644 index 0000000..eef04d4 --- /dev/null +++ b/vrobbler/apps/scrobbles/migrations/0090_audioscrobblertsvimport_error_log_and_more.py @@ -0,0 +1,53 @@ +# Generated by Django 4.2.29 on 2026-06-08 14:57 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("scrobbles", "0089_favoritemedia"), + ] + + operations = [ + migrations.AddField( + model_name="audioscrobblertsvimport", + name="error_log", + field=models.TextField(blank=True, null=True), + ), + migrations.AddField( + model_name="bgstatsimport", + name="error_log", + field=models.TextField(blank=True, null=True), + ), + migrations.AddField( + model_name="ebirdcsvimport", + name="error_log", + field=models.TextField(blank=True, null=True), + ), + migrations.AddField( + model_name="koreaderimport", + name="error_log", + field=models.TextField(blank=True, null=True), + ), + migrations.AddField( + model_name="lastfmimport", + name="error_log", + field=models.TextField(blank=True, null=True), + ), + migrations.AddField( + model_name="retroarchimport", + name="error_log", + field=models.TextField(blank=True, null=True), + ), + migrations.AddField( + model_name="scalecsvimport", + name="error_log", + field=models.TextField(blank=True, null=True), + ), + migrations.AddField( + model_name="trailgpximport", + name="error_log", + field=models.TextField(blank=True, null=True), + ), + ] diff --git a/vrobbler/apps/scrobbles/models.py b/vrobbler/apps/scrobbles/models.py index cea70dd..7041f96 100644 --- a/vrobbler/apps/scrobbles/models.py +++ b/vrobbler/apps/scrobbles/models.py @@ -74,6 +74,7 @@ class BaseFileImportMixin(TimeStampedModel): processed_finished = models.DateTimeField(**BNULL) process_log = models.TextField(**BNULL) process_count = models.IntegerField(**BNULL) + error_log = models.TextField(**BNULL) class Meta: abstract = True @@ -158,6 +159,14 @@ class BaseFileImportMixin(TimeStampedModel): self.process_count = len(scrobbles) self.save(update_fields=["process_log", "process_count"]) + def record_error(self, error_message): + log_line = f"{timezone.now().isoformat()}: {error_message}" + if self.error_log: + self.error_log += "\n" + log_line + else: + self.error_log = log_line + self.save(update_fields=["error_log"]) + @property def upload_file_path(self): raise NotImplementedError @@ -213,9 +222,14 @@ class KoReaderImport(BaseFileImportMixin): return self.mark_started() - scrobbles = process_koreader_sqlite_file(self.upload_file_path, self.user.id) - self.record_log(scrobbles) - self.mark_finished() + try: + scrobbles = process_koreader_sqlite_file(self.upload_file_path, self.user.id) + self.record_log(scrobbles) + except Exception as e: + self.record_error(f"Import failed: {e}") + logger.exception(f"Import failed for {self}") + finally: + self.mark_finished() class AudioScrobblerTSVImport(BaseFileImportMixin): @@ -255,10 +269,14 @@ class AudioScrobblerTSVImport(BaseFileImportMixin): return self.mark_started() - - scrobbles = import_audioscrobbler_tsv_file(self.upload_file_path, self.user.id) - self.record_log(scrobbles) - self.mark_finished() + try: + scrobbles = import_audioscrobbler_tsv_file(self.upload_file_path, self.user.id) + self.record_log(scrobbles) + except Exception as e: + self.record_error(f"Import failed: {e}") + logger.exception(f"Import failed for {self}") + finally: + self.mark_finished() class ScaleCSVImport(BaseFileImportMixin): @@ -297,9 +315,14 @@ class ScaleCSVImport(BaseFileImportMixin): return self.mark_started() - scrobbles = import_scale_csv(self.upload_file_path, self.user.id) - self.record_log(scrobbles) - self.mark_finished() + try: + scrobbles = import_scale_csv(self.upload_file_path, self.user.id) + self.record_log(scrobbles) + except Exception as e: + self.record_error(f"Import failed: {e}") + logger.exception(f"Import failed for {self}") + finally: + self.mark_finished() class TrailGPXImport(BaseFileImportMixin): @@ -337,11 +360,16 @@ class TrailGPXImport(BaseFileImportMixin): return self.mark_started() - scrobbles = import_trail_gpx( - self.upload_file_path, self.user.id, self.original_filename - ) - self.record_log(scrobbles) - self.mark_finished() + try: + scrobbles = import_trail_gpx( + self.upload_file_path, self.user.id, self.original_filename + ) + self.record_log(scrobbles) + except Exception as e: + self.record_error(f"Import failed: {e}") + logger.exception(f"Import failed for {self}") + finally: + self.mark_finished() class LastFmImport(BaseFileImportMixin): @@ -391,11 +419,14 @@ class LastFmImport(BaseFileImportMixin): last_processed = last_import.processed_finished self.mark_started() - - scrobbles = lastfm.import_from_lastfm(last_processed, time_to=time_to) - - self.record_log(scrobbles) - self.mark_finished() + try: + scrobbles = lastfm.import_from_lastfm(last_processed, time_to=time_to) + self.record_log(scrobbles) + except Exception as e: + self.record_error(f"Import failed: {e}") + logger.exception(f"Import failed for {self}") + finally: + self.mark_finished() class RetroarchImport(BaseFileImportMixin): @@ -426,43 +457,48 @@ class RetroarchImport(BaseFileImportMixin): logger.info(f"You told me to force import from Retroarch") self.mark_started() + scrobbles = None + try: + if self.lrtl_file: + import os + import tempfile + import zipfile - if self.lrtl_file: - import os - import tempfile - import zipfile + tmpdir = tempfile.mkdtemp() + try: + zip_path = os.path.join(tmpdir, "archive.zip") + with open(zip_path, "wb") as f: + f.write(self.lrtl_file.read()) + with zipfile.ZipFile(zip_path, "r") as zf: + zf.extractall(tmpdir) + os.unlink(zip_path) + scrobbles = retroarch.import_retroarch_lrtl_files( + tmpdir + "/", + self.user.id, + ) + finally: + import shutil + + shutil.rmtree(tmpdir, ignore_errors=True) + else: + if not self.user.profile.retroarch_path: + logger.info( + "Trying to import Retroarch logs, but user has no retroarch_path configured" + ) + self.mark_finished() + return - tmpdir = tempfile.mkdtemp() - try: - zip_path = os.path.join(tmpdir, "archive.zip") - with open(zip_path, "wb") as f: - f.write(self.lrtl_file.read()) - with zipfile.ZipFile(zip_path, "r") as zf: - zf.extractall(tmpdir) - os.unlink(zip_path) scrobbles = retroarch.import_retroarch_lrtl_files( - tmpdir + "/", + self.user.profile.retroarch_path, self.user.id, ) - finally: - import shutil - shutil.rmtree(tmpdir, ignore_errors=True) - else: - if not self.user.profile.retroarch_path: - logger.info( - "Tying to import Retroarch logs, but user has no retroarch_path configured" - ) - self.mark_finished() - return - - scrobbles = retroarch.import_retroarch_lrtl_files( - self.user.profile.retroarch_path, - self.user.id, - ) - - self.record_log(scrobbles) - self.mark_finished() + self.record_log(scrobbles) + except Exception as e: + self.record_error(f"Import failed: {e}") + logger.exception(f"Import failed for {self}") + finally: + self.mark_finished() class BGStatsImport(BaseFileImportMixin): @@ -499,17 +535,21 @@ class BGStatsImport(BaseFileImportMixin): return self.mark_started() + try: + import json - import json + from scrobbles.scrobblers import email_scrobble_board_game - from scrobbles.scrobblers import email_scrobble_board_game + with open(self.upload_file_path, "r", encoding="utf-8") as f: + parsed_json = json.load(f) + scrobbles = email_scrobble_board_game(parsed_json, self.user_id) - with open(self.upload_file_path, "r", encoding="utf-8") as f: - parsed_json = json.load(f) - scrobbles = email_scrobble_board_game(parsed_json, self.user_id) - - self.record_log(scrobbles) - self.mark_finished() + self.record_log(scrobbles) + except Exception as e: + self.record_error(f"Import failed: {e}") + logger.exception(f"Import failed for {self}") + finally: + self.mark_finished() class EBirdCSVImport(BaseFileImportMixin): @@ -554,9 +594,16 @@ class EBirdCSVImport(BaseFileImportMixin): return self.mark_started() - scrobbles = import_birding_csv(self.upload_file_path, self.user_id) - self.record_log(scrobbles) - self.mark_finished() + try: + scrobbles = import_birding_csv( + self.upload_file_path, self.user_id, record_error=self.record_error + ) + self.record_log(scrobbles) + except Exception as e: + self.record_error(f"Import failed: {e}") + logger.exception(f"Import failed for {self}") + finally: + self.mark_finished() class ScrobbleQuerySet(models.QuerySet):