Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jellyfin: Don't fail entire sync if artist mbid is corrupt #1332

Merged
merged 5 commits into from
Jun 9, 2024

Conversation

Jc2k
Copy link
Contributor

@Jc2k Jc2k commented Jun 8, 2024

Jellyfin does not seem to validate musicbrainz ids. It's basically a free text field in the Jellyfin UI, accepting even daft strings like "FEEDME". MA does validate them though, raising InvalidDataError if basic checks fail when setting the value on a model object. If that InvalidDataError exception is raised during a library sync then the sync will stop. This leads to a library with some artists, no albums and no tracks. This happened to my library after running Picard over it.

I would like to handle the exception, log a message the user can action, and then continue without bringing the MBID into MA. The rest of the users library is imported. And the user can see which track is broken, fix it, and then on the next sync it can be pulled in.

If you are happy with this change, I'd like to do a follow up PR to cover the other unprotected mbid cases in the sync process.

(In my case the field is "corrupt" because Jellyfin can't handle albums with multiple artists very well and has decided to put multiple artist mbid's against a single artist. This is so broken even parts of the Jellyfin UI can't handle it).

Jc2k added 2 commits June 8, 2024 15:14
Have a case where a musicbrainz id on an album is incorrect, and it was blocking my entire library from importing. Luckily MA already catches the error and raises an InvalidDataError on the Artist model setter. This just catches it, so that this artist is imported without the broken data and the other artists are still imported correctly.
Jc2k and others added 2 commits June 9, 2024 12:59
Co-authored-by: Marcel van der Veldt <m.vanderveldt@outlook.com>
Co-authored-by: Marcel van der Veldt <m.vanderveldt@outlook.com>
Copy link
Member

@marcelveldt marcelveldt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Thanks!

@marcelveldt marcelveldt merged commit 1e85046 into music-assistant:main Jun 9, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants