Cache Warmup Breaks Namespaced Kernel #9525
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
My kernel has been moved and namespaced to `Cfs\Bundle\MultiSiteBundle\Kernel\CfsKernel`. This worked fine until a change was made to how the kernel temp stuff is handled in the warmup phase. When the app generates its own cache (i.e you run cache without warmup and access the site) everything is generated ok and the .meta files generate the proper reference to the FQN of the Kernel. However if the warmup is used, it uses `Cfs\Bundle\MultiSiteBundle\Kernel\CfsKerne_` as the temporary Kernel, and when it does "fix references to the Kernel in .meta files" it generates 2 errors. 1. It does not use a string safe tempKernel name, so it never finds the reference to the kernel 2. If you fix that, then it replaces the FQN of the tempKernel with `CfsKernel`, the non-namespaced name of the proper Kernel (it also leaves the character count wrong in the serialization `C:43:<class>` where 43 is the char count for the FQN above) The two changes above fix this, by escaping the string and replacing it with a FQN Kernel Class name. What are your thoughts on this?
| @@ -120,10 +120,14 @@ protected function warmup($warmupDir, $realCacheDir, $enableOptionalWarmers = tr | |||
| $warmer->warmUp($warmupDir); | |||
|
|
|||
| // fix references to the Kernel in .meta files | |||
|
|
|||
| $safeTempKernel = str_replace('\\', '\\\\', get_class($tempKernel)); | |||
| $realKernelFQN = get_class($realKernel); | |||
There was a problem hiding this comment.
AFAIU, this is the same value as $realKernelClass created above and which was used before. I must be missed something here but I don't see the difference.
fabpot
added a commit
that referenced
this pull request
Nov 23, 2013
This PR was merged into the master branch. Discussion ---------- Cache Warmup Breaks Namespaced Kernel | Q | A | ------------- | --- | Bug fix? | [yes] | New feature? | [no] | BC breaks? | [no] | Deprecations? | [no] | Tests pass? | have not tried yet | License | MIT | Fixed tickets | further fixes #1431 My kernel has been moved and namespaced to `Cfs\Bundle\MultiSiteBundle\Kernel\CfsKernel`. This worked fine until a change was made to how the kernel temp stuff is handled in the warmup phase. When the app generates its own cache (i.e you run cache without warmup and access the site) everything is generated ok and the .meta files generate the proper reference to the FQN of the Kernel. However if the warmup is used, it uses `Cfs\Bundle\MultiSiteBundle\Kernel\CfsKerne_` as the temporary Kernel, and when it does "fix references to the Kernel in .meta files" it generates 2 errors. 1. It does not use a string safe tempKernel name, so it never finds the reference to the kernel 2. If you fix that, then it replaces the FQN of the tempKernel with `CfsKernel`, the non-namespaced name of the proper Kernel (it also leaves the character count wrong in the serialization `C:43:<class>` where 43 is the char count for the FQN above) The two changes above fix this, by escaping the string and replacing it with a FQN Kernel Class name. What are your thoughts on this? If this sounds reasonable i'll do further enhancements and check tests. Commits ------- 9e7788e Cache Warmup Breaks Namespaced Kernel
|
@fabpot, how do i proceed to get this into the 2.3 branch? just open a PR on that branch? |
rdohms
added a commit
to rdohms/symfony
that referenced
this pull request
Nov 25, 2013
Backported the patch in symfony#9525 to the 2.3 branch.
fabpot
added a commit
that referenced
this pull request
Nov 26, 2013
…dohms) This PR was merged into the 2.3 branch. Discussion ---------- Adjusting CacheClear Warmup method to namespaced kernels Backported the patch in #9525 to the 2.3 branch. | Q | A | ------------- | --- | Bug fix? | [yes] | New feature? | [no] | BC breaks? | [no] | Deprecations? | [no] | Tests pass? | have not tried yet | License | MIT | Fixed tickets | further fixes #1431 My kernel has been moved and namespaced to `Cfs\Bundle\MultiSiteBundle\Kernel\CfsKernel`. This worked fine until a change was made to how the kernel temp stuff is handled in the warmup phase. When the app generates its own cache (i.e you run cache without warmup and access the site) everything is generated ok and the .meta files generate the proper reference to the FQN of the Kernel. However if the warmup is used, it uses `Cfs\Bundle\MultiSiteBundle\Kernel\CfsKerne_` as the temporary Kernel, and when it does "fix references to the Kernel in .meta files" it generates 2 errors. 1. It does not use a string safe tempKernel name, so it never finds the reference to the kernel 2. If you fix that, then it replaces the FQN of the tempKernel with `CfsKernel`, the non-namespaced name of the proper Kernel (it also leaves the character count wrong in the serialization `C:43:<class>` where 43 is the char count for the FQN above) The two changes above fix this, by escaping the string and replacing it with a FQN Kernel Class name. What are your thoughts on this? If this sounds reasonable i'll do further enhancements and check tests. Commits ------- 00d79d5 Adjusting CacheClear Warmup method to namespaced kernels
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
My kernel has been moved and namespaced to
Cfs\Bundle\MultiSiteBundle\Kernel\CfsKernel. This worked fine until a change was made to how the kernel temp stuff is handled in the warmup phase.When the app generates its own cache (i.e you run cache without warmup and access the site) everything is generated ok and the .meta files generate the proper reference to the FQN of the Kernel.
However if the warmup is used, it uses
Cfs\Bundle\MultiSiteBundle\Kernel\CfsKerne_as the temporary Kernel, and when it does "fix references to the Kernel in .meta files" it generates 2 errors.CfsKernel, the non-namespaced name of the proper Kernel (it also leaves the character count wrong in the serializationC:43:<class>where 43 is the char count for the FQN above)The two changes above fix this, by escaping the string and replacing it with a FQN Kernel Class name.
What are your thoughts on this? If this sounds reasonable i'll do further enhancements and check tests.