n8n: new service #1630
n8n: new service #1630
Conversation
i-am-logger
force-pushed
the
logger/n8n
branch
5 times, most recently
from
13c929f to
f5fbdba
Compare
i-am-logger
force-pushed
the
logger/n8n
branch
from
f5fbdba to
8c87d38
Compare
|
working, would appriciate a review |
There was a problem hiding this comment.
Thanks, @i-am-logger. I've requested a bunch of minor changes, but otherwise looks good overall.
| assertions = [{ | ||
| assertion = cfg.enable; | ||
| message = '' | ||
| To use n8n, you have to enable it. (services.n8n.enable = true;) | ||
| ''; | ||
| }]; |
There was a problem hiding this comment.
There's no need to assert this.
| assertions = [{ | |
| assertion = cfg.enable; | |
| message = '' | |
| To use n8n, you have to enable it. (services.n8n.enable = true;) | |
| ''; | |
| }]; |
| with lib; | ||
|
|
There was a problem hiding this comment.
inherit is strongly preferred to with, as it can be statically type-checked. In this case, it can be removed entirely.
| with lib; |
| { | ||
| options = { | ||
| services.n8n = { | ||
| enable = mkEnableOption "n8n"; |
There was a problem hiding this comment.
| enable = mkEnableOption "n8n"; | |
| enable = lib.mkEnableOption "n8n"; |
| type = types.str; | ||
| description = "Listen address"; | ||
| default = "127.0.0.1"; | ||
| example = "127.0.0.1"; |
There was a problem hiding this comment.
| example = "127.0.0.1"; |
|
|
||
| port = lib.mkOption { | ||
| type = types.port; | ||
| default = 5432; |
There was a problem hiding this comment.
The default in their docs is 5678. 5432 is the default for postgres, so we should avoid using that.
| default = 5432; | |
| default = 5678; |
| }; | ||
|
|
||
| processes.n8n = { | ||
| exec = "${pkgs.n8n}/bin/n8n"; |
There was a problem hiding this comment.
It's nice to have a package option in the module to let people override the package used.
| @@ -0,0 +1,7 @@ | |||
| { | |||
There was a problem hiding this comment.
To test that it launches, you can add a .test.sh script in this folder and curl a health endpoint of some kind. There are lots of examples of this in the repo.
| @@ -0,0 +1,2 @@ | |||
| { ... }: | |||
There was a problem hiding this comment.
The n8n can be removed in favor of the test.
| }; | ||
|
|
||
| webhookUrl = lib.mkOption { | ||
| type = lib.types.str; |
There was a problem hiding this comment.
If null, then the WEBOOK_URL env var won't be set at all, as long as you remove the quotes when setting the env var later.
| type = lib.types.str; | |
| type = types.nullOr types.str; |
|
|
||
| webhookUrl = lib.mkOption { | ||
| type = lib.types.str; | ||
| default = ""; |
There was a problem hiding this comment.
| default = ""; | |
| default = null; |
|
@i-am-logger, I think quite a few of the requested changes are also applicable to your previous PR. I'll be happy to review it once updated. |
having an issue #1628 will appriciate the help.