vencloud: init at 0.0.1 #374132
vencloud: init at 0.0.1 #374132
Conversation
|
I will fix the build failure when I wake up I am going to head to bed as I have been up all night, I thought I would push this anyways. |
| options = { | ||
| DISCORD_CLIENT_ID = mkOption { | ||
| type = types.nullOr types.int; | ||
| default = null; |
There was a problem hiding this comment.
Can the server still function without any of these set?
If not, it would probably be a good idea to make sure these can't be nullable and require the user to set them -- else you end up with errors at runtime after only setting services.vencloud.enable = true
There was a problem hiding this comment.
I will have to verify this because I am not to sure if this would endup with runtime errors of it not being set, but the problem I run into is having a proper discord application to use for the test.
There was a problem hiding this comment.
Aside from the suggestions here, I'm not sure if this should be included in general. Documentation is nice, but this doesn't really do more than link to the upstream docs -- which is already done in the option descriptions
Ideally this would look more like Garage's docs for example; otherwise I think it'd be fine to omit
There was a problem hiding this comment.
Well in the nixos modules checklist it says to add the docs which i did.
There was a problem hiding this comment.
That isn't a hard requirement. If you want to keep it though, it will definitely need to be expanded, as in it's current form it's not super helpful IMO
There was a problem hiding this comment.
Well I will keep it incase things get deprecated or something drastically changes.
| description = "The client id that should be used for discord OAuth"; | ||
| }; | ||
|
|
||
| DISCORD_CLIENT_SECRET = mkOption { |
There was a problem hiding this comment.
Secrets should be always read from a file.
You could add a clientSecretFile option and use LoadCredential to load it into the systemd service with systemd-creds.
There was a problem hiding this comment.
So read it from builtins.readFile? Can you please evaluate more or provide an example because this is not making sense to me?
There was a problem hiding this comment.
Sure! Sorry, we should really document this pattern somewhere.
Here's an example in nixpkgs:
Something in this direction:
serviceConfig = {
# ...
LoadCredential = "discord-client-secret:${cfg.discordClientSecretFile}"
};
script = ''
export DISCORD_CLIENT_SECRET="$(${lib.getExe' config.systemd.package "systemd-creds"} cat discord-client-secret)"
exec ${lib.getExe cfg.package}
'';nixos/vencloud: init nixos/vencloud: use lib instead of pkgs for maintainers nixos/vencloud: fix tests; nixos/vencloud: fix module nixos/vencloud: reformat meta vencloud: fix tests passthru nixos/vencloud: add release notes nixos/vencloud: applied suggestions from code review Co-authored-by: seth <getchoo@tuta.io> vencloud: applied suggestions from code review Co-authored-by: seth <getchoo@tuta.io> nixos/vencloud: fix trailing space nixos/vencloud: fix formatting
|
Idk what the issue with NixOS manual build can anyone tell me? As I can't use my x86_linux machine because of the power supply sparking (which is causing a fire hazard, I have ordered a new one dw). |
You'll need to update {
"module-services-vencloud": [
"index.html#module-services-vencloud"
],
"module-services-vencloud-basic-usage": [
"index.html#module-services-vencloud-basic-usage"
]
} |
Co-authored-by: Seth Flynn <getchoo@tuta.io>
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.