Conversation
|
@roblabla, thanks for your PR! By analyzing the annotation information on this pull request, we identified @edolstra, @joachifm and @offlinehacker to be potential reviewers |
|
Looks like this needs a rebase |
There was a problem hiding this comment.
What is the use case for making this configurable?
There was a problem hiding this comment.
Having the "package" configurable allows using a different version of the package with the service unit. I believe most of the packages have it ?
There was a problem hiding this comment.
But why is that useful in this case. What I'm getting at is that adding options just because you can should be avoided.
There was a problem hiding this comment.
That said, it's up to you of course, you know the service better than I.
There was a problem hiding this comment.
I often have packages that are outdated on nixos. Being able to quickly override the package from my configuration.nix allows me to quickly test if a new version will need some changes to the nix definition or not.
TBH, I can remove this either way. I included it mostly because it seems to be there in most other nix services. I've often found it useful, so I kind of just always put it in my packages.
There was a problem hiding this comment.
I think your use case would be covered by packageOverrides. I've no idea how many existing services have a legitimate use case for this type of option, but I know a fair amount have not.
There was a problem hiding this comment.
I don't understand how packageOverrides would fix my problem. As far as I understand, packageOverride works for the "nix-env" use-case, but it doesn't modify the package that will be used for the NixOS service ?
How can I create a nixos configuration.nix that enables a service but uses a different version of the package using packageOverrides ?
There was a problem hiding this comment.
Package overrides is not really about nix-env, it is a general mechanism for extending/changing the package set.
For example,
nixpkgs.config.packageOverrides = super: {
telegraf = super.callPackage ./my/newer/telegraf.nix {};
}
ought to affect uses of pkgs.telegraf in the service definition. You should be able to do the same via ~/.nixpkgs/config.nix.
roblabla
force-pushed
the
feature-addTelegraf
branch
from
33dcd9b to
9fbdb31
Compare
| systemd.services.telegraf = { | ||
| description = "Telegraf Agent"; | ||
| wantedBy = [ "multi-user.target" ]; | ||
| after = [ "network-interfaces.target" ]; |
There was a problem hiding this comment.
Is there a reason why you set this to network-interfaces.target instead of networking.target?
There was a problem hiding this comment.
@groxxda good point. This target is actually used by quite a few services ... From the looks of it, network-interfaces is akin to an implementation detail and shouldn't be referenced directly by ordinary services (which are probably best served by using special targets for ordering and whatnot).
There was a problem hiding this comment.
@joachifm a follow up to #18319 is on my to-do to get rid of network-interfaces.target since so many targets tend to confuse everyone. upstream network targets should really be enough for ordinary services.
feel free to assist 😉
There was a problem hiding this comment.
Sure: see https://github.com/joachifm/nixpkgs/tree/network-interfaces for some wip
There was a problem hiding this comment.
@groxxda it's a fairly mechanical translation, so needs a bit of sanity checking
There was a problem hiding this comment.
@joachifm sorry for not notifying you, I also started the work on this https://github.com/groxxda/nixpkgs/commits/network-interfaces
As you said, most is mechanical translation, but there are one or two exceptions.
Also I did a batch of changes to scripted-interfaces because I think ordering is broken there. But this needs more review and was blocking me from a pr. I'll push the current wip state soon
I'll have a look at your changes now
|
I'm with @joachifm on the |
|
Superceded by #18437 |
4 participants
Motivation for this change
Adds telegraf, a plugin-driven server agent for collecting & reporting metrics commonly used with influxdb as a more flexible replacement for collectd.
Things done
(nix.useChroot on NixOS,
or option
build-use-chrootinnix.confon non-NixOS)
nix-shell -p nox --run "nox-review wip"./result/bin/)