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.

Sign up for GitHub

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

Jump to bottom

uptime-kuma: init package and module #189807

Merged
merged 2 commits into from
Oct 23, 2022
Merged

uptime-kuma: init package and module #189807

merged 2 commits into from
Oct 23, 2022

Conversation

JulienMalka
Copy link
Member

@JulienMalka JulienMalka commented Sep 5, 2022 •
edited

Description of changes

This adds the uptime-kuma package and module. Uptime-kuma is a fancy self-hosted monitoring tool.

This PR also contain a nixos test for the uptime-kuma module, testing basic functionality (getting http response).

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 22.11 Release Notes (or backporting 22.05 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@JulienMalka JulienMalka force-pushed the uptime-kuma branch 2 times, most recently from 61d2aff to b1acd70 Compare September 5, 2022 06:19
@JulienMalka JulienMalka force-pushed the uptime-kuma branch 2 times, most recently from e4ff1b9 to 6d26920 Compare September 5, 2022 12:45
@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/1174

SuperSandro2000
SuperSandro2000 reviewed Sep 5, 2022
View reviewed changes
pkgs/servers/monitoring/uptime-kuma/pingbinarypath.patch Outdated Show resolved Hide resolved
pkgs/servers/monitoring/uptime-kuma/default.nix Outdated Show resolved Hide resolved
pkgs/servers/monitoring/uptime-kuma/default.nix Outdated Show resolved Hide resolved
pkgs/servers/monitoring/uptime-kuma/default.nix Outdated Show resolved Hide resolved
pkgs/servers/monitoring/uptime-kuma/default.nix Outdated Show resolved Hide resolved
pkgs/servers/monitoring/uptime-kuma/.npmrc
@@ -0,0 +1 @@
legacy-peer-deps=true
Copy link
Member

Choose a reason for hiding this comment

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

Is that required for the update script? Can't we supply that via a argument?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes it is required, if I delete .npmrc I get this kind of errors : request to https://registry.npmjs.org/chart.js failed: cache mode is 'only-if-cached' but no cached response is available.
We may be able to supply it via argument but I have no idea how to achieve that.

Copy link
Member

Choose a reason for hiding this comment

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

We may be able to supply it via argument but I have no idea how to achieve that.

We need a custom update script anyway so we should supply it there.

Copy link
Member Author

Choose a reason for hiding this comment

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

@SuperSandro2000 I've added an update script. Sorry I wasn't clear earlier, the .npmrc file is required for building the package, not for the update script.

nixos/modules/services/monitoring/uptime-kuma.nix Outdated Show resolved Hide resolved
nixos/modules/services/monitoring/uptime-kuma.nix Outdated Show resolved Hide resolved
nixos/modules/services/monitoring/uptime-kuma.nix Outdated Show resolved Hide resolved
nixos/modules/services/monitoring/uptime-kuma.nix Outdated Show resolved Hide resolved
@JulienMalka JulienMalka force-pushed the uptime-kuma branch 3 times, most recently from 3e8d27a to b81db18 Compare September 6, 2022 21:09
@JulienMalka
Copy link
Member Author

@RaitoBezarius @SuperSandro2000 thanks for your reviews! I have implemented your suggestions. I have also bumped the package to version 1.18.0 (released yesterday).

@JulienMalka JulienMalka force-pushed the uptime-kuma branch 7 times, most recently from 812e071 to de59bd2 Compare September 19, 2022 14:34
@JulienMalka JulienMalka mentioned this pull request Sep 24, 2022
Closed
2 tasks
aanderse
aanderse approved these changes Sep 25, 2022
View reviewed changes
Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

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

Module LGTM 👍

RaitoBezarius
RaitoBezarius approved these changes Sep 25, 2022
View reviewed changes
Copy link
Member

@RaitoBezarius RaitoBezarius left a comment

Choose a reason for hiding this comment

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

New changes LGTM.

@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-already-reviewed/2617/617

SuperSandro2000
SuperSandro2000 reviewed Sep 28, 2022
View reviewed changes
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

How about we use a vendorHash isntead of 29k lines of lock file?

pkgs/servers/monitoring/uptime-kuma/update.sh Show resolved Hide resolved
pkgs/servers/monitoring/uptime-kuma/update.sh Outdated Show resolved Hide resolved
pkgs/servers/monitoring/uptime-kuma/fix-database-permissions.patch Outdated Show resolved Hide resolved
@JulienMalka
Copy link
Member Author

How about we use a vendorHash isntead of 29k lines of lock file?

@SuperSandro2000 I'm not sure to know what you're talking about. I've seen references about vendorHash only in the context of go modules.

@JulienMalka
Copy link
Member Author

@SuperSandro2000 I've made the changes you've suggested, I think we can merge if everything look fine to you

06kellyjac
06kellyjac reviewed Oct 3, 2022
View reviewed changes
Copy link
Member

@06kellyjac 06kellyjac left a comment

Choose a reason for hiding this comment

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

Currently using uptime-kuma via podman but it'd be nice to have a module 🚀

I've added some comments

Also please update the PR title to at least have uptime-kuma matching the package name rather than Uptime Kuma

pkgs/servers/monitoring/uptime-kuma/default.nix Outdated Show resolved Hide resolved
pkgs/servers/monitoring/uptime-kuma/default.nix Outdated Show resolved Hide resolved
pkgs/servers/monitoring/uptime-kuma/default.nix Outdated Show resolved Hide resolved
pkgs/servers/monitoring/uptime-kuma/default.nix Outdated Show resolved Hide resolved
pkgs/servers/monitoring/uptime-kuma/package.json Show resolved Hide resolved
nixos/doc/manual/release-notes/rl-2211.section.md Show resolved Hide resolved
@JulienMalka JulienMalka changed the title Uptime kuma: init package and module uptime-kuma: init package and module Oct 3, 2022
@Mindavi
Copy link
Contributor

Mindavi commented Oct 5, 2022

Could you squash into 2 commits? Otherwise the large files are still part of the history, which I'd like to prevent

@JulienMalka
Copy link
Member Author

Could you squash into 2 commits? Otherwise the large files are still part of the history, which I'd like to prevent

@Mindavi I'll squash, I'm not completely done taking into account the remarks from @06kellyjac
Do you see any solution to prevent committing the big files ? I guess it's the problem with nodejs projects...

@Mindavi
Copy link
Contributor

Mindavi commented Oct 6, 2022

Oops, I totally misread and thought those files were not needed anymore :). Then it doesn't matter too much. Just make sure to adhere to guidelines regarding commit messages then, then it's all good. Nothing to do about the large files I guess

