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

[11.x] Defer registering schedule registered using ApplicationBuilder::withScheduling() #51364

Closed
wants to merge 1 commit into from

Conversation

crynobone
Copy link
Member

fixes #51354

`ApplicationBuilder::withScheduling()`

fixes #51354

Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com>
@crynobone crynobone marked this pull request as ready for review May 10, 2024 01:29
@Plytas
Copy link
Contributor

Plytas commented May 10, 2024

Thanks for your input @crynobone, but this doesn't solve the full problem. The problem still exists if you use just console.php route file.

This change introduces a new issue where you cannot use both ->withSchedule() on app bootstrap AND console.php route file. You can observe this by having a task scheduled in both places and running php artisan schedule:list.

Note: inspire is scheduled inside console.php (comes by default in new install) and test is scheduled inside bootstrap.

Before this change:
image

After this change:
image

@crynobone
Copy link
Member Author

You should defer resolving code dedicated for Schedule if console.php. The file itself needs to be loaded for all artisam requests.

@Plytas
Copy link
Contributor

Plytas commented May 10, 2024

You are correct, I forgot the fact that console.php is used for more than just schedule.

So the only issue I see now is that both methods cannot be used simultaneously while it could have been before.

@crynobone
Copy link
Member Author

So the only issue I see now is that both methods cannot be used simultaneously while it could have been before.

As in Laravel 10? It should only be possible to register Schedule within App\Console\Kernel::schedule(). So this should make withScheduling() behave similar to the old method.

@Plytas
Copy link
Contributor

Plytas commented May 10, 2024

As in Laravel 11 before this change. I'll re-itterate my previous message.

This change introduces a new issue where you cannot schedule commands in both ->withSchedule() on app bootstrap AND console.php route file. You can observe this by having a task scheduled in both places and running php artisan schedule:list.

Note: inspire is scheduled inside console.php (comes by default in new install) and test is scheduled inside bootstrap.

Before this change:
image

After this change:
image

With this change ->withSchedule() is only resolved when console.php route file is empty, otherwise it only lists (and processes) schedule from console.php and ignores ->withSchedule().

@crynobone crynobone closed this May 10, 2024
@crynobone crynobone deleted the fixes-51354 branch May 13, 2024 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Schedule gets registered with every artisan call
2 participants