-
Notifications
You must be signed in to change notification settings - Fork 11.3k
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
Schedule gets registered with every artisan call #51354
Comments
`ApplicationBuilder::withScheduling()` fixes #51354 Signed-off-by: Mior Muhammad Zaki <[email protected]>
Thanks @crynobone for taking your time to look at this. Unfortunately your attempt unveiled another issue. I tried investigating this myself and here are my findings:
Currently, Artisan::starting(fn () => $callback($this->app->make(Schedule::class))); Meaning that the code inside Artisan::starting(function () use ($callback) {
$this->app->afterResolving(Schedule::class, fn ($schedule) => $callback($schedule));
}); Which unfortunately doesn't work if At this point, for this to work we would need to either detect scheduled commands inside Of course, another option is to treat this "bug" as "working as expected" and perhaps document it. After all, all code inside |
Why you did not run 'schedule' inside a Artisan::command() function? like following:
So when run |
Well... Unless I'm missing something? |
How about having a dedicated |
@flexchar You are able to do that using |
I would urge to consider @crynobone PR #51364 as a valid solution for this issue. It might be a breaking change for people using scheduled commands in both places, though I'm not sure how common that is. If you are using |
`ApplicationBuilder::withScheduling()` fixes #51354 Signed-off-by: Mior Muhammad Zaki <[email protected]>
Looks like we won't be changing anything here anytime soon, sorry. |
@driesvints PR #51364 is still open so maybe we can keep the issue open until then? There were 2 PRs for this so that's why you see a closed one here. |
Ah sorry, missed that one. Thanks |
Taylor closed the PR:
For now I suggest keeping your scheduler as light as possible. If anyone is experiencing issues, please do try out the changes in #51364 and leave your feedback. |
Laravel Version
11.7.0
PHP Version
8.3.6
Database Driver & Version
No response
Description
When running any
artisan
command (justphp artisan
does the same), the code insideconsole.php
route file and/or->withSchedule()
bootstrap method gets executed. Usually this wouldn't be a big deal, because the code should only register cron events. However, sometimes dynamic scheduling might involve database queries which can inflict unwanted performance penalty or in worst case scenario fail in CI setup.We noticed this in our CI while upgrading from Laravel 10 to Laravel 11 (with slimmed down app structure). Our scheduler needs to be dynamic and it makes a DB query for that reason. When CI is setting up the application it runs
composer install
which then runsphp artisan package:discover --ansi
as a composerpost-autoload-dump
script. This is standard in every Laravel project. Sincepackage:discover
is an artisan command it boots up the scheduler as well. At this stage in CI database is not yet available thus we get aQueryException: SQLSTATE[HY000] [2002] Connection refused
.I have checked and this wasn't the case with Laravel 10. I'm not sure if this is now intended or not, but I would expect scheduler code to only execute when calling
php artisan schedule:run
andphp artisan schedule:work
.Steps To Reproduce
dd();
insideconsole.php
route file or inside->withSchedule()
app bootstrap method.php artisan
and observe the command to fail.The text was updated successfully, but these errors were encountered: