Skip to content

[FrameworkBundle] Add framework config for DBAL cache adapter #44065

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 17, 2021
Merged

[FrameworkBundle] Add framework config for DBAL cache adapter #44065

merged 1 commit into from
Nov 17, 2021

Conversation

GromNaN
Copy link
Member

@GromNaN GromNaN commented Nov 15, 2021 •
edited
Loading

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets doctrine/DoctrineBundle#1417
License MIT
Doc PR -

The framework configuration was missing from #43362.

Additionnaly, the depreciation message on PdoCacheAdapterDoctrineSchemaSubscriber must be removed. This class needs to be used in 5.4 whenever a PdoAdapter is used, because it could have a DBAL connection and we need to keep the deprecated behavior. A depreciation message is already triggered in the PdoAdapter itself when it gets a DBAL connection.

@carsonbot carsonbot added this to the 5.4 milestone Nov 15, 2021
@GromNaN GromNaN force-pushed the 5.4-dbal-cache-config branch from 4f10486 to d7ad8ab Compare November 15, 2021 22:52
@GromNaN GromNaN force-pushed the 5.4-dbal-cache-config branch from 55d257f to 1ad7865 Compare November 15, 2021 23:12
@GromNaN GromNaN mentioned this pull request Nov 16, 2021
Merged
nicolas-grekas
nicolas-grekas reviewed Nov 16, 2021
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please do two more things?

  • remove final on DoctrineDbalAdapter, it's not needed
  • remove the class_exists check in cache.php and move it to the extension

@GromNaN
Copy link
Member Author

GromNaN commented Nov 16, 2021

  • remove final on DoctrineDbalAdapter, it's not needed

👍🏻 to simplify. That would remove the interface that has been added for mocking.
Original discussion #43362 (comment).

  • remove the class_exists check in cache.php and move it to the extension

Do you mean the existing one?

symfony/src/Symfony/Bundle/FrameworkBundle/Resources/config/cache.php

Line 99 in 1ad7865

if (class_exists(DoctrineAdapter::class)) {

@nicolas-grekas
Copy link
Member

yes, please remove the interface
yes, this line :)

@nicolas-grekas
Copy link
Member

btw, the deprecation message should be updated also:
The "%service_id%" service inherits from "cache.adapter.doctrine" which is deprecated.

@GromNaN GromNaN force-pushed the 5.4-dbal-cache-config branch from 5e75678 to 108184b Compare November 16, 2021 10:12
@carsonbot carsonbot changed the title Add framework config for DBAL cache adapter [FrameworkBundle] Add framework config for DBAL cache adapter Nov 16, 2021
nicolas-grekas
nicolas-grekas approved these changes Nov 16, 2021
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget to update the LockRegistry to fix tests and GTM!

@GromNaN GromNaN force-pushed the 5.4-dbal-cache-config branch from 108184b to 16399c3 Compare November 16, 2021 10:17
@GromNaN
Copy link
Member Author

GromNaN commented Nov 16, 2021

Thanks for the review. Ready!

@GromNaN GromNaN force-pushed the 5.4-dbal-cache-config branch from 739ecbd to 672545d Compare November 16, 2021 20:12
@fabpot
Copy link
Member

fabpot commented Nov 17, 2021

Thank you @GromNaN.

@fabpot fabpot merged commit a8bee28 into symfony:5.4 Nov 17, 2021
@GromNaN GromNaN deleted the 5.4-dbal-cache-config branch November 17, 2021 08:58
This was referenced Nov 18, 2021
Merged
Merged
@Tobion
Copy link
Contributor

Tobion commented Jan 20, 2022

The new cache adapter seems to be missing in the documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derrabus derrabus derrabus left review comments

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@ogizanagi ogizanagi ogizanagi approved these changes

Assignees
No one assigned
Labels
Bug FrameworkBundle Status: Reviewed
Projects
None yet
Milestone
5.4
Development

Successfully merging this pull request may close these issues.
7 participants
@GromNaN @nicolas-grekas @fabpot @Tobion @derrabus @ogizanagi @carsonbot