opentracker: init module #19176
opentracker: init module #19176
Conversation
|
@makefu, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @joachifm and @offlinehacker to be potential reviewers. |
| let | ||
| cfg = config.services.opentracker; | ||
|
|
||
| out = { |
There was a problem hiding this comment.
What is the motivation for this unusual construct?
There was a problem hiding this comment.
You are right, this pattern is not as widespread in nixpkgs as i have in mind. i will change it to match the general pattern. But generally this implementation gives a better overview about the Options and Implementation which are needed both for every nixos module at the very top of the file
| api = { | ||
| enable = mkEnableOption "opentracker"; | ||
|
|
||
| package = mkOption { |
There was a problem hiding this comment.
To override the default opentracker package? This seems to be a general pattern in a lot of nixos modules.
There was a problem hiding this comment.
But why would you want that in this case. That's more important than what other modules are doing.
There was a problem hiding this comment.
To use the latest version of opentracker without the need to override the package with nixpkgs.config.packageOverrides.
Right now the nixpkgs opentracker package is 2 years (2014-08-03 vs 2016-08-02) behind the latest commit and this option provides the ability to use a more current version instead.
| @@ -276,6 +276,7 @@ | |||
| telegraf = 256; | |||
| gitlab-runner = 257; | |||
| postgrey = 258; | |||
| opentracker = 259; | |||
There was a problem hiding this comment.
Why does this service need a static uid?
| wantedBy = [ "multi-user.target" ]; | ||
| restartIfChanged = true; | ||
| serviceConfig = { | ||
| Type = "simple"; |
| default = ""; | ||
| }; | ||
|
|
||
| user = mkOption { |
There was a problem hiding this comment.
Why would I want to run this as a different user?
There was a problem hiding this comment.
you are right, the code before transfer this PR didn't use another use but i figured it would be in line with all the other modules in nixpkgs.
There was a problem hiding this comment.
It's probably used too much, I suspect it has proliferated mainly because it's seen in existing modules.
There was a problem hiding this comment.
removed the code. It might be a good idea to have a pointer to a good sample for implementing nixos modules - https://nixos.org/nixos/manual/index.html#sec-writing-modules is pretty much the worst example tbh, it doesn't even use mkEnableOption
| description = '' | ||
| Working directory of opentracker | ||
| ''; | ||
| default = "/var/empty"; |
There was a problem hiding this comment.
This default suggests to me that opentracker won't generate any data (it will fail/crash if it tries). Given this, why would I want to change the work directory?
| type = types.str; | ||
| description = '' | ||
| User which will run opentracker. by default opentracker drops all | ||
| privileges and runs in chroot after starting up as root. |
There was a problem hiding this comment.
This comment indicates to me that opentracker does chrooting/privsep by itself, but that won't work with serviceConfig.User. But if it doesn't do this itself, the isolation options used below are not comparable to an ordinary chroot, making the description somewhat misleading.
There was a problem hiding this comment.
removed extra user creation
makefu
force-pushed
the
module-opentracker
branch
from
b51dac3
to
566f351
Compare
makefu
force-pushed
the
module-opentracker
branch
from
566f351
to
e9039c8
Compare
|
thanks. |
Motivation for this change
Implements services.opentracker for easy enabling of an opentracker service via systemd.
Main motivation for the change is that the current torrent tests simply run
opentracker &before starting torrent clients which is pretty crude.Things done