-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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
fluent-bit: link against Nix dependencies, fix Darwin builds, and add NixOS module #365493
base: master
Are you sure you want to change the base?
Conversation
f956bdc
to
d699387
Compare
ci/OWNERS
Outdated
/nixos/modules/services/monitoring/fluent-bit.nix @samrose @fpletz | ||
/nixos/tests/fluent-bit.nix @samrose @fpletz |
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.
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.
Unfortunately, I'm not using fluent-bit anymore. Please remove me because I can't really maintain it. AFAIK @samrose wasn't really active in the last few years maintaining this package so if he doesn't respond it's probably best to remove him too.
You can add yourself and I'll be happy to review your changes though. 😄
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.
Removed you from the ci/OWNERS
file and the package maintainers list. I'll keep @samrose in the meanwhile.
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.
@tiredofit @elohmeier Either of you interested in being a maintainer for the fluent-bit
package and NixOS module? Saw both of you are using it in public repos and are already in maintainers/maintainers-list.nix
.
Decided to go with Vector instead of Fluent Bit while fixing up the package and adding the NixOS module so I won't be using this anymore.
d699387
to
3a9ac5a
Compare
1a806f0
to
a77fc83
Compare
a77fc83
to
29466a6
Compare
29466a6
to
4f85137
Compare
4f85137
to
95cfa06
Compare
95cfa06
to
50b271d
Compare
bf82913
to
236b9b4
Compare
Please rebase due to the version bump from #366830. |
236b9b4
to
a90c607
Compare
82b291a
to
c5f278a
Compare
c5f278a
to
43fd7da
Compare
''; | ||
example = "/etc/fluent-bit/fluent-bit.yaml"; | ||
}; | ||
configuration = 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.
Let's follow RFC 42 here
configuration = lib.mkOption { | |
settings = 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.
Does the RFC mandate the option be named settings
? Outside of the option name, it seems like this is adhering to the RFC.
Generally I've been trying to expose options with names based on the program's command line options (in this case fluent-bit
uses --config
so I've expanded that to configuration
) or their relevant config file field name (e.g. graceLimit
based on grace
).
description = '' | ||
See {option}`configurationFile`. | ||
|
||
{option}`configurationFile` takes precedence over {option}`configuration`. |
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.
{option}`configurationFile` takes precedence over {option}`configuration`. | |
{option}`configurationFile` takes precedence over {option}`settings`. |
43fd7da
to
4f31d27
Compare
04feac1
to
a178dea
Compare
a178dea
to
5853cde
Compare
Link against Nix dependencies, fix Darwin builds, and add NixOS module (upstreams commiterate/nix-fluent-bit).
fluent-bit vends its dependencies by keeping a copy of their sources under
lib/
in their source tree and building them from source. All of the dependencies their CMake setup can source from the system are already Nix packages (e.g.libbacktrace
,luajit
).Darwin builds have also been silently broken for awhile now.
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.