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

YTMusic: Fix missing metadata for Artists on library sync #1418

Closed
wants to merge 4 commits into from

Conversation

MarvinSchenkel
Copy link
Contributor

@MarvinSchenkel MarvinSchenkel commented Jun 26, 2024

Fixes missing thumbs and descriptions during initial library sync. Previously, Artists were added as ItemMappings because of the missing info. However, those were never refreshed unless the were actively clicked on by the user. With this change, full artist details will be fetched during the library sync from YT Music, also relieving some stress on MusicBrainz.

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.

I'm not sure I can agree with this. This will result in a shitshow of requests against their api if I'm reading this right ?

ItemMapping exists to prevent all these additional per-item calls.

@MarvinSchenkel
Copy link
Contributor Author

MarvinSchenkel commented Jun 29, 2024

I'm not sure I can agree with this. This will result in a shitshow of requests against their api if I'm reading this right ?

ItemMapping exists to prevent all these additional per-item calls.

True, but the side effect is an artist overview without any thumbs and other Metadata. Somehow a force refresh on these item mappings do not result in resolving the artist from either ytm or music brains, so th user is left with empty Metadata here.

Also, these additional requests are only made during the sync.

Anyway, if you don't agree it's fine to close the pr. All artists in YTM coming from albums and tracks just won't have any Metadata. Not the end of the world

@marcelveldt
Copy link
Member

Let's concentrate on the issue then. By default artists (that go into the db from albums or tracks) wont have metadata unless you first click them. Is that not working ? Then we should fix that. Doing a full artist lookup at every sync is really way too heavy. What we can do however if this is really wanted, direct the request to the artists controller, as that will use the cache.

so instead of grabbing the artist within your own provider, do it by using mass.music.artists.get_provider_item

Another approach we could consider is restoring the background check but with a lot more caution to prevent too many calls.

@MarvinSchenkel
Copy link
Contributor Author

Let's concentrate on the issue then. By default artists (that go into the db from albums or tracks) wont have metadata unless you first click them. Is that not working ? Then we should fix that. Doing a full artist lookup at every sync is really way too heavy. What we can do however if this is really wanted, direct the request to the artists controller, as that will use the cache.

so instead of grabbing the artist within your own provider, do it by using mass.music.artists.get_provider_item

Another approach we could consider is restoring the background check but with a lot more caution to prevent too many calls.

Yeah I have to agree with this. First things first, I'll see if I can figure out why an item mapping is never updated with the full details. I can see the request coming in in the YT Music provider's get_artist but it never reaches the update_db in the artist controller.

@marcelveldt
Copy link
Member

superseded by #1480

@marcelveldt marcelveldt closed this Jul 8, 2024
@marcelveldt marcelveldt deleted the fix/artist_metadata_sync branch July 24, 2024 22:45
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