`php artisan schedule:list` does not support fluent-style scheduling
raveren opened this issue · 15 comments
Laravel Version
11.0.7
PHP Version
8.3.4
Database Driver & Version
No response
Description
As found out in #50120 - the php artisan schedule:list is broken for fluent-style scheduling entries.
The scheduling still works as expected, but the list is displayed incorrectly.
Using the example from the docs (https://laravel.com/docs/11.x/scheduling#between-time-constraints):
$schedule->command('emails:send')
->hourly()
->between('7:00', '22:00');php artisan schedule:list:
0 * * * * php artisan emails:send ......................... Next Due: in 8 Minutes
As you can see the configuration is displayed as a seemingly random value, in this case just hourly.
Related Laracasts topic:
https://laracasts.com/discuss/channels/laravel/schedule-between-not-working-as-expected
Steps To Reproduce
add
$schedule->command('emails:send')
->hourly()
->between('7:00', '22:00');to routes/console.php and launch
php artisan schedule:list
Are you saying it doesn't run between 7:00 - 22:00 hourly every day. Or does the CLI output does not match your expectations but the schedule still executed correctly?
I did not test if it runs or not according to the schedule, but people in the Laracasts forum claim that:
between() works fine, just the schedule:list command does not display it OK. the tasks are not translated to individual cron entries, so the cron representation is a bit misleading (and wrong in this case). the kernel is run every minute (via cron) and it checks the schedule internally and run respective tasks/commands. and since it is not translated to the cron syntax (edit: well, it might, but i am not going to dig into it:), i just think it would be pain to represent all of the contraints that laravel scheduler provides.
someone tried to improve it, but unfortunately... #47983
So this issue is about php artisan schedule:list producing wrong output.
I tested it and I confirm that the between() method work fine but the php artisan schedule:list show the wrong output so it's only a visual matter.
Thank you for reporting this issue!
As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.
If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.
Thank you!
Sorry I closed your previous issue @raveren. Would love to get some help with this one.
No problem, totally understandable, however I think @JonPurvis, the author of #47983 knows this stuff best, however his PR was denied by @taylorotwell for reasons I couldn't fully understand.
Anyway me personally, I am out of my depth here, I've never did anything in Laravel internals at this point I and am frankly out of motivation to take this one up, as I accidentally lost the huge real-life schedule file that took me all evening to convert from cron to fluent-calls. I couldn't verify I did no unintended changes and eventually lost the change :)
I actually forgot I'd worked on that until I got the GitHub notification for the mention on this issue 😆
#47983 should fix this issue though. I could try tagging Taylor for a re-review of it and see what he says?
@JonPurvis Ideally, we should try to fix this issue without touching on the generation of the cron expression.
Yep of course, would be awesome to get this sorted somehow!
@JonPurvis would you mind sending your PR to 11.x and we can take another look?
The between or unlessBetween method uses when or skip, which are filters applied after CRON expression. They can be very difficult to represent, because you can have complex expressions like this:
$schedule->command('...')->when(fn () => Birthday::existsOnDate(today()));Coming from #51107, can confirm that ->between() also doesn't work with ->isDue() and ->nextRunDate() methods on schedule events. Therefore, making it really difficult to do assertions and tests. Any news on the issue? Is someone working on it? If not, I may attempt to mitigate the issue by slightly altering ->isDue() and ->nextRunDate(), as well as php artisan schedule:list in order to make them take ->between() into account. I agree that changing ->between() to CRON based solution would be dangerous, therefore we can do a workaround.
@JonPurvis would you maybe be up for re-attempting your PR against master/12.x? Maybe we can make it happen on a next release.
Hey @driesvints
Certainly, I actually meant to do this when Taylor tagged me but it slipped my mind. I'll re-create this PR targeted to Laravel 12 this week so I don't forget! I believe it's still worth looking into.
Going to close this one now as non-supported but happy to receive a new attempt at 12.x, thanks.