-
Notifications
You must be signed in to change notification settings - Fork 45
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
Dockerfile rebuild #1881
Dockerfile rebuild #1881
Conversation
This is missing a line to set the pool count for the database. I added it locally, but i'm still getting an error in my logs
I'm finally setting up debugging so I can track down if this is related to a config issue on my part. The impact seems to be that previews intermittently don't load. Edit: I fixed this. The entrypoint file was not setting the pool configuration item in database.yml |
This is fantastic work, thank you! The use of I'll roll an |
Thanks! As for the PUID/PGID options in the docker-compose file they might be redundant? I really like linuxserver images, but i'm not sure how they are doing those assignments. I'll have to poke around. I think they might be redundant here, because i'm statically the manyfold user to 1000:1000 in the dockerfile, so setting it the docker-compose feels like it probably isn't doing anything. At least the files are no longer generated as root, so that's nice. |
Code Climate has analyzed commit 12b4975 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 73.6% (0.0% change). View more on Code Climate. |
Oh, yes, I see they're not actually used! I think they could probably replace the hardcoded numbers in the adduser line. |
I've finally had time to take a look at this, and now there are some conflicts because the PR does a few things that have already been merged in - sorry! I'm probably going to have to pull it apart a bit in order to merge in what's still needed, bear with me :) |
I took a different approach to this, using
I think the main missing thing you've got in here is the multi-database support by adding them into I'll convert this to draft if you don't mind, because I don't think it will ever merge as-is, and it will help me remember :) |
Checklist
🚨 Please review the guidelines for contributing to this repository. 🚨
Warnings
Summary
This allows the docker to run the application as an arbitrary non-root user (currently set to 1000:1000)
Linked issues
resolves #1704
Description of changes
I mangled and salvaged a majority of this from the redmine project.
I've tested it and pared down extraneous code, but I'm sure that there are still parts that are unnecessary. This shouldn't break anything, but let me know if you notice anything.