-
-
Notifications
You must be signed in to change notification settings - Fork 229
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
Add priority field to processor tags #455
base: master
Are you sure you want to change the base?
Conversation
So that the order in which the processors are used by monolog can be determined.
foreach ($container->findTaggedServiceIds('monolog.processor') as $id => $tags) { | ||
foreach ($tags as $tag) { | ||
if (!empty($tag['channel']) && !empty($tag['handler'])) { | ||
throw new \InvalidArgumentException(sprintf('you cannot specify both the "handler" and "channel" attributes for the "monolog.processor" tag on service "%s"', $id)); | ||
if (!isset($tag['priority'])) { |
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.
Should the priority
option also be added to #[AsMonologProcessor]
?
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.
I should be able to add it without a problem, thanks for the suggestion!
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.
Update: 1797
usort( | ||
$processors, | ||
function (array $left, array $right) { | ||
return $left['tag']['priority'] <=> $right['tag']['priority']; | ||
} |
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.
Can the PriorityTaggedServiceTrait
be used here?
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.
Thanks for the suggestion, however the PriorityTaggedServiceTrait can´t be used since it will require some changes because the Trait sorts it ascending (as expected). But monologs pushProcessor actually array_shifts instead of adding it on top of the stack, so I need a descending sort. Which is clarified in the comment.
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.
We could still use the trait to get the sorted services and reverse the list before adding method calls.
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.
\Symfony\Component\DependencyInjection\Compiler\PriorityTaggedServiceTrait::findAndSortTaggedServices
returns an array of references, but we need tag attributes like handler
and channel
.
Could we resolve this comment and merge this PR?
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.
Please, I'm sorry for my lack of updates, feel free to take over.
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.
Related PR has been merged, can you finish the review @HypeMC? Thanks! |
Hey, what's the status here? I'm also interested in the feature. Thanks :) |
Following comment is still open: https://github.com/symfony/monolog-bundle/pull/455/files#r1369858964 |
usort( | ||
$processors, | ||
function (array $left, array $right) { | ||
return $left['tag']['priority'] <=> $right['tag']['priority']; | ||
} |
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.
\Symfony\Component\DependencyInjection\Compiler\PriorityTaggedServiceTrait::findAndSortTaggedServices
returns an array of references, but we need tag attributes like handler
and channel
.
Could we resolve this comment and merge this PR?
@Seldaek could you merge this? |
@nicolas-grekas could you help to merge this? |
So that the order in which the processors are used by monolog can be determined.
Example config:
(PR in favor of old PR)