-
Notifications
You must be signed in to change notification settings - Fork 2k
Fix original release id access for a pseudo release #6250
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've left some high level feedback:
- Accessing
rel["release"]["id"]assumes both keys are always present; consider using.getwith a defensive fallback or an early-continue to avoid aKeyErroron unexpected MusicBrainz responses.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Accessing `rel["release"]["id"]` assumes both keys are always present; consider using `.get` with a defensive fallback or an early-continue to avoid a `KeyError` on unexpected MusicBrainz responses.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
f1c7064 to
f9c3aae
Compare
|
Thanks for the quick fix. It doesn't crash anymore. A quick side question, is it normal for https://musicbrainz.org/release-group/0c522c35-de39-4c7c-8bc4-0c33ed02f224 Based on the docs, I think I have the config right. plugins:
- convert
- fetchart
- embedart
- lastgenre
- lyrics
- duplicates
- replaygain
- mbsync
- scrub
- zero
# Autotaggers.
- chroma
- mbpseudo
- fromfilename
directory: ~/music
library: ~/music/library.db
convert:
auto: true # Automatically transcode non-MP3 files.
never_convert_lossy_files: true
delete_originals: true
format: opus
formats:
opus:
command: opus-transcode $source $dest
lyrics:
synced: true # Prefer synced lyrics.
sources:
# - google
- lrclib
replaygain:
backend: gstreamer
import:
move: true
languages: en # Prefer English aliases.
mbpseudo:
scripts:
- Latn
zero:
fields: comments
update_database: true |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6250 +/- ##
=======================================
Coverage 68.24% 68.24%
=======================================
Files 138 138
Lines 18781 18781
Branches 3163 3163
=======================================
Hits 12817 12817
Misses 5291 5291
Partials 673 673
🚀 New features to boost your workflow:
|
|
@UtkarshVerma ideally, I think, we would like to return both the original and pseudo release as two different candidates. Is this what you would expect to see? |
|
@snejus Yes, I would expect to see them both but this is what is happening so far: $ beet import -L "album:haruhayuku marie"
/home/user/music/Aimer/haruhayuku marie (1 items)
Match (82.7%):
Aimer - haruhayuku marie
≠ missing tracks, data source, tracks
MusicBrainz, Digital Media, 2020, None, SME Records, [none], Spotify exclusive
https://musicbrainz.org/release/e3f7a78d-afa6-4b45-bf3d-08638f93db73
* Artist: Aimer
* Album: haruhayuku marie
Missing tracks (3/4 - 75.0%):
! marie (#2) (5:07)
! Run Riot (#3) (4:16)
! 花の唄 end of spring ver. (#4) (5:51)
➜ [A]pply, More candidates, Skip, Use as-is, as Tracks, Group albums,
Enter search, enter Id, aBort? Based on this page: I expect at least one more release for the translation, but it never shows up, sadly. If it's okay with you can we move this to a discussion or IRC? Thanks! |
|
No worries, I guess I can just say that support for this is in progress: #6163 |
|
Great, nice to see that it's already being worked on. Will wait for the changes to get merged. Thanks! |
Fixes #6248