Skip to content
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

[Cache] Add new cache.adapter.doctrine_dbal #16502

Open
wants to merge 1 commit into
base: 5.4
Choose a base branch
from

Conversation

Jibbarth
Copy link
Contributor

Hi 👋

Mention the new cache.adapter.doctrine_dbal in the list as a preconfigured adapter.

@@ -138,6 +143,8 @@ will create pools with service IDs that follow the pattern ``cache.[type]``.
default_memcached_provider: 'memcached://localhost'
# service: cache.pdo
default_pdo_provider: 'doctrine.dbal.default_connection'
# service: cache.doctrine_dbal
default_doctrine_dbal_provider: 'doctrine.dbal.default_connection'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this here too, as the service cache.default_doctrine_dbal_provider is registered as an alias of service doctrine.dbal.default_connection.

However, i'm not sure to understand properly this part of the doc. As far as i understand, a new pool should be created, named cache.doctrine_dbal, and I should be able to use it, but I don't see such a pool in my list of service.
Did I miss something ?

Copy link

Choose a reason for hiding this comment

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

https://github.com/symfony/framework-bundle/blob/5.4/DependencyInjection/Configuration.php#L1081

Suggested change
default_doctrine_dbal_provider: 'doctrine.dbal.default_connection'
default_doctrine_dbal_provider: 'database_connection'

@@ -138,6 +143,8 @@ will create pools with service IDs that follow the pattern ``cache.[type]``.
default_memcached_provider: 'memcached://localhost'
# service: cache.pdo
default_pdo_provider: 'doctrine.dbal.default_connection'
Copy link
Member

Choose a reason for hiding this comment

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

The PDO adapter no longer accept DBAL Connection. This line needs an update too.

Copy link

Choose a reason for hiding this comment

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

Depends on the availability of "doctrine/dbal": https://github.com/symfony/framework-bundle/blob/5.4/DependencyInjection/Configuration.php#L1082
If available: database_connection

Copy link
Member

Choose a reason for hiding this comment

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

Isn't the default config missing an update? DBAL doesn't extend PDO anymore, isn't it? So we can't set a default value here (unless we can access the PDO object when we have a DBAL connection?)
Would anyone like to dig this question?

@stchr
Copy link

stchr commented Sep 15, 2022

Any updates on this?
I guess this PR should be merged even if there are some unanswered questions as improves docs either way. Was searching way to long for the doctrine_dbal configuration...

@javiereguiluz
Copy link
Member

I'd like to merge this, but there are still some unanswered questions that I can't answer. Maybe @nicolas-grekas can help us here because he knows everything about the cache. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants