Skip to content
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/gatus: init module #294469

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

nixos/gatus: init module #294469

wants to merge 2 commits into from

Conversation

pizzapim
Copy link

@pizzapim pizzapim commented Mar 9, 2024

Description of changes

In #269477, the Gatus package was added to nixpkgs. I created a NixOS module to run Gatus as a Systemd service. I have been running this myself for a week with no problems. You can see on my personal Git server how this can be used: here I define Gatus endpoints which are services I monitor, and here I configure the service itself.

This is my first contribution to nixpkgs so any comments are greatly appreciated!

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@NyCodeGHG
Copy link
Member

Hi! Welcome and thank you for your first contribution :)

Please squash your commits into a single commit and use a proper commit message.
See the commit conventions

@pizzapim pizzapim changed the title Add Gatus NixOS module nixos/gatus: init module Mar 16, 2024
nixos/modules/services/monitoring/gatus.nix Outdated Show resolved Hide resolved
nixos/modules/services/monitoring/gatus.nix Outdated Show resolved Hide resolved
nixos/modules/services/monitoring/gatus.nix Outdated Show resolved Hide resolved
nixos/modules/services/monitoring/gatus.nix Outdated Show resolved Hide resolved
nixos/modules/services/monitoring/gatus.nix Outdated Show resolved Hide resolved
nixos/modules/services/monitoring/gatus.nix Outdated Show resolved Hide resolved
nixos/modules/services/monitoring/gatus.nix Outdated Show resolved Hide resolved
@NyCodeGHG
Copy link
Member

I noticed that gatus also supports postgresql as a storage backend, an option for that would be nice, but could also be done in a future PR (and also by someone else, if you don't plan to use it).

@fsnkty
Copy link
Member

fsnkty commented Mar 16, 2024

none of this is killer so felt iffy for review buuut
mdDoc is pointless and will be removed tree wide eventually anyway.
restart always may be better as unless stopped?
and personal preference not everyone seems to agree with, another inherit for (lib.types) foo bar would be nicer than the using with at all
also why not add yourself as a maintainer?

@pizzapim
Copy link
Author

I noticed that gatus also supports postgresql as a storage backend, an option for that would be nice, but could also be done in a future PR (and also by someone else, if you don't plan to use it).

Indeed, for my use case SQLite is more than enough, so I'd say we can leave this for a future PR of (probably) someone else :) Does seem like a good idea though.

@pizzapim pizzapim force-pushed the gatus branch 3 times, most recently from 40a539a to 9235a7f Compare March 18, 2024 19:17
@pizzapim
Copy link
Author

Thanks for all the reviews! Unless there are further comments I think this might be ready for a merge?

@NyCodeGHG
Copy link
Member

i have one small thing, would be nice if you could create an extra commit for creating your maintainer in maintainer-list.nix
with the commit message maintainers: add pizzapim, other than that this should be ready to merge :)

@pizzapim
Copy link
Author

Done and done!

Copy link
Member

@NyCodeGHG NyCodeGHG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@patcher-ms
Copy link

Hello @pizzapim thanks for the work you put in this PR. What’s left to do before it gets merged?

@NyCodeGHG
Copy link
Member

The merge conflict needs to be resolved by rebasing on master, then it should be ready to merge

@pizzapim pizzapim force-pushed the gatus branch 2 times, most recently from 819aee9 to b46c362 Compare May 28, 2024 19:42
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/4005

@patcher-ms
Copy link

@drupol, sorry for the ping. The changes you requested have been made.
Could you please have another look. Is there anything else blocking this from being merged?

@@ -95,6 +95,8 @@ The pre-existing [services.ankisyncd](#opt-services.ankisyncd.enable) has been m

- [GNS3](https://www.gns3.com/), a network software emulator. Available as [services.gns3-server](#opt-services.gns3-server.enable).

- [Gatus](https://github.com/TwiN/gatus), an automated developer-oriented status page. Available as [services.gatus](#opt-services.gatus.enable).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this supposed to nixos/doc/manual/release-notes/rl-2411.section.md ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants