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

Add package for unmarshalling events sent by *arr #25

Closed
ionut-maxim opened this issue Jan 19, 2022 · 15 comments · Fixed by #30
Closed

Add package for unmarshalling events sent by *arr #25

ionut-maxim opened this issue Jan 19, 2022 · 15 comments · Fixed by #30

Comments

@ionut-maxim
Copy link

Hey, I think it would be nice if this library would be able to unmarshall environment variables or json received from any of the *arr apps so we can use it in a custom script or webhook.
As a quick example this is what I'm already doing for a custom script: https://github.com/ionut-maxim/notarry/blob/main/pkg/radarr/event.go

I know it is a bit out of scope for this library.

@davidnewhall
Copy link
Contributor

You're using this code with the Custom Script option in Connect?

@ionut-maxim
Copy link
Author

That is correct

@davidnewhall
Copy link
Contributor

I've been thinking about this request off and on since you opened it. I'm having a difficult time figuring out how this implementation would fit into this library. Everything in this library is geared toward HTTP API requests. There is no env unmarshaling code in this library, and I purposely avoid importing the library I wrote that does env unmarshaling.

All that said....

I think this is doable, but it's challenging. To test this I'd have to trigger every eventType in all four starr apps, and that sounds like a ton of "figuring it out." I do not think the complexity of this requires an external library for environment unmarshaling. I'd use os.Getenv and strings.Split. In the end you'd reduce that long chain of if statements you have to a small list of assignments. You could also pre-load your struct with defaults instead of checking for nil after unmarshaling. All of that can definitely be handled in a library, so all you do is call a function and get back a struct of data from the environment.

I'll leave this request open. I think this is not unreasonable, and I'll find some time to put into it.

@davidnewhall
Copy link
Contributor

I installed this script into all of my apps (they run in Docker).

#!/bin/bash

env >> /config/env
echo >> /config/env

In a week or so I should have enough data to make some pieces.

@ionut-maxim
Copy link
Author

I've been thinking about this request off and on since you opened it. I'm having a difficult time figuring out how this implementation would fit into this library. Everything in this library is geared toward HTTP API requests. There is no env unmarshaling code in this library, and I purposely avoid importing the library I wrote that does env unmarshaling.

Yeah I agree, this doesn't really fit with the library. It actually doesn't tie in at all with the rest of the library, but in a way I still see it as being an API for Starr applications.

I think this is doable, but it's challenging. To test this I'd have to trigger every eventType in all four starr apps, and that sounds like a ton of "figuring it out." I do not think the complexity of this requires an external library for environment unmarshaling. I'd use os.Getenv and strings.Split. In the end you'd reduce that long chain of if statements you have to a small list of assignments. You could also pre-load your struct with defaults instead of checking for nil after unmarshaling. All of that can definitely be handled in a library, so all you do is call a function and get back a struct of data from the environment.

I'll leave this request open. I think this is not unreasonable, and I'll find some time to put into it.
Regar

Thank you for the pointers regarding unmarshalling, it's certainly better to avoid dependencies if possible. Quick disclaimer, I'm just starting with Go 😄

Let me know if you need help with testing and thank you very much for considering this.

@davidnewhall
Copy link
Contributor

Got a few so far...

# grep event */env
lidarr/env:lidarr_eventtype=Test
lidarr/env:lidarr_eventtype=Test
lidarr/env:lidarr_eventtype=Grab
lidarr/env:lidarr_eventtype=AlbumDownload
radarr/env:radarr_eventtype=Test
radarr/env:radarr_eventtype=Test
radarr/env:radarr_eventtype=Grab
radarr/env:radarr_eventtype=Download
readarr/env:readarr_eventtype=Test
readarr/env:readarr_eventtype=Test
readarr/env:readarr_eventtype=Grab
readarr/env:readarr_eventtype=Grab
readarr/env:readarr_eventtype=Grab
readarr/env:readarr_eventtype=Grab
readarr/env:readarr_eventtype=Grab
sonarr/env:sonarr_eventtype=Test
sonarr/env:sonarr_eventtype=Test

@ionut-maxim
Copy link
Author

You can find all the environment variables being set here

@davidnewhall
Copy link
Contributor

Oh I fully intend to dig through the source for all these apps too. In reality, these things often differ when implemented, so I really like to see actual payloads. I've already found and filed bugs for several api paths as I sleuth through the outputs. Appreciate the link!

@davidnewhall
Copy link
Contributor

davidnewhall commented Jan 30, 2022

I have most of Radarr done. It's tedious work...

@ionut-maxim
Copy link
Author

Yeah, I imagine is not as easy as I thought 😄

@davidnewhall
Copy link
Contributor

I'd love your feedback on the linked pull request. Check out the new module here:
https://github.com/golift/starr/tree/dn2_starr_command/starrcmd

@ionut-maxim
Copy link
Author

I will test the module as soon as possible. Hopefully within the next two to three days. Thanks

@ionut-maxim
Copy link
Author

ionut-maxim commented Feb 5, 2022

Hey, done a quick test and as it currently is it works great with custom scripts.

I was thinking that it would be better for this module to also support webhooks.
This is a sample webhook body from Radarr

{
  "movie": {
    "id": 1141,
    "title": "Porco Rosso",
    "year": 1992,
    "releaseDate": "2005-04-13",
    "folderPath": "/data/movies/Porco Rosso (1992)",
    "tmdbId": 11621,
    "imdbId": "tt0104652"
  },
  "remoteMovie": {
    "tmdbId": 11621,
    "imdbId": "tt0104652",
    "title": "Porco Rosso",
    "year": 1992
  },
  "release": {
    "quality": "Bluray-720p",
    "qualityVersion": 1,
    "releaseGroup": "NoGroup",
    "releaseTitle": "Porco Rosso.1992.1080.BluRay.x264.DTS-NoGroup",
    "indexer": "NZBgeek (Prowlarr)",
    "size": 6685496000
  },
  "downloadClient": "NZBGet",
  "downloadId": "be684823e1514d2395984982e0dd94fa",
  "eventType": "Grab"
}

I guess that would mean restructuring to structs and adding tags for both json and env.
If you take a quick look at what I did in my first comment, it kind of emulates both webhook and custom script. I stopped working on it right now because I'm a bit busy at the moment :)

@davidnewhall
Copy link
Contributor

I'd just write it all into another module, and not try to dual purpose this one. Webhooks are a bit more interesting. We could definitely store the data structures, but it's not clear to me if this module would also provide http handlers. I don't think I've ever written boilerplate incoming web request code. I'd have to find some examples. Can you make a new issue with this new request? This issue is going to close when I merge that open pull request. More honesty, I probably wont get it done as quickly, but maybe the data structures are a good and easy start.

@ionut-maxim
Copy link
Author

Sounds good, gonna open a new issue for webhooks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants