[Scheduler] Add MessageHandler result to the PostRunEvent
| Q | A |
|---|---|
| Branch? | 7.2 |
| Bug fix? | no |
| New feature? | yes |
| Deprecations? | no |
| Issues | |
| License | MIT |
This PR will add the result of a MessageHandler to the PostRunEvent of the Scheduler Component. This can be used for example for retrieving the output of a RunCommandMessage scheduled by the scheduler component.
<?php
namespace App\Service;
use Symfony\Component\Console\Messenger\RunCommandMessage;
use Symfony\Component\Scheduler\Attribute\AsSchedule;
use Symfony\Component\Scheduler\RecurringMessage;
use Symfony\Component\Scheduler\Schedule;
use Symfony\Component\Scheduler\ScheduleProviderInterface;
#[AsSchedule]
class ScheduleProvider implements ScheduleProviderInterface
{
public function getSchedule(): Schedule
{
$schedule = new Schedule();
$schedule->add(RecurringMessage::cron('* * * * *', new RunCommandMessage('about')));
return $schedule;
}
}
<?php
namespace App\EventListener;
use Symfony\Component\Console\Messenger\RunCommandContext;
use Symfony\Component\EventDispatcher\Attribute\AsEventListener;
use Symfony\Component\Scheduler\Event\PostRunEvent;
#[AsEventListener(event: PostRunEvent::class, method: 'onMessage')]
class SchedulerListener
{
public function onMessage(PostRunEvent $event): void
{
$result = $event->getResult();
if ($result instanceof RunCommandContext) {
echo $result->output; //Log or store output somewhere
}
}
}
How does it work if the handler result type is "never"?
I think It's better to use a message to transfer result. Possible to use a setter to add a result to the message inside a handler.
How does it work if the handler result type is "never"?
I think It's better to use a message to transfer result. Possible to use a setter to add a result to the message inside a handler.
If the MessageHandler does never return the $result value will be null. If the messagehandler is not in your own control(for instance RunCommandMessageHandler) it is not possible to use a setter to set the result to the message from the handler. The HandledStamp already collects the result of the message handler. This change will only allow you to use this result in the scheduler component as well.
@kbond Could you take a look at this maybe?
Thanks for the reply. I have also changed the ServiceCallMessageHandler to return it's result. This way the PostRunEvent result would also be available for AsPeriodicTask and AsCronTask. Also when using RedispatchMessage with a sync:// transport configured while developing I noticed the result didn't come through so I also returned the result in the RedispatchMessageHandler. Let me know what you think of these latest changes.
@alli83 or @kbond could you review this maybe?
Retrieving the result could indeed be interesting e.g depending on the outcome, we might decide whether to continue or stop. However, regarding the logging example, imho, I wonder if it’s really necessary to handle it in the listener rather than directly within the handler.
@kbond I have rebased the branch on 7.3
@kbond Sure, I have updated the test to use the event object instead of the booleans. It can now assert on properties that are in the event, is this what you meant?
Thanks for the review @kbond! I think it's ready on my side, is another review required or is it ready for merging by you/somebody else?
Just need to wait for another core team member approval.
@fabpot Could you maybe take a look at this?
@bartholdbos, since this has been approved, it'll likely get approved by another core member and merged closer to the 7.3 feature freeze.
Thank you @bartholdbos.