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

[Scheduler] Add new component documentation #18136

Closed
wants to merge 2 commits into from

Conversation

MrYamous
Copy link
Contributor

Fixes #18067

This is an early draft for the Scheduler component and his Messenger integration to gather early feedback

While starting the documentation, I asked myself a few questions :

  • Scheduler is a standalone component but shares some keys concepts with Messenger (message, handlers...), should we document them here too or only refers to related parts in Messenger doc ?
  • There are already several issues in docs for different parts of the component (initial integration, the debug command, new triggers...), should we have them all in the same pull request or keep split ?

@OskarStark OskarStark added this to the 6.3 milestone Apr 3, 2023
public function getSchedule(string $id): Schedule
{
return (new Schedule())
->add(new RecurringMessage::every('1 week', new EndofTrialMessage()))
Copy link
Contributor

Choose a reason for hiding this comment

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

The new keyword should not be there 😉

Suggested change
->add(new RecurringMessage::every('1 week', new EndofTrialMessage()))
->add(RecurringMessage::every('1 week', new EndofTrialMessage()))

Copy link
Member

@kbond kbond left a comment

Choose a reason for hiding this comment

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

Excellent start!

Register a Schedule provider
----------------------------

A Schedule provider is a class defining your schedule : which messages should
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
A Schedule provider is a class defining your schedule : which messages should
A Schedule provider is a class defining your schedule. Which messages should

----------------------------

A Schedule provider is a class defining your schedule : which messages should
be processed depending a trigger you choose::
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
be processed depending a trigger you choose::
be processed depends on the trigger you choose::

Copy link

Choose a reason for hiding this comment

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

Suggested change
be processed depending a trigger you choose::
be processed depending on the trigger you choose:

use Symfony\Component\Scheduler\Schedule;
use Symfony\Component\Scheduler\ScheduleProviderInterface;

#[AsSchedule('trial')]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#[AsSchedule('trial')]
#[AsSchedule('default')]

I'm thinking we should just use default here. Multiple schedules is an advanced use-case imo.

use Symfony\Component\Scheduler\ScheduleProviderInterface;

#[AsSchedule('trial')]
final class EndOfTrialScheduleProvider implements ScheduleProviderInterface
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
final class EndOfTrialScheduleProvider implements ScheduleProviderInterface
final class AppSchedule implements ScheduleProviderInterface


.. code-block:: terminal

$ php bin/console messenger:consume scheduler_trial
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
$ php bin/console messenger:consume scheduler_trial
$ php bin/console messenger:consume scheduler_default

`cron`. By extending the provided TriggerInterface you can also create
your own trigger for your app.

Every
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Every
PeriodicalTrigger


RecurringMessage::every('monday', $msg, until: '2023-06-12');

Cron
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Cron
CronExpressionTrigger


Cron
~~~~

Copy link
Member

Choose a reason for hiding this comment

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

We'll need a note that https://github.com/dragonmantank/cron-expression is required to use this trigger.

// trigger every monday at 12:00
RecurringMessage::cron('0 12 * * 1', $msg);

.. note::
Copy link
Member

Choose a reason for hiding this comment

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

We should document symfony/symfony#49792

.. note::

The minimal interval allowed with cron is 1 minute.

Copy link
Member

Choose a reason for hiding this comment

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

TODO:

  • JitterTrigger
  • CallbackTrigger
  • ExcludeTimeTrigger

@alexander-schranz
Copy link
Contributor

Is there something we can help here to move this forward? Think at current state would be better to have a incomplete documentation then no documentation of the scheduler component.

@MrYamous
Copy link
Contributor Author

Sorry I created this PR then I've been pretty absent.
AFAIK Scheduler is still undocumented, but it's on hold referring to #18290 (see comment)

@javiereguiluz Maybe we can help in some way ?

@MrYamous
Copy link
Contributor Author

Closing in favor of #19244

@MrYamous MrYamous closed this Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants