Skip to content

Comments

nixos/lubelogger: init#371458

Merged
MattSturgeon merged 2 commits intoNixOS:masterfrom
bct:lubelogger-module
Nov 19, 2025
Merged

nixos/lubelogger: init#371458
MattSturgeon merged 2 commits intoNixOS:masterfrom
bct:lubelogger-module

Conversation

@bct
Copy link
Contributor

@bct bct commented Jan 6, 2025

Adds a module for LubeLogger, and adds myself as maintainer.

Closes #333954

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` labels Jan 6, 2025
@bct
Copy link
Contributor Author

bct commented Jan 6, 2025

This is mostly #335653 , with a few modifications:

  • fixes "cannot coerce integer to a string"
  • adds an environmentFile option, for setting sensitive environment variables
  • updates the /var/lib/lubelogger/wwwroot symlinks when the nix package is updated
  • hardens the systemd configuration

@bct bct force-pushed the lubelogger-module branch from 9486491 to b9d457b Compare January 6, 2025 14:26
@github-actions github-actions bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Jan 6, 2025
@nix-owners nix-owners bot requested a review from Lyndeno January 6, 2025 14:26
@bct bct changed the title Lubelogger module nixos/lubelogger: init Jan 6, 2025
@Lyndeno
Copy link
Contributor

Lyndeno commented Jan 6, 2025

I'll have a look at this later, I am happy to also be added to the maintainers field for the module.

@bct
Copy link
Contributor Author

bct commented Jan 6, 2025

👍 Only changed the maintainer because I wasn't sure you were still up for it.

Unfortunately I pulled at a thread and discovered that this module is not working correctly. Configuration changes get lost when the service is restarted - it's loading userConfig from the ContentRoot in the /nix/store, not from /var/lib/lubelogger.

I've put up a PR for this issue here: hargata/lubelog#781

@bct bct force-pushed the lubelogger-module branch from b9d457b to 974d0a1 Compare January 9, 2025 05:14
@bct
Copy link
Contributor Author

bct commented Jan 10, 2025

Putting this on hold until 1.4.3 comes out - it's going to remove the need to mix read-only and read-write paths.

@bct bct force-pushed the lubelogger-module branch 3 times, most recently from c5f56a3 to fa41375 Compare January 11, 2025 15:06
@h7x4 h7x4 added the 8.has: module (new) This PR adds a module in `nixos/` label Jan 17, 2025
@Lyndeno
Copy link
Contributor

Lyndeno commented Jan 22, 2025

Putting this on hold until 1.4.3 comes out - it's going to remove the need to mix read-only and read-write paths.

Check out #375887 and let me know what you think

@bct bct force-pushed the lubelogger-module branch from fa41375 to c455022 Compare January 22, 2025 21:55
@bct
Copy link
Contributor Author

bct commented Jan 23, 2025

This is ready to go.

@Lyndeno
Copy link
Contributor

Lyndeno commented Jan 23, 2025

I think we should have a line in the 25.04 release notes for this

@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/2218

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Feb 15, 2025
Copy link
Member

Choose a reason for hiding this comment

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

Does it matter if localhost resolves to ipv4 or ipv6?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On my system curl '[::1]:5000' and curl 127.0.0.1:5000 both work.

Copy link
Contributor Author

@bct bct Mar 31, 2025

Choose a reason for hiding this comment

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

https://learn.microsoft.com/en-us/aspnet/core/fundamentals/servers/kestrel/endpoints?view=aspnetcore-9.0

When localhost is specified, Kestrel attempts to bind to both IPv4 and IPv6 loopback interfaces.

Copy link
Contributor

@Lyndeno Lyndeno left a comment

Choose a reason for hiding this comment

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

I agree with Sandro's suggestions

@Lyndeno
Copy link
Contributor

Lyndeno commented Mar 24, 2025

Is there anything else I can do to support this? Would be great to see this in the 25.05 release.

@bct bct force-pushed the lubelogger-module branch from c455022 to aa17fd0 Compare March 29, 2025 14:19
@github-actions github-actions bot added 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation labels Mar 29, 2025
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 29, 2025
@github-actions github-actions bot added the 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. label Mar 29, 2025
@bct
Copy link
Contributor Author

bct commented Mar 29, 2025

Thanks for the push - updated.

@bct bct requested a review from Lyndeno March 29, 2025 14:54
@bct bct requested a review from SuperSandro2000 March 29, 2025 20:28
@wegank wegank added 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. 12.approvals: 1 This PR was reviewed and approved by one person. labels Mar 29, 2025
@Lyndeno
Copy link
Contributor

Lyndeno commented Apr 9, 2025

@SuperSandro2000 is there anything else needed to get this through?

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jun 9, 2025
@nixpkgs-ci nixpkgs-ci bot removed the 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. label Jun 25, 2025
@bct bct force-pushed the lubelogger-module branch from aa17fd0 to 6fc0769 Compare July 17, 2025 14:06
@nixpkgs-ci nixpkgs-ci bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 17, 2025
@bct
Copy link
Contributor Author

bct commented Jul 17, 2025

Rebased & moved the release notes to 25.11.

@Lyndeno
Copy link
Contributor

Lyndeno commented Oct 25, 2025

LGTM

@jmartindf
Copy link
Contributor

I would like to have both this PR and #453649 make it into NixOS 25.11, giving us LubeLogger 1.5.4 and an associated module to run it.

Copy link
Contributor

@MattSturgeon MattSturgeon left a comment

Choose a reason for hiding this comment

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

I have not tested anything, but LGTM.

@bct bct force-pushed the lubelogger-module branch from 6fc0769 to 8b46cc4 Compare November 16, 2025 19:47
@bct bct force-pushed the lubelogger-module branch from 8b46cc4 to bbab9f5 Compare November 16, 2025 19:54
Copy link
Contributor

@MattSturgeon MattSturgeon left a comment

Choose a reason for hiding this comment

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

Sorry, I think #371458 (comment) got misinterpreted slightly?

@bct bct force-pushed the lubelogger-module branch from bbab9f5 to 99f53db Compare November 16, 2025 20:17
Copy link
Contributor

@MattSturgeon MattSturgeon left a comment

Choose a reason for hiding this comment

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

There are a couple minor things I should've spotted in my earlier review. Still LGTM otherwise.

@bct bct force-pushed the lubelogger-module branch from 99f53db to 8cbcf5b Compare November 19, 2025 13:51
@MattSturgeon MattSturgeon added this pull request to the merge queue Nov 19, 2025
@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Nov 19, 2025
Merged via the queue into NixOS:master with commit e5f988f Nov 19, 2025
29 of 32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 8.has: module (new) This PR adds a module in `nixos/` 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lubelogger runs but several features (image & document uploads, authentication, etc.) are non-functional

8 participants