10000 [Messenger][Scheduler] Add AsCronTask & AsPeriodicTask attributes by valtzu · Pull Request #51525 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Messenger][Scheduler] Add AsCronTask & AsPeriodicTask attributes #51525

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.

Sign up for GitHub

By clicking “Sign up for GitHub&rd 8000 quo;, 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

Jump to bottom
Merged
merged 1 commit into from
Oct 1, 2023
Merged

[Messenger][Scheduler] Add AsCronTask & AsPeriodicTask attributes #51525

merged 1 commit into from
Oct 1, 2023

Conversation

valtzu
Copy link
Contributor
@valtzu valtzu commented Aug 31, 2023 •
edited
Loading
Q A
Branch? 6.4
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #51432
License MIT
Doc PR symfony/symfony-docs#... // todo

Simplify scheduler usage by allowing to declare an attribute AsCronTask / AsPeriodicTask on any registered & autoconfigured service.

Example usage:

#[AsPeriodicTask(frequency: 5, jitter: 1, arguments: ['Hello from periodic trigger'])]
class BusinessLogic
{
    public function __invoke(string $message): void
    {
        echo "$message\n";
    }
}
#[AsCronTask('* * * * *', arguments: 'hello -v')]
#[AsCommand('app:do-stuff')]
class DoStuffCommand extends Command
{
    // ...
}
services:
  some_other_service:
    class: # ...
    tags:
      - name: scheduler.task
        trigger: cron
        expression: '0 9-17 * * *'
        method: 'someMethod'
        transports: [async]

bin/console debug:schedule output:

Scheduler
=========

default
-------

 ------------------------------------------- ---------------------------------------- --------------------------------- 
  Message                                     Trigger                                  Next Run                         
 ------------------------------------------- ---------------------------------------- --------------------------------- 
  @App\BusinessLogic                          every 5 seconds with 0-1 second jitter   Sat, 02 Sep 2023 10:55:36 +0000  
  app:do-stuff hello -v                       * * * * *                                Sat, 02 Sep 2023 13:56:00 +0300  
  @some_other_service::someMethod via async   0 9-17 * * *                             Sat, 02 Sep 2023 14:00:00 +0300  
 ------------------------------------------- ---------------------------------------- ---------------------------------

And then run bin/console messenger:consume scheduler_default to run the scheduler, like the usual.

To-do (help needed):

  1. tests
  2. docs
  3. validate the approach (creating RecurringMessages using DI)

@valtzu valtzu requested a review from kbond as a code owner August 31, 2023 14:23
@carsonbot carsonbot added this to the 6.4 milestone Aug 31, 2023
@valtzu valtzu force-pushed the 51432-scheduler-improvements branch 3 times, most recently from fd2967e to d6a2819 Compare August 31, 2023 19:01
@valtzu valtzu changed the title [Messenger] Add AsCronTask & AsPeriodicTask attributes [Scheduler][Messenger] Add AsCronTask & AsPeriodicTask attributes Aug 31, 2023
@valtzu valtzu changed the title [Scheduler][Messenger] Add AsCronTask & AsPeriodicTask attributes [Scheduler] Add AsCronTask & AsPeriodicTask attributes Aug 31, 2023
@valtzu
Copy link
Contributor Author
valtzu commented Sep 1, 2023

What could also be a cool feature here is add transports property to both attributes/tags, then if it is set, wrap ServiceCallMessage in RedispatchMessage.

@kbond
Copy link
Member
kbond commented Sep 1, 2023

This looks nice! I'll give a proper review and help finish up for 6.4 when I have some time.

What are you thinking for commands? I was thinking:

#[AsCronTask(expression: '* * * * *', arguments: 'arg --option')]
class MyCommand extends Command
{
}

@valtzu
Copy link
Contributor Author
valtzu commented Sep 2, 2023 •
edited
Loading

What are you thinking for commands?

I was hoping we don't need such, as business logic usually is wrapped in a separate class and commands just act as an entry point, just like controllers. But I think this is only my reality.

About the attribute syntax for commands, ArrayInput syntax was my first thought, but I see RunCommandMessage uses string input so probably your suggestion makes the most sense if the approach would be to produce those instead of ServiceCallMessage. Also StringInput syntax is less verbose and since the whole point here is to make thing easier, that's the way to go then.

@kbond
Copy link
Member
kbond commented Sep 7, 2023

Looking good! Glad you got commands working - using RunCommandMessage is exactly what I was thinking.

@valtzu valtzu force-pushed the 51432-scheduler-improvements branch from 6341a0a to cd499ee Compare September 13, 2023 16:23
@valtzu
Copy link
Contributor Author
valtzu commented Sep 14, 2023 •
edited
Loading

I didn't realize it's possible to use RedispatchMessage without explicitly defining the transports (and rely on routing configuration), we need to add support for that also. Probably rename transports to redispatch and allow boolean.

Hmm, ok it does not seem possible after all, if transportNames are not defined, then the message is not dispatched to any transport – unlike the Messenger docs are stating.

@carsonbot carsonbot changed the title [Scheduler] Add AsCronTask & AsPeriodicTask attributes [Messenger][Scheduler] Add AsCronTask & AsPeriodicTask attributes Sep 26, 2023
@fabpot fabpot force-pushed the 51432-scheduler-improvements branch from cd499ee to ed27b20 Compare October 1, 2023 08:32
@fabpot
Copy link
Member
fabpot commented Oct 1, 2023

Thank you @valtzu.

@fabpot fabpot merged commit 513e13a into symfony:6.4 Oct 1, 2023
@fabpot fabpot mentioned this pull request Oct 1, 2023
Closed
fabpot
fabpot reviewed Oct 2, 2023
View reviewed changes
src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/Scheduler/config.yml
@@ -10,6 +10,9 @@ services:
Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\DummySchedule:
autoconfigure: true

Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\DummyTask:
autoconfigure: true
Copy link
Member

Choose a reason for hiding this comment

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

There are no tests associated with this, can you add some @valtzu?
I've moved these to a new schedule to avoid breaking the existing tests (#51802).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fabpot I'm not too familiar with the framework tests yet, and I'm also a bit busy now. I would need a week or two, if that's ok then I can try.

Copy link
Member

Choose a reason for hiding this comment

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

Sure, there is no rush. Ping me if you need help.

This was referenced Oct 21, 2023
Merged
Merged
@valtzu valtzu mentioned this pull request Oct 26, 2023
Merged
fabpot added a commit that referenced this pull request Nov 3, 2023
@fabpot
minor #52313 [FrameworkBundle][Scheduler] Add test for autoconfigured…
ed105f1 
… schedule (valtzu)

This PR was merged into the 6.4 branch.

Discussion
----------

[FrameworkBundle][Scheduler] Add test for autoconfigured schedule

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| License       | MIT

Add functional tests for autoconfigured schedules / tasks, as discussed in #51525 (comment).

Commits
-------

ea2a8cd Add test for autoconfigured schedule
symfony-splitter pushed a commit to symfony/framework-bundle that referenced this pull request Nov 3, 2023
@fabpot
minor #52313 [FrameworkBundle][Scheduler] Add test for autoconfigured…
f75ebd3 
… schedule (valtzu)

This PR was merged into the 6.4 branch.

Discussion
----------

[FrameworkBundle][Scheduler] Add test for autoconfigured schedule

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| License       | MIT

Add functional tests for autoconfigured schedules / tasks, as discussed in symfony/symfony#51525 (comment).

Commits
-------

ea2a8cd7255 Add test for autoconfigured schedule
@faizanakram99
Copy link
Contributor
faizanakram99 commented Jan 4, 2024 •
edited
Loading

@valtzu @kbond iiuc, this doesn't support stateful schedules (equivalent to $schedule->stateful($cache)), right?

nvm, it will only add tasks to given schedule (and schedule can be optionally be stateful)

@W0rma W0rma mentioned this pull request Oct 1, 2024
Merged
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull request Jan 22, 2025
@javiereguiluz
minor #20278 [Scheduler] Add example about how to pass arguments to a…
4a1d96d 
… Symfony command (W0rma)

This PR was merged into the 6.4 branch.

Discussion
----------

[Scheduler] Add example about how to pass arguments to a Symfony command

We recently wanted to pass arguments to a Symfony command which is configured for the scheduler component with the `#[AsPeriodicTask]` attribute.

The syntax mentioned in symfony/symfony#51525 (comment) worked.
This PR adds an example to the docs.

Commits
-------

fc90d83 [Scheduler] Add example about how to pass arguments to a Symfony command
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@kbond kbond Awaiting requested review from kbond kbond is a code owner

Assignees
No one assigned
Labels
Feature Messenger Scheduler Status: Reviewed
Projects
None yet
Milestone
6.4
Development

Successfully merging this pull request may close these issues.

[RFC][Scheduler] Simplify schedule usage
5 participants
@valtzu @kbond @fabpot @faizanakram99 @carsonbot
0