-
-
Notifications
You must be signed in to change notification settings - Fork 13.8k
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
canaille: init at 0.0.55, add module #333225
base: master
Are you sure you want to change the base?
Conversation
Co-Authored-By: Auguste Baum <[email protected]>
Co-Authored-By: Sofi <[email protected]>
Co-Authored-By: Sofi <[email protected]> Co-Authored-By: Janik <[email protected]>
Co-Authored-By: Janik <[email protected]>
# We can use some kind of fix point for the config anyways, and | ||
# /etc/canaille is recommended by upstream. The alternative would be to use | ||
# a double wrapped canaille executable, to avoid having to rebuild Canaille | ||
# on every config change. |
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 something that could be done upstream to help with this?
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.
Not really, and this is quite the nice solution already. Many webservices have the problem that the config file needs to be independent of the package (to avoid rebuilding the package on config change), but also the config needs to be available to both the executable and the service at runtime. Having a fixpoint (/etc/canaille) solves this.
sqlalchemy-utils | ||
]; | ||
# This isn't defined by upstream actually, but seems to be required. | ||
# Possibly included by using the sqlalchemy[postgresql] extra? |
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 might need to add this upstream indeed. Maybe the sql
extra is too broad and we should provide postgresql
and mysql
extras, or maybe we should just document what extra package are needed for the different DB types 🤔
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.
Yeah, a dedicated postgresql
extra would be great!
# We can use some kind of fix point for the config anyways, and | ||
# /etc/canaille is recommended by upstream. The alternative would be to use | ||
# a double wrapped canaille executable, to avoid having to rebuild Canaille | ||
# on every config change. |
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.
Not really, and this is quite the nice solution already. Many webservices have the problem that the config file needs to be independent of the package (to avoid rebuilding the package on config change), but also the config needs to be available to both the executable and the service at runtime. Having a fixpoint (/etc/canaille) solves this.
sqlalchemy-utils | ||
]; | ||
# This isn't defined by upstream actually, but seems to be required. | ||
# Possibly included by using the sqlalchemy[postgresql] extra? |
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.
Yeah, a dedicated postgresql
extra would be great!
@dotlambda Do you think you can have a look at this? |
Thanks for the review. I'll squash later to keep the commit messages for now. |
Description of changes
The module is already usable and can be deployed as can be seen here: https://canaille.erictapen.name
Most of the options are missing currently, and it would be nice to have a working deployment check in the NixOS test.Addresses:
I'll plan to get this into reviewable shape beginning next week.✔️Pinging people involved so far: @azmeuk @Janik-Haag @fricklerhandwerk (@soupglasses because I used parts of your package definition and you seem to be involved)
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.