Loosen APCu check for SourceCache #3
Conversation
…t cache and do not throw an exception - so that php-di does not break if there is any problem with apcu - instead of having to wrap ContainerBuilder::build() with try...catch block
milanorszagh
left a comment
milanorszagh
left a comment
There was a problem hiding this comment.
Looks good to me.
Was there a reason why we wanted to throw an exception in this case before?
|
@milanorszagh the original PHP-DI throws the exception, and I'd prefer it not to |
| if (!SourceCache::isSupported()) { | ||
| throw new \Exception('APCu is not enabled, PHP-DI cannot use it as a cache'); | ||
| } | ||
| // use cache if isSupported check passes, otherwise proceed without cache and do not throw an exception |
There was a problem hiding this comment.
I like this change but if there was an exception before then this change may break BC because someone may expect this exception for some cases...
I mean that exception was throwing when someone was trying to use APCu cache without enabled php extension. The new version just skipping the cache layer.
Maybe we should notify users somehow about that:
trigger_error("You're trying to use APCu that is not available", E_USER_WARNING);Of course adding such message still will break BC
There was a problem hiding this comment.
yeah, since this is a BC I see small chances for it to be merged into original repo. But still it might be more convenient to just leave sourceCache turned on and expect the things to work (without cache), even if apcu breaks - the project should not crash in such case - and it should not be required to wrap $container->build() with try...catch in every implementation
3 participants
Use sourceCache if isSupported check passes, otherwise proceed without cache and do not throw an exception