Early prototype of node module(s) #33
Conversation
avnik
force-pushed
the
avnik/node-module
branch
3 times, most recently
from
ff7c471 to
8e083ab
Compare
aciceri
left a comment
aciceri
left a comment
There was a problem hiding this comment.
I know you said to to review concepts because you had to refactor it but I couldn't resist, sorry...
Wasn't there already another PR about pre-commit-hooks? If you can please remove relative commits from there, probably you inadvertently created this branch starting from the other one.
modules/cardano-node/types.nix
Outdated
| shellyEraKeysType = submodule { | ||
| # Shelly+ era secrets path definitions | ||
| vrfKey = mkOption { | ||
| # "${name}-vrf.skey"; | ||
| type = either path str; | ||
| }; | ||
| kesKey = mkOption { | ||
| type = either path str; | ||
| # "${name}-kes.skey"; | ||
| }; | ||
| coldVerification = mkOption { | ||
| type = either path str; | ||
| # "${name}-cold.vkey"; | ||
| }; | ||
| operationalCertificate = mkOption { | ||
| type = either path str; | ||
| # "${name}.opcert"; | ||
| }; | ||
| bulkCredentials = mkOption { | ||
| type = nullOr (either path str); | ||
| # "${name}-bulk.creds"; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Perhaps you could create a type alias at this point.
There was a problem hiding this comment.
If you want I believe you can define types as options of type type, which is better than having a file like this because it allows people overriding them. (can't think a reason why it would be useful but still better provide the maximum customizability)
modules/cardano-node/instance.nix
Outdated
| script = '' | ||
| # Show commandline before execution | ||
| set -x | ||
| exec ${getBin instance.package}/bin/cardano-node run ${toGNUCommandLineShell instance.options} |
There was a problem hiding this comment.
| exec ${getBin instance.package}/bin/cardano-node run ${toGNUCommandLineShell instance.options} | |
| exec ${getExe instance.package} run ${toGNUCommandLineShell instance.options} |
| systemd.services = | ||
| { | ||
| # default systemd service to control them all (FIXME: now a stub) | ||
| # FIXME: just rename "cardano-node-${instance.name}" to cardano-node in case of single node? |
There was a problem hiding this comment.
just rename "cardano-node-${instance.name}" to cardano-node in case of single node?
if it's easier for you I also like having always a default service that orchestrate other ones (optionally only one)
| }; | ||
| }; | ||
|
|
||
| config = mkIf (length (attrNames cfg.instances) > 0) { |
There was a problem hiding this comment.
| config = mkIf (length (attrNames cfg.instances) > 0) { | |
| config = mkIf (attrNames cfg.instances != []) { |
|
Also somehow docs are broken now (nix doesn't evaluate). So at the end we are not re-using at all the upstream module, right? I accept the choice, just wanted to be sure. Anyway it would be nice at least providing the same options (I mean, the same customization possibilities) that they allow. |
avnik
force-pushed
the
avnik/node-module
branch
from
8cba274 to
b8f07d7
Compare
avnik
force-pushed
the
avnik/node-module
branch
from
b8f07d7 to
fe868f7
Compare
|
Thank you! |
No description provided.