-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
FetchArt crashes with TypeError for particular album #3773
Comments
Huh, this is quite odd. It should be impossible for Is there any way you could either share your database file or narrow down what's going on with that particular album—to check whether its SQLite entry looks different from all other albums, for some reason? |
Okay, this is what I've come up with so far:
So what this looks like to me is that somewhere down the line in this import process, the album object has been modified in such a way that the type of that field has changed. Any ideas? |
I've dug deep and after lots of debugging print statements I think I've found the issue :). Along the way I monitored the type of the Here's my explanation. During the import process, the matched info is copied to the library model, which is then added to the library. item.mb_releasegroupid = album_info.releasegroup_id So I thought: since Discogs was the data source here (which uses all-digits IDs as I said), I might want to see how that plugin builds up that metadata. Please pay attention to this part of # Retrieve master release id (returns None if there isn't one).
master_id = result.data.get('master_id')
# Assume `original_year` is equal to `year` for releases without
# a master release, otherwise fetch the master release.
original_year = self.get_master_year(master_id) if master_id else year
return AlbumInfo(album, album_id, artist, artist_id, tracks, asin=None,
albumtype=albumtype, va=va, year=year, month=None,
day=None, label=label, mediums=len(set(mediums)),
artist_sort=None, releasegroup_id=master_id,
catalognum=catalogno, script=None, language=None,
country=country, albumstatus=None, media=media,
albumdisambig=None, artist_credit=None,
original_year=original_year, original_month=None,
original_day=None, data_source='Discogs',
data_url=data_url) Now, what I did is log the type of So, mystery solved I guess? I'm a but uncertain because I don't know this code too well, and I also don't know how to judge this situation exactly. But it feels like this is where the issue occurs. |
Hmm, this is definitely pointing in the right direction, and perhaps we should just change this here to stringily the ID. But it is still somewhat confusing because the
should be setting the field to be a string, even if the RHS of the assignment is an integer. Clearly that's not happening here, but I'm still sort of stumped about why. Anyway, like I said, maybe we should just stick our heads in the sand and do the conversion at that point, which would definitely be nice to do anyway. But I remain slightly disturbed that the automatic type normalization that's supposed to go on at that assignment is apparently not happening. 😱 |
Yes, that is unsettling. I don't like not understanding it properly, so I dug a little deeper. >>> i = lib.get_item(2)
>>> print(i.mb_releasegroupid, type(i.mb_releasegroupid))
c2841928-501f-37f7-a59e-cb1629471258 <class 'str'>
>>> i.mb_releasegroupid = 1
>>> print(i.mb_releasegroupid, type(i.mb_releasegroupid))
1 <class 'int'>
>>> i.store()
>>> i = lib.get_item(2)
>>> print(i.mb_releasegroupid, type(i.mb_releasegroupid))
1 <class 'str'> So, the Is it possible that this has something to do with the problem? That maybe somehow FetchArt uses the direct model instance, instead of retrieving it after storing? |
Strings were not being normalized, unlike some other types, leading to downstream problems like #3773.
Wow, awesome (and scary re. the implications of the problem)—thank you for digging more deeply into this. Is there any chance you might be able to give the proposed fix in #3774 a shot to see if it resolves the problem? |
Do you still want me to test this? Or should we wait for the discussion in #3774 to progress further first? |
If you wouldn't mind testing it, that would be awesome. If it works I think we should merge it as-is and revisit the broader change later. |
Okay. It won't be before Sunday evening at the earliest, but I'll let you know! |
I just patched my Beets installation with this change, and it resolved this issue :). |
Yay!! I'll merge it. |
I've imported hundreds of albums just fine using
fetchart
inauto
mode, and then this one came along and crashed the import.The issue in short:
So it seems
album.mb_releasegroupid
is anint
. This seems like a bug.Problem
Running this command in verbose (
-vv
) mode:Led to this problem:
Setup
My configuration (output of
beet config
) is:The text was updated successfully, but these errors were encountered: