-
Notifications
You must be signed in to change notification settings - Fork 330
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
Fix for PHP S2I Opcache configuration #151
Conversation
… of generated 10-opcache.ini file
Can one of the admins verify this patch? |
1 similar comment
Can one of the admins verify this patch? |
+1 |
lgtm, thanks @lucasnetau |
Please run tests, before merging. Tests are failing for 7.0, so image is not built.
@lucasnetau @bparees @andrewklau @remicollet How to fix it? |
opcache.memory_consumption is obviously to small
|
@omron93 Please see #152 which explains the issue. This patch corrected the installation of the opcache profile, I left the defaults as set in the original file. Unfortunately in all my environments I have OPCACHE_MEMORY_CONSUMPTION to values greater than 16 so did not pick up on the issue until after the pull request was merged. Setting OPCACHE_MEMORY_CONSUMPTION=18 or higher will allow the container to run. I've posed a question to the maintainers as to what a sensible default should be. |
@lucasnetau @remicollet Thanks for help and quick fix. |
@remicollet sorry, I saw green and thought it meant the tests had run, didn't notice that the bot was asking for admin approval before running tests. |
Fixes #150 and #149