-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[FrameworkBundle][Serializer] Fix APC cache service name #18567
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FrameworkBundle][Serializer] Fix APC cache service name #18567
Conversation
@dunglas: do you know what happened after your comment: #16795 (comment) ? |
ping @wouterj |
The validator service was renamed because the Validator component switched from using their own The serializer service was added in Symfony 2.7 (refs #13107). As it directly used doctrine cache, I think it should have been named I think it makes sense to do this renaming now. Just like all other renamings, the old service should be deprecated in favor of the new service and it should be merged into master (probably 3.1). The old service should then be removed in Symfony 4. Meanwhile, a PR should be submitted to the 2.8 branch of the Standard Edition to revert symfony/symfony-standard@0080556 This was a mistake (and it shows the need for consistency btw). |
bd84187
to
7d5e658
Compare
I update this PR to have the behavior @wouterj recomanded. I'm going to create the 2.8 PR. |
3c6016d
to
6293226
Compare
I think this is ready, targeting 3.1. |
👍 for 3.1 |
Status: reviewed Looks good. Can you please also update the |
I'll do that tomorrow. Thanks for your review! |
6293226
to
d6b1f5d
Compare
I did the change. |
@@ -83,6 +83,11 @@ FrameworkBundle | |||
- `"form.type.submit"` | |||
- `"form.type.reset"` | |||
|
|||
* The service `serializer.mapping.cache.apc` has been renamed to |
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.
[...] has been deprecated in favor of [...]
d6b1f5d
to
5348344
Compare
5348344
to
88ef89c
Compare
I'm not sure what the PR in 2.8 should look like. Shouldn't we apply the same PR as this one on 2.8? The validator cache service was renamed in 2.8, we could rename the serializer in 2.8 too couldn't we? |
@tgalopin It's about updating the commented config in the Symfony Standard Edition where we use the wrong service id: https://github.com/symfony/symfony-standard/blob/2.8/app/config/config_prod.yml#L8 |
What do you think about deprecating the class itself and recommending the PSR-6 backend instead? |
@dunglas IMHO it's too early to deprecate for 3.1, but for sure we should consider deprecating once we're used to PSR-6 caching and we're sure this wouldn't prevent any valid use case (which needs some time to me to be confirmed). |
So 👍 for this change. |
The failure is unrelated. |
👍 |
Thank you @tgalopin. |
…me (tgalopin) This PR was merged into the 3.1-dev branch. Discussion ---------- [FrameworkBundle][Serializer] Fix APC cache service name | Q | A | ------------- | --- | Branch? | master or 3.0 (not sure) | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - In the commit symfony/symfony-standard@0080556, we introduced in the standard edition the usage of `serializer.mapping.cache.doctrine.apc` instead of `serializer.mapping.cache.apc` in `config_prod.yml` comments. Earlier, we introduced the validator equivalent modification (`validator.mapping.cache.doctrine.apc` instead of `validator.mapping.cache.apc`) but while we adapted the validator configuration in the FrameworkBundle in #16822, we did not adapt the FrameworkBundle configuration for the serializer. I tested the current master of symfony-standard and it's indeed failing: ``` [Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException] The service "serializer" has a dependency on a non-existent service "serializer.mapping.cache.doctrine.apc". ``` This PR renames the serializer APCu cache service name to fix this issue. However, I'm not sure when the validator cache service modification was merged and released so I'm not sure how this PR should handle this. Is this a bug? Or is this a new feature and we should trigger a depreciation but keep the service `serializer.mapping.cache.apc` usable? Commits ------- 88ef89c [FrameworkBundle][Serializer] Fix APC cache service name and deprecate old name
@tgalopin Just noticed that we already have a PR for the Standard Edition (see symfony/symfony-standard#949). So no need to do anything there. |
In the commit symfony/symfony-standard@0080556, we introduced in the standard edition the usage of
serializer.mapping.cache.doctrine.apc
instead ofserializer.mapping.cache.apc
inconfig_prod.yml
comments.Earlier, we introduced the validator equivalent modification (
validator.mapping.cache.doctrine.apc
instead ofvalidator.mapping.cache.apc
) but while we adapted the validator configuration in the FrameworkBundle in #16822, we did not adapt the FrameworkBundle configuration for the serializer.I tested the current master of symfony-standard and it's indeed failing:
This PR renames the serializer APCu cache service name to fix this issue. However, I'm not sure when the validator cache service modification was merged and released so I'm not sure how this PR should handle this. Is this a bug? Or is this a new feature and we should trigger a depreciation but keep the service
serializer.mapping.cache.apc
usable?