update nextcloud.nix to change log type #141018
Conversation
enable ability to change logtype for nextcloud
| @@ -84,6 +84,11 @@ in { | |||
| default = 2; | |||
| description = "Log level value between 0 (DEBUG) and 4 (FATAL)."; | |||
| }; | |||
| logType = mkOption { | |||
| type = types.str; | |||
There was a problem hiding this comment.
Please use types.enum with all possible options.
| logType = mkOption { | ||
| type = types.str; | ||
| default = "syslog"; | ||
| description = "log driver used, could be errorlog, file, syslog or systemd"; |
There was a problem hiding this comment.
First character of a sentence is upper, also please terminate the sentence with a ..
There was a problem hiding this comment.
Also, what's the behavior of nextcloud if file is used?
There was a problem hiding this comment.
for file: (https://docs.nextcloud.com/server/latest/admin_manual/configuration_server/logging_configuration.html#file)
By default, a log file named nextcloud.log will be created in the directory which has been configured by the datadirectory parameter in config/config.php
change to enum
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/nextcloud-logging-is-forcibly-configured-to-syslog/8648/3 |
|
Some questions: If logging type is If logging type is |
|
logfile parameter covered, |
| description = "Log driver used for nextcloud logging. Possible options: errorlog, file, syslog or systemd (see https://docs.nextcloud.com/server/latest/admin_manual/configuration_server/logging_configuration.html)."; | ||
| }; | ||
| logFile = mkOption { | ||
| type = types.str; |
There was a problem hiding this comment.
I'd prefer to make this types.nullOr types.str and set it to null by default.
There was a problem hiding this comment.
agree, change it as you like, you know the best practices ;-)
There was a problem hiding this comment.
If we set this to null by default, we also have to amend the strings below ${optionalString ...}.
There was a problem hiding this comment.
I'm open to everything and would be glad if this could be solved soon, because everything is an improvement 😉
|
Another question (not sure if this fits here, but no idea where to ask). Do you think we could add an |
| description = "Log driver used for nextcloud logging. Possible options: errorlog, file, syslog or systemd (see https://docs.nextcloud.com/server/latest/admin_manual/configuration_server/logging_configuration.html)."; | ||
| }; | ||
| logFile = mkOption { | ||
| type = types.str; |
There was a problem hiding this comment.
If we set this to null by default, we also have to amend the strings below ${optionalString ...}.
|
Would love to see this soon upstream, so whats needed to get an approvement? |
The logFile option should be null by default IMHO. |
| 'log_type' => 'syslog', | ||
| 'log_level' => '${builtins.toString cfg.logLevel}', | ||
| 'log_type' => '${cfg.logType}', | ||
| ${optionalString cfg.logFile "'logfile' => '${cfg.logFile}',"} |
There was a problem hiding this comment.
This doesn't evaluate, in any case:
error: value is a string while a Boolean was expected
with logFile being declared and
error: value is null while a Boolean was expected
Please fix this accordingly and test the changes e.g. using a VM test (you can perform a single run via e.g. nix-build nixos/tests/nextcloud -A basic23).
Also, please make sure that your commit messages adhere to our contribution guidelines.
There was a problem hiding this comment.
The current change doesn't evaluate. As long as that's still the case, this cannot be merged.
|
Hi! Is there anything I can do at the moment? It seems this PR is stuck a little bit. As far as I can see, the only thing left is to set the log file to |
|
hey, |
|
After revisiting this I think that in 99% it's preferable to have the logs written to journald. Additional things that aren't super-common should be implemented via #118093. |
enable ability to change logtype for nextcloud
Motivation for this change
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)