-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
nixos/seaweedfs: init #353890
base: master
Are you sure you want to change the base?
nixos/seaweedfs: init #353890
Conversation
f984c57
to
ed49f65
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks quite ok, some initial feedback.
017e2f1
to
ebed001
Compare
Some information in how to use : This is for single Node use :
This is for multi Node use :
and add in first node :
For mount seaweedfs :
|
b3fc391
to
319d5fa
Compare
ff92140
to
0c2343f
Compare
While I haven’t reviewed the module, I’d recommend writing a nixos test for this and linking it to the package. |
You can test module with :
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've fixed the PR title, please update the commit to match it.
StateDirectory = "seaweedfs"; | ||
RuntimeDirectory = "seaweedfs"; | ||
Restart = "always"; | ||
RestartSec = "60s"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we really increase the default?
description = "Prometheus metrics listen port."; | ||
}; | ||
|
||
tomlConfig = lib.mkOption { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tomlConfig = lib.mkOption { | |
config = lib.mkOption { |
|
||
defaultReplicaPlacement = lib.mkOption { | ||
type = lib.types.str; | ||
default = ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default = ""; | |
default = cfg.<...>; |
should we make this clear like that?
Why not add this test to the tree in this PR? |
5f245c5
to
fcc4be8
Compare
Because I am not accustomed to the practice of nixos with regard to tests. |
8f8883e
to
fd1c0db
Compare
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-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.