search

beetbox / beets

music library manager and MusicBrainz tagger
http://beets.io/
MIT License
12.57k stars 1.8k forks source link

Handle NULL values in the path column #5214

Closed snejus closed 2 months ago

snejus commented 2 months ago

As I was devving, I did something wrong and had beet mv command fail on me.

Later, having spent an hour investigating why beets kept throwing me 'User-defined function raised exception' I discovered that it was failing because that previous beet mv command ended up writing value NULL in one of the items' path column. This was not handled well by the BYTELOWER implementation.

Since we do not have a NOT NULL constraint for the path column in the db, it's best to insure ourselves against this kind of stuff anyways.

github-actions[bot] commented 2 months ago

Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry.

wisp3rwind commented 2 months ago

lgtm :tada:

Serene-Arc commented 2 months ago

...wth why did the changelog bot suddenly work for this PR and not any other?

wisp3rwind commented 2 months ago

I guess because this branch was pushed to beetbox/ rather than snejus/. So, actually, there seems to be some permission error, the docs of the github action mention something along those lines.

snejus commented 2 months ago

@Serene-Arc Could I by any chance get access to the repo settings? I've had this issue before and fixed it, but now I don't remember which specific settings were to blame, so I'd be happy just to have a look at what we've currently got.

Serene-Arc commented 2 months ago

It is a permission error, 100%, it's in the PR for the change I think. I don't have the access required to change it, but that makes sense as to why it works for this.

So @snejus no, because I don't have access either. You'll have to as @sampsyo for those permissions (and I wouldn't mind getting them either if it's not a security risk).

snejus commented 2 months ago

Cool, will drop him a message :)