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

php artisan schedule:list does not support fluent-style scheduling #50670

Open
raveren opened this issue Mar 20, 2024 · 12 comments
Open

php artisan schedule:list does not support fluent-style scheduling #50670

raveren opened this issue Mar 20, 2024 · 12 comments

Comments

@raveren
Copy link

raveren commented Mar 20, 2024

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
@crynobone
Copy link
Member

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?

@raveren
Copy link
Author

raveren commented Mar 20, 2024

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.

@wanted80
Copy link

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.

Copy link

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!

@nunomaduro nunomaduro self-assigned this Mar 20, 2024
@driesvints
Copy link
Member

Sorry I closed your previous issue @raveren. Would love to get some help with this one.

@driesvints driesvints added the bug label Mar 20, 2024
@raveren
Copy link
Author

raveren commented Mar 20, 2024

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 :)

@JonPurvis
Copy link

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?

@nunomaduro
Copy link
Member

@JonPurvis Ideally, we should try to fix this issue without touching on the generation of the cron expression.

@JonPurvis
Copy link

Yep of course, would be awesome to get this sorted somehow!

@taylorotwell
Copy link
Member

@JonPurvis would you mind sending your PR to 11.x and we can take another look?

@nunomaduro nunomaduro removed their assignment Mar 21, 2024
@ottoszika
Copy link
Contributor

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()));

@skywarth
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants