-
-
Notifications
You must be signed in to change notification settings - Fork 14k
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
Searxng rework #346777
base: master
Are you sure you want to change the base?
Searxng rework #346777
Conversation
…inx with uwsgi over a socket
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generally looks really good, the explicit nginx configuration is very nice! just a few suggestions on improving this (and of course excluding the changes from the general nginx/uwsgi rework)
upstreams.searx.servers."unix:${config.services.uwsgi.instance.vassals.searx.socket}" = { }; | ||
virtualHosts."${cfg.domain}".locations = { | ||
"/" = { | ||
uwsgiPass = "searx"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there any reason why this needs to be an nginx upstream instead of just configuring uswgiPass = "unix:...";
directly here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question!
It was conflicting with my config but through the user of explicit uwsgiPass we could drop this again as I can cleanly overwrite it.
nginx = lib.mkIf cfg.configureNginx { | ||
extraGroups = [ "searx" ]; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setting systemd.services.nginx.serviceConfig.SupplementaryGroups = ["searx"];
seems like the better option for this
include ${config.services.nginx.package}/conf/uwsgi_params; | ||
''; | ||
}; | ||
"/static/".alias = lib.mkDefault "${config.services.searx.package}/share/static/"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"/static/".alias = lib.mkDefault "${config.services.searx.package}/share/static/"; | |
"/static/".alias = lib.mkDefault "${cfg.package}/share/static/"; |
RemainAfterExit = true; | ||
User = "searx"; | ||
RuntimeDirectory = "searx"; | ||
RuntimeDirectoryMode = "750"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should add RuntimeDirectoryPreserve = "yes";
here to keep the socket dir around over restart/stop-start of searx-init.service
.
After discovering that I had missed adding the uwsgi_params to my nginx config I thought we need an option to add a working nginx config automatically.
While I was at it, I also gave the module a spring clean.
Requires #346776
fyi @999eagle
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.