-
Notifications
You must be signed in to change notification settings - Fork 40
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
Add ParsonLabs Music to Application Catalog #1074
base: master
Are you sure you want to change the base?
Conversation
WillKirkmanM
commented
Nov 29, 2024
•
edited
Loading
edited
…mes for ParsonLabs Music
… + Network Certificates for HTTPS
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.
Hello, this does not seem to work at all.
I cannot even render the template.
Image is recommended to use a versioned tag and not latest.
Port is static so users cannot edit it. Also it would be nice if internal container port is also changed via env variable to match the external one.
So users can also use host network without chances of port conflicts.
certificate is not used anywhere
container name is different in the labels section than the one that is used to create the container.
values are statically set in the compose file instead of using the values file.
For readme I'd suggest having a small description like the rest of the apps,
and have a link to your github for the rest. (as the rest of the apps)
app.yaml file should be the same format as the rest, as it is modified from automated tools and will create big diffs on subsequent automated PRs
questions.yaml seems to have a user/group group
defined at the top but is not used anywhere.
Is the container running as root or rootless?
Based on the compose it seems to run as rootless, but uses cannot edit that. as you reference it from the consts.
Thanks for the review, will fix. |