-
Notifications
You must be signed in to change notification settings - Fork 17
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
feat: recyclarr #44
base: main
Are you sure you want to change the base?
feat: recyclarr #44
Conversation
Here's an example of how I use it: services.nixarr = {
enable = true;
# ... other stuff
recyclarr = {
enable = true;
configFile = ./recyclarr.yaml;
};
# ...
}; Example
|
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.
This looks to be in really good shape. As mentioned on discord the build failure is on me, since I updated my nix website builder to be reproducible, which yielded an annoying bug. But I fixed it now!
I only have a few nitpicks, and:
By default, it is started via timer on a daily basis - maybe we want to expose that option too.
Yeah, that would be best. As long as the default is sane, I think daily is absolutely fine.
I also did not integrate this with the VPN.
That's a requirement for any nixarr submodule, so I would really prefer that. Check out the radarr/sonarr submodules and just copy over the boilerplate config for VPN (and change the names lol), that should be fine.
nixarr/recyclarr/default.nix
Outdated
}; | ||
|
||
configFile = mkOption { | ||
type = types.path; |
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.
This should probably just be an attribute set, and then converted into yaml. Then a user wouldn't have to write a seperate yaml file, and could just write recyclarr.config.sonarr.base_url = "http://localhost:8989"
.
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.
In many cases you will find provided profiles, and they're all in plain YAML (eg. TRaSH Guide provides YAML examples). Maybe we could offer both? Upstream uses attrsets.
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.
I actually like both, it would be "unclean" for nixpkgs, but Nixarr is meant to be more specialized and opinionated so it could work. Just make sure that the options are mutually exclusive and document the fact.
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.
Added it. I also tried mapping the yaml file to the new attrset option, but no native YAML support in nix, and readJSON
bails if the YAML contains comments.
Also, not for this PR, but would it make sense to add a wiki page for this? |
Thanks for the review! For the actual configuration there's plenty of information on recyclarr and TRaSH guide. What else do you think we should document? |
👍
I didn't do it because recyclarr does not expose a port and is just a CLI command. When starting up, it syncs some Github repos, not sure if we should route that through VPN. |
Makes sense, then don't do it. Once the other stuff is fixed, I'll merge it. |
Add the recyclarr service.
Add the recyclarr service. This enables users to import custom settings from eg. the TRaSH guide easily.
Recyclarr is available in nixpkgs-unstable, but I had to update the lockfile as we were tracking an older version.
Unlike the upstreamed module the config file can be referenced as path. This way we can reference the Sonarr and Radarr API keys using the
!env_file
macro, example:By default, it is started via timer on a daily basis - maybe we want to expose that option too? I also did not integrate this with the VPN.
I was unable to
nix build
the docs for some reason:Hope this is useful.