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.

<?phpnamespace 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;    }}

<?phpnamespace 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        }    }}

bartholdbos requested a review fromkbond as acode owner

October 11, 2024 18:31

carsonbot added Status: Needs Review Feature Scheduler labels

Oct 11, 2024

carsonbot added this to the7.2 milestone

Oct 11, 2024

Copy link

slutsky commentedOct 11, 2024•
edited
Loading

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.

Copy link

ContributorAuthor

bartholdbos commentedOct 18, 2024

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.

bartholdbos force-pushed thescheduler-post-run-event-result branch from1705445 to4e56ad4Compare

October 18, 2024 09:46

Copy link

ContributorAuthor

bartholdbos commentedOct 25, 2024

@kbond Could you take a look at this maybe?

kbond reviewed

Oct 25, 2024

View reviewed changes

Copy link

Member

kbond left a comment

Choose a reason for hiding this comment

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

I think this makes sense.@alli83, have any thoughts?

bartholdbos force-pushed thescheduler-post-run-event-result branch from03dceb0 to468c02dCompare

October 25, 2024 13:34

Copy link

ContributorAuthor

bartholdbos commentedOct 25, 2024•
edited
Loading

Thanks for the reply. I have also changed theServiceCallMessageHandler to return it's result. This way thePostRunEvent result would also be available forAsPeriodicTask andAsCronTask. Also when usingRedispatchMessage with async:// transport configured while developing I noticed the result didn't come through so I also returned the result in theRedispatchMessageHandler. Let me know what you think of these latest changes.

bartholdbos force-pushed thescheduler-post-run-event-result branch from468c02d to959b270Compare

October 26, 2024 10:04

Copy link

ContributorAuthor

bartholdbos commentedOct 29, 2024

@alli83 or@kbond could you review this maybe?

Copy link

Contributor

alli83 commentedOct 29, 2024•
edited
Loading

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 requested changes

Oct 31, 2024

View reviewed changes

Copy link

Member

kbond left a comment

Choose a reason for hiding this comment

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

I'm good with this feature. Can you add an entry to the changelog?

src/Symfony/Component/Messenger/Handler/RedispatchMessageHandler.phpShow resolvedHide resolved

carsonbot added Status: Needs Work and removed Status: Needs Review labels

Oct 31, 2024

kbond approved these changes

Oct 31, 2024

View reviewed changes

Copy link

Member

kbond left a comment

Choose a reason for hiding this comment

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

7.2 is in feature freeze so this will likely not make it until 7.3.

carsonbot added Status: Reviewed and removed Status: Needs Work labels

Oct 31, 2024

fabpot modified the milestones:7.2,7.3

Nov 20, 2024

bartholdbos force-pushed thescheduler-post-run-event-result branch from268e589 to05fe4a3Compare

December 13, 2024 10:02

Copy link

ContributorAuthor

bartholdbos commentedDec 13, 2024

@kbond I have rebased the branch on 7.3

kbond approved these changes

Dec 13, 2024

View reviewed changes

Copy link

Member

kbond left a comment

Choose a reason for hiding this comment

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

Looks good!

@bartholdbos, would you like to try updating the test? I was thinkinghere, instead of setting the props to true, set them to the event object and then assert on them in the test. If not, no worries, I can add after merge.

Copy link

ContributorAuthor

bartholdbos commentedDec 20, 2024

@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?