Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[FrameworkBundle] Don't rely on any parent definition for "cache.annotations"#20601
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
| if ($container->getParameter('kernel.debug')) { | ||
| $container->addCompilerPass(newAddDebugLogProcessorPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, -1); | ||
| $container->addCompilerPass(newAddDebugLogProcessorPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, -32); |
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.
changing to 32 to leave room for a bit of extensibility
nicolas-grekas commentedNov 24, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
ping@stof for validation/approval please |
| $annotationsPool->addArgument(newReference('monolog.logger.cache')); | ||
| }elseif ($container->has('cache.system')) { | ||
| $systemPool =$container->getDefinition('cache.system'); | ||
| if ($factory ===$systemPool->getFactory() &&5 ===count($systemArgs =$systemPool->getArguments())) { |
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.
wouldn't it be more future-proof to use5 <= ?
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.
updated
9afa586 tof62b820Comparenicolas-grekas commentedNov 24, 2016
Ping @symfony/deciders , we need to make a decision on this one before the end of the week for 3.2 to be ready (3/3). |
nicolas-grekas commentedNov 25, 2016
Last issue on the 3.2 milestone!@stof any comment? |
fabpot commentedNov 26, 2016
Thank you@nicolas-grekas. |
…"cache.annotations" (nicolas-grekas)This PR was merged into the 3.2 branch.Discussion----------[FrameworkBundle] Don't rely on any parent definition for "cache.annotations"| Q | A| ------------- | ---| Branch? | 3.2| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -Instead of a generic approach that failed in#20537, let's focus on the `cache.annotations` service, which is the one that needs special care because it can be required while the container is being built. See e.g.:-#20234-http://stackoverflow.com/questions/39625863/vichuploadbundle-inb-symfony-3-cant-load-cache-annotations-service/40626277-schmittjoh/JMSDiExtraBundle#262When the service is required at build time, we can't provide it a logger, because no logger service is ready at that time. Still, that doesn't prevent the service from working. The late `CachePoolClearerPass` wires the logger for later instantiations so that `cache.annotations` has a properly configured logger *for the next requests*.Commits-------f62b820 [FrameworkBundle] Dont rely on any parent definition for "cache.annotations"
xabbuh commentedNov 27, 2016
@nicolas-grekas So does this mean that whether or not the |
nicolas-grekas commentedNov 27, 2016
cache.annotations will always have a logger, but only after the container is loaded from its dumped version. Which means always if you do offline cache warmup, and not for the very first request otherwise. |
…fore removing passes (nicolas-grekas)This PR was merged into the 3.2 branch.Discussion----------[FrameworkBundle] Dont wire "annotations.cached_reader" before removing passes| Q | A| ------------- | ---| Branch? | 3.2| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#21339| License | MIT| Doc PR | -This PR basically reverts#20601 and wires "annotations.cached_reader" later, so that any compiler passes needing "annotation_reader" at compile time don't get any cache - anyway, it's useless at compile time.Commits-------e59f0e0 [FrameworkBundle] Dont wire "annotations.cached_reader" before removing passes
Uh oh!
There was an error while loading.Please reload this page.
Instead of a generic approach that failed in#20537, let's focus on the
cache.annotationsservice, which is the one that needs special care because it can be required while the container is being built. See e.g.:When the service is required at build time, we can't provide it a logger, because no logger service is ready at that time. Still, that doesn't prevent the service from working. The late
CachePoolClearerPasswires the logger for later instantiations so thatcache.annotationshas a properly configured loggerfor the next requests.