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

Fix error for logging middleware when logger is missing #1488

Conversation

l-vo
Copy link
Contributor

@l-vo l-vo commented Mar 30, 2022

Fix #1487

@l-vo l-vo force-pushed the remove_logging_middleware_when_logger_not_available branch from 9a0ff93 to cc418eb Compare March 30, 2022 20:59
@l-vo l-vo changed the title Fix error for logginf middleware when logger is missing Fix error for logging middleware when logger is missing Mar 30, 2022
@l-vo l-vo force-pushed the remove_logging_middleware_when_logger_not_available branch 2 times, most recently from 2f28f6a to cb0ad28 Compare March 30, 2022 21:10
Copy link
Contributor

@dmaicher dmaicher left a comment

Choose a reason for hiding this comment

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

thanks for looking into this 👍

@dmaicher dmaicher added this to the 2.6.1 milestone Mar 31, 2022
@dmaicher dmaicher added the Bug label Mar 31, 2022
@l-vo l-vo force-pushed the remove_logging_middleware_when_logger_not_available branch from cb0ad28 to f2aeead Compare March 31, 2022 09:00
@l-vo l-vo force-pushed the remove_logging_middleware_when_logger_not_available branch from f2aeead to cbd8a94 Compare March 31, 2022 09:02
@l-vo
Copy link
Contributor Author

l-vo commented Mar 31, 2022

@dmaicher changes applied

@dmaicher
Copy link
Contributor

dmaicher commented Mar 31, 2022

@loic425 could you confirm that this fixes #1487 for you? tested it myself.

@dmaicher dmaicher requested a review from ostrolucky March 31, 2022 09:06
@dmaicher
Copy link
Contributor

dmaicher commented Apr 1, 2022

Tested it myself: Works fine if no logger service is defined and fixes #1487

@ostrolucky do you want to have a look? Or shall I go ahead and merge it?

@ostrolucky
Copy link
Member

I probably won't review this today. You can go ahead if you feel this is urgent. I don't personally see the urgency: who the heck doesn't have logger in prod?

@ostrolucky ostrolucky merged commit b700275 into doctrine:2.6.x Apr 2, 2022
@l-vo l-vo deleted the remove_logging_middleware_when_logger_not_available branch April 2, 2022 17:18
@loic425
Copy link

loic425 commented Apr 2, 2022

Thank you! I'll try on monday. Have a good weekend

@yura3d
Copy link

yura3d commented Mar 13, 2023

@l-vo Is it really need to always remove doctrine.dbal.logging_middleware definition in case no service named logger presents in container, even if the definition itself contains argument with its own logger?

final class RemoveLoggingMiddlewarePass implements CompilerPassInterface
{
public function process(ContainerBuilder $container)
{
if ($container->has('logger')) {
return;
}
$container->removeDefinition('doctrine.dbal.logging_middleware');
}
}

I'm trying to set my custom SQLLogger service only for DBAL logs (if I name it logger, it starts to receive symfony-specific logs, that I don't need):

App\Doctrine\SQLLogger:
    arguments:
        $logFilename: '%kernel.logs_dir%/sql.log'
doctrine.dbal.logging_middleware:
    class: 'Doctrine\DBAL\Logging\Middleware'
    arguments:
        - '@App\Doctrine\SQLLogger'

And this config doesn't work until I do something like this:

logger:
    class: 'Psr\Log\NullLogger'

@l-vo
Copy link
Contributor Author

l-vo commented Mar 22, 2023

@yura3d What about using your own service id instead of reusing doctrine.dbal.logging_middleware ? You just need to add the doctrine.middleware tag or enable autoconfiguration for the service.

@yura3d
Copy link

yura3d commented Mar 23, 2023

@l-vo Sorry, seems I didn't read the docs carefully. Thanks!

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

Successfully merging this pull request may close these issues.

Error on middleware on 2.6 with logger
5 participants