BUGFIX check for apc.enable_cli if PHP_SAPI==cli #25080
Conversation
b60434c to
e0c5155
Compare
| @@ -23,7 +23,13 @@ | |||
| { | |||
| public static function isSupported() | |||
| { | |||
| return function_exists('apcu_fetch') && ini_get('apc.enabled'); | |||
| if ('cli' === PHP_SAPI) { | |||
There was a problem hiding this comment.
return function_exists('apcu_fetch') && ini_get('cli' === PHP_SAPI ? 'apc.enable_cli' : 'apc.enabled');
e0c5155 to
40e415a
Compare
|
This breaks cache warmups: APCu should be considered as supported when the cache is warmed up on the command line, which is what the current logic provides. |
|
It seems then that this needs to be able to handle that as an exception - maybe we can do something along the lines of: public static function isSupported($checkCLI = true) {...}Or the check on cache warmup should be explicitly checking for |
|
RIght now, I'd think you should to the |
|
Sure - have patched at the SilverStripe level. Would you consider a Warning being thrown, rather than returning |
|
I think the warning is already thrown if you provide a logger. |
|
@andrewandante are you using the ApcuAdapter directly, or do you use it through the default configuration? If via default, I'm wondering if we could/should patch the ChainAdapter to return true when the last adapter succeeds, no matter the other ones. |
|
We are calling it directly - see here: https://github.com/andrewandante/silverstripe-framework/blob/master/src/Core/Cache/DefaultCacheFactory.php#L87 Not sure how that affects your decision on ChainAdapter :) |
neither am I :) |
|
Sure thing. |
Experienced an issue when running PHPUnit via CLI on my application. A slew of errors like:
Issue was that I was missing
apc.enable_cli=1in myapcu.ini- however, APCU was still being detected asisSupported()because the check wasn't taking into account that I was running via CLI. This PR checks based onPHP_SAPI.TODO