@JulienMalka
Copy link
Member Author

Oops, I totally misread and thought those files were not needed anymore :). Then it doesn't matter too much. Just make sure to adhere to guidelines regarding commit messages then, then it's all good. Nothing to do about the large files I guess

No problem, I'll finish taking into account the last comments that I got and then squash the commits :)

@JulienMalka JulienMalka force-pushed the uptime-kuma branch 2 times, most recently from 434c919 to 0830027 Compare October 15, 2022 21:51
@JulienMalka
Copy link
Member Author

I think I addressed all the remarks. @Mindavi @06kellyjac if you want to have another look !

@Mindavi Mindavi merged commit b54ae5a into NixOS:master Oct 23, 2022
@JulienMalka JulienMalka deleted the uptime-kuma branch October 23, 2022 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RaitoBezarius RaitoBezarius RaitoBezarius approved these changes

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@Mindavi Mindavi Mindavi left review comments

@06kellyjac 06kellyjac 06kellyjac left review comments

@aanderse aanderse aanderse approved these changes

Assignees
No one assigned
Labels
6.topic: nixos 8.has: changelog 8.has: documentation 8.has: module (update) 8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@JulienMalka @nixos-discourse @Mindavi @RaitoBezarius @SuperSandro2000 @aanderse @06kellyjac @bobby285271