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

Redmine server options #85003

Closed
wants to merge 3 commits into from
Closed

Conversation

pvgoran
Copy link
Contributor

@pvgoran pvgoran commented Apr 11, 2020

Motivation for this change

This PR consists of three commits:

  1. The primary enhancement that is introduced here is the prefix option that allows to specify the URL prefix where Redmine will be served. To allow it to work under, say, https://example.org/redmine instead of https://example.org/ . This commit also includes quality-of-life enhancements: the serverAccessLog option that allows to disable access logging that WEBrick (HTTP server that runs/hosts Redmine) performs, and addition of the SyslogIdentifier setting for the systemd service.
  2. The listen.address option was added to allow specifying the IP address to listen to (which is quite desirable for security purposes); for unification, the port option was renamed to listen.port.
  3. The server option was introduced, which allows to use the Puma HTTP server instead of WEBrick. There is a wide-spread opinion that WEBrick is badly suited for production - although I don't know if it's actually true. Also, in my configuration WEBrick had a specific problem: its access logs show IP address of reverse proxy, rather than IP address of the actual client, whereas Puma shows the client's IP address reported by reverse proxy.

The third commit has a potential problem. It relies on the puma gem, but its presence is not guaranteed. The Gemfile of Redmine's package includes it in the :test group, with the comment "For running system tests". At the minimum, this gem's specification needs to be moved to the root level of the Gemfile with the appropriate comment. It would be even better to somehow ensure its presence in the redmine module; I don't know if it's possible or not.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Sorry, something went wrong.

pvgoran added 3 commits April 11, 2020 21:25
Also, SyslogIdentifier added for the systemd service.
This option allows to choose which Ruby HTTP server runs Redmine: WEBrick or Puma.
@ofborg ofborg 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/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Apr 11, 2020
@aanderse
Copy link
Member

@pvgoran looking good 👍 I'll test this in the next week or two.

If you're a redmine user do you have capacity and ability to test this: #84823 as well? Of specialnote to you is the following line in https://github.com/aanderse/nixpkgs/blob/8eefb7ce147bccc9ef4a17175455fe0bc9bcaab1/pkgs/applications/version-management/redmine/default.nix

groups = [ "development" "ldap" "markdown" "minimagick" "openid" "test" ];

@aanderse aanderse self-assigned this Apr 12, 2020
@pvgoran
Copy link
Contributor Author

pvgoran commented Apr 14, 2020

@pvgoran looking good +1 I'll test this in the next week or two.

If you're a redmine user do you have capacity and ability to test this: #84823 as well?

I'll try to test it during this week.

Of specialnote to you is the following line in https://github.com/aanderse/nixpkgs/blob/8eefb7ce147bccc9ef4a17175455fe0bc9bcaab1/pkgs/applications/version-management/redmine/default.nix

groups = [ "development" "ldap" "markdown" "minimagick" "openid" "test" ];

Did you mean to say that this line ensures that the test group's gem are included in nixpkgs' redmine package?

@aanderse
Copy link
Member

@pvgoran I'm not really familiar enough with ruby to give an educated answer on that. Maybe you can comment on that PR as I've already posed a question like that to @manveru and am hoping to hear back.

@ryantm ryantm added 2.status: merge conflict This PR has merge conflicts with the target branch and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Oct 3, 2020
@stale
Copy link

stale bot commented Jun 7, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 7, 2021
@pvgoran
Copy link
Contributor Author

pvgoran commented Jun 7, 2021

I need to bring this up to date one day...

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 7, 2021
@stale
Copy link

stale bot commented Jan 9, 2022

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 9, 2022
@wegank wegank marked this pull request as draft March 20, 2024 15:40
@ghost ghost closed this Dec 21, 2024
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 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/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants