Allow providers to be loaded lazy #113
Conversation
|
@chregu Can you add a test for this? |
|
Tests added |
| { | ||
| $container = $this->compileContainer('lazy'); | ||
|
|
||
| $providers = array( |
There was a problem hiding this comment.
Do you know about the phpunit data provider ?
It would allow you to get rid of the foreach.
There was a problem hiding this comment.
And if the first test fail you still see that the other pass.
There was a problem hiding this comment.
The other tests in the same class do it the exact same way.
|
@chregu making the service lazy still does not give you any guarantee that the instantiation of the service will happen inside your try/catch |
|
Maybe. But for me it's better than "doesn't happen inside a try/catch block at all". |
|
Having the option to lazy load these is very helpful. If chregu wouldn't have made the PR I would have. Tried the branch and works really well for me. |
|
TBH, I believe this is the wrong approach to solve the problem described by @chregu. If you want to avoid exceptions e.g. if the redis server is unavailable, you can write a wrapper adapter that simply reverts to NOP for set/get calls and behaves as if there was no cache present (with all the problems go along with it). Deferring the exception to the set/get stage is a bad idea IMO. What do you think @stof @mikeSimonson? |
|
isn't the root problem that a cache service should never ever throw exceptions in production mode? if redis is gone, the adapter should simply report it did not find the cached information. on construction it should also never explode. i guess during testing / development we want to know when things fail, they could be misconfigured or whatever. but during production, logging is fine but exceptions is not good. maybe inject the kernel.debug option as a flag whether errors should lead to exceptions? |
|
I agree - no point pursuing this any further. |
Currently, providers are not loaded lazy and there's no easy way (or I didn't find out) to let them load lazy. The problem is, that if you inject such a provider into a class in symfony, there is no easy way to catch an exception happening during constructing the class. And at least in the redis provider, an exception happens simply when the redis server is not reachable (which for a cache shouldn't lead in an not easy catchable exception). When lazy loading the provider, we can handle this case with a common try/catch construct in the get/set calls