-
-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
fosspay: init at unstable-2020-12-02 #119750
base: master
Are you sure you want to change the base?
Conversation
Result of 1 package built successfully:
3 suggestions:
|
b497478
to
c2e4226
Compare
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.
Thanks for choosing to contribute to nixos! It looks like you have a good start but there are a few things I'd like to mention about your module. I decided it might be helpful to hack on your code a little bit and show you my take on the module, which you can see here.
A few points:
- As you mentioned secrets are an issue with the nix store. We should create a nixos option for every type of secret, for the most part.
- I replaced your configuration file stuff with a
settings
option to give more flexibility to the sysadmin. - The user needs to put this behind a reverse proxy like
nginx
. Did you want to configure that for the user, or leave it up to them? - You don't run the
fosspay
cronjob.py
script at all. - I'm not familiar with this software, but you see to diverge from the rough suggestion on how to write the systemd unit. Can you discuss your approach vs using
gunicorn
? - You shouldn't muck around with a users
services.postgresql.authentication
unless you have very good reason to do so. In this case I'm not convinced you do. - You don't allow external databases, but I have put in some code which allows this (though needs to be finished, I just roughed in the idea).
Please let me know if you have any questions or need help. The code I posted was really a starting point and hasn't been tested, etc... It is just to convey a few ideas.
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.
There should be three commits:
maintainers: add pniedzwiedzinski
fosspay: init at unstable-2020-12-02
nixos/fosspay: init
Update:
|
Nice update! A few notes:
|
I marked this as stale due to inactivity. → More info |
Motivation for this change
#119290
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)Testing