Add lidarr as downloader

[avedor]

Implements an MVP version of a Lidarr downloader using the lidarr-go sdk. Currently, can't seem to automatically download using slskd, but the general workflow is functional. Would probably close #16

[LumePart]

I'll try running it in my test environment when I have the time. the code looks really good at the moment.

It would be nice to get a function to check the download progress of the album, and if it hasn't progressed at all, let's say in 15 minutes, then the album download will stop (and removed from Lidarr). That would allow for the track to be downloaded from another downloader.

[avedor]

sounds good! i've got the actual logic written up. how would you envision something like that running? i've added it as a goroutine for now, but if there's a different architecture you'd prefer, just name it!

[LumePart]

i've added it as a goroutine for now

Yeah, goroutine is what I was thinking too

I looked through and ran the code and I'm a bit confused about findArtist() and addArtistIfNeeded(), previously when I investigated Lidarr, I did a manual flow of API requests to download the album and I only used /api/v1/album endpoint.

This is the request I used for adding and sending the search to indexers:

POST http://URL:PORT/api/v1/album
Header: X-Api-Key: API_KEY
Body:

{
 "foreignAlbumId": "70bdc19f-66fb-4adf-9124-925ace531101",
 "monitored": true,
 "qualityProfileId": 1,
 "rootFolderPath": "/music",
 "monitored": true,
 "artist": {
     "foreignArtistId": "89ea85c2-c90a-46ea-93db-9bed95d5c8f8",
     "qualityProfileId": 1,         // Artist's Quality Profile ID
     "metadataProfileId": 2,        // Artist's Metadata Profile ID
     "monitored": true,
     "rootFolderPath": "/music"
     },
 "addOptions": {
     "searchForNewAlbum": true
     }
}

AlbumID and artistID came from album lookup and the ProfileID's could probably be config parameters if the SDK supports it.
I had trouble implementing the same flow with the SDK, but I can try again tomorrow.

And i fixed a small issue with track.MainArtist in the dev branch, when searching for albums it's best to use that field, track.Artist could contain all the artists on the track (i.e Justice feat. Tame Impala)

[avedor]

Yeah! so the whole flow was a little confusing for me as well and the sdk doesn't seem to map 1:1 onto the api endpoints which is... frustrating.

my understanding is we've got the foreign (read musicbrainz) ids for everything and the internal (lidarr) ids. Lidarr ids for artist/album don't get set if the artist isn't in lidarr already, so findArtist does the lookup using a string query--though could probably be switched to use the artist mbid if we add that as a field in the track struct--then the addArtistIfNeeded is creating the artist in lidarr, if it isn't already there, using defaults pulled from the root settings.

i'll dig into the sdk a little further to see if i can bypass a lot of the lookups and just use the mbids to create the album in lidarr

[avedor]

just for complete context, between these changes, and those in #22, i've now got a new playlist in jellyfin with 33/50 tracks!

as far as the flow using sdk is concerned, i think we're roughly stuck at:

1. look up artist using string term search and return ArtistResource
2. pass that into addArtistIfNeeded which checks to see if a. the artist has a path set and b. has a non-zero timestamp for when the artist was added. if not, it creates a new ArtistResource with some defaults from the root settings and calls CreateArtist to add the artist to lidarr
3. repeat the same basic flow for album and release

[LumePart]

Wouldn't it be more straightforward to do it without the SDK? It would reduce the amount of flows needed and would give us greater control of the request payloads. Especially when we don't need most of the API functionality (most likely 4-5 endpoints for a full workflow).

The flow would look pretty much like this:

1. Query album to get artistID and AlbumID
2. Request album to be downloaded/sent to indexer
3. Periodically check if it's downloading
4. If downloaded, mark track as Present otherwise clean Lidarr of artist/track

Feel free to debate me on this, maybe i just don't get the point of the SDK when Lidarr's API documentation is pretty good.

[avedor]

This is the first time I've used an SDK, so I'm happy to do it either way! I just pushed up a couple squashed commits that should do all of this a little more cleanly (storing the MBIDs) in the track struct for ease. I also updated the goroutine to both clear stale downloads and mark present newly available tracks

Looking forward to your thoughts!

[LumePart]

Made a few changes in the dev branch, main changes include:

• Added a MonitorDownloads([]*models.Track) function to the Downloader interface, Lidarr most likely can make use of this.
• queryTrack() and getTrack() are asynchronous now
• Added a few fields to Track struct, including a field to store the track size (if needed) and an field from ListenBrainz (length (in ms))

I'm also pretty much finished with implementing slskd, you can check how I implemented download monitoring there

Also, the linter fails, but that is expected atm.

[avedor]

i did something that might be a little too crazy with the download monitoring. go doesn't allow for reusability quite as much as my SRE brain would like, but i think this could be a path forward to keep from just copying the same code into lidarr.go

of course happy to find a different solution if this feels too out there!

[LumePart]

Makes perfect sense, I can hopefully test it this week.

I also left a couple of reviews in May, regarding the Lidarr part. I don't believe they have been addressed (or Github is just wonky)

A related question as well:
Is Lidarr fixed now? I heard there were some issues with their metadata servers, but some people say it's working for them 🤷

[tessamerrill]

Is Lidarr fixed now? I heard there were some issues with their metadata servers, but some people say it's working for them 🤷

I'll jump in and say that I've been using a fork that uses a different metadata server (that actually works).

[LumePart]

I think it would be better to move the monitor refactoring into a separate pull request, after Lidarr is fully implemented. This would simplify testing and allow for a cleaner refactor, especially since monitor.go will likely require a shared struct.

Right now, I can't test Lidarr properly because my test environment doesn't like hearring-aid (the MusicBrainz server requirements are a bit too much for it). Since I'm not sure what fields Lidarr needs yet, I can't provide meaningful feedback at the moment.

But according to Lidarr's Issues page, they are pretty close in making it work again.

[tessamerrill]

I think it would be better to move the monitor refactoring into a separate pull request, after Lidarr is fully implemented. This would simplify testing and allow for a cleaner refactor, especially since monitor.go will likely require a shared struct.

Right now, I can't test Lidarr properly because my test environment doesn't like hearring-aid (the MusicBrainz server requirements are a bit too much for it). Since I'm not sure what fields Lidarr needs yet, I can't provide meaningful feedback at the moment.

But according to Lidarr's Issues page, they are pretty close in making it work again.

I would be more than happy to test using my environment, are there any docker images or documentation i can use?

[avedor]

So i've moved the refactor onto its own branch (#40). It may make more sense to address that first so that this PR can leverage that function instead of implementing it again and removing it in the follow up. Everything else, I think depends on that?

That being said, if your preference is to do Lidarr as is and the refactor after, I'll gladly adapt your work for this!

[LumePart]

Sorry for the long wait, I have completed the monitoring refactor.
I added a Monitoring interface that is implemented by Downloader, it has 3 functions:

GetDownloadStatus - handles the querying of track statuses from a downloader and compiling a map of structs to be used in monitor.go.

GetConf - Returns the monitoring configuration (or an error if monitoring isn't used (like with Youtube)).

Cleanup - For removal of search requests (or temp files) from the system after the track has been downloaded/failed to be downloaded.

Things likely would need to be changed to fit lidarr (like the shared struct), as the refactor is entirely based off slskd.

If any details are needed then just ask,
I'm also available on Discord (lumepart) or Matrix (@LumePart:tchncs.de) for faster collaboration

[avedor]

I appreciate the update! I think I've gotten everything hooked up correctly, but there's been an unfortunate wrinkle in that the Lidarr metadata server is having issues. As it stands, the only response I can seemingly get back from the Lidarr API is a 503.

It looks like they're nearly done with their upgrade/transition, but until it's working even remotely reliably, there's not a whole lot more testing I'll be able to do. 🙁

[LumePart]

Thanks!
Seems like fixing the metadata server is taking a lot longer than anticipated.

I have a question about the LIDARR_SCHEME variable, it's still used in some of the functions, but not in the Monitor functions. Can it be taken out completely? Seems redundant

[Nedra1998]

I believe the Lidarr metadata server issues have been resolved now. I love this project, but would prefer to use lidarr for the downloads.

Lidarr/Lidarr#5498 (comment)