Add services tagged with 'doctrine.dbal.logger' to the connection's chain logger#692
Add services tagged with 'doctrine.dbal.logger' to the connection's chain logger#692renan wants to merge 6 commits intodoctrine:masterfrom
Conversation
mikeSimonson
left a comment
There was a problem hiding this comment.
I think that this is good to go minus the wording of the error message.
I just want @stof to look a it to confirm that there would be no performance penality.
Thanks for the PR
…g is enabled" This reverts commit 753957b.
|
The feedback has been applied. I undid all changes to If the configuration is using a chain logger it will simply append the new loggers into it. Otherwise it will wrap the existing logger plus the new loggers into a new chain. |
| } | ||
| } | ||
|
|
||
| private function ensureIsValidServiceDefinition(ContainerBuilder $container, string $serviceId): void |
There was a problem hiding this comment.
Is this needed?
I have never noticed any other doing this much for detecting miss configurations of services and symfony it self is happy to fail when you try to retrieve the service from the container.
There was a problem hiding this comment.
Anyone have a comment on this?
We don't have this anywhere else neither have i seen others do it.
|
@renan Is it possible for you to have a go at resolving the conflict? |
| $chainId = self::BASE_CHAIN_NAME . '.' . $connectionName; | ||
|
|
||
| $chainReference = new Reference($chainId); | ||
| $chainDefinition = new Definition(self::BASE_CHAIN_NAME); |
There was a problem hiding this comment.
Shouldn't there be LoggerChain::class?
I am getting error Symfony\Component\DependencyInjection\Exception\InvalidArgumentException: "'doctrine.dbal.logger.chain'" is not a valid class name for the "doctrine.dbal.logger.chain.default" service. with self::BASE_CHAIN_NAME
|
Hey! Thank you for your contribution. I've reviewed this PR in detail as part of #EUFOSSA and I'm afraid this is not mergeable in current state because of following reasons:
|
I've run into a use case where I need more than one
SQLLoggeron my application. I could've simply redefined the whole service that is provided by Doctrine Bundle but thought supporting this use case in the Bundle itself would be more beneficial as it opens other bundles to also hook into Doctrine SQL Logging.Services that are tagged with
doctrine.dbal.loggerare added to all connections.