Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Config][FrameworkBundle/Translation][Router] Interchangeable ConfigCache#5912
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| private function getConfigCacheFactory() | ||
| { | ||
| if (!$this->configCacheFactory) { | ||
| $this->configCacheFactory = new \Symfony\Component\Config\ConfigCacheFactory($this->options['debug']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Please add this as a use above and refer to the alias.
Made some fixes, let's wait for the next Travis build |
The problem with tests is that now the service "translator.default" has a dependency on a non-existent service "config.config_cache_factory". |
| @@ -70,6 +71,9 @@ | |||
| </argument> | |||
| <argument type="service" id="router.request_context" on-invalid="ignore" /> | |||
| <argument type="service" id="logger" on-invalid="ignore" /> | |||
| <call method="setConfigCacheFactory"> | |||
| <argument type="service" id="config.config_cache_factory" /> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
wrong service id here
…ory altogether, as Router and Translator will still fall back to old behaviour (BC).
I removed all the new service definitions as all code will by default behave as before (creating default implementations). Only clients that want to change the factory implementation need to inject the right (new) one and can do so in a compiler pass. |
It is a common pattern for clients of ConfigCache/ConfigCacheFactory to check whether the cache is fresh and if not, fill it. This removes a little bit of repeated code.Additionally, it gives factory implementations more control over how the cache is written to (for example they can employ other file locking/atomic write strategies).
@fabpot Do you think this PR is a worthwhile improvement that has any chance of being merged at all? Otherwise we'd need to find other ways to solve the problem in our projects. |
Closing for the most recent work on this topic in#7230 |
We would like to use a custom implementation of ConfigCache to employ a modified caching strategy. Basically, it is about special kinds of resources that might change on runtime also during production (will post more details if anyone is interested).
Currently the ConfigCache implementation is not exchangeable, which this what this PR addresses. I have tried to keep BC as far as possible. If that is not a hard requirement, I could also come up with different implementations.
Alternatively, a factory method for the ConfigCache in client classes could do.
Additionally, the kernel uses ConfigCache for the service container itself. We did not touch that part because there is probably no useful way to configure or inject alternative implementations before the container itself is available.