[9.x] Move maintenance mode logic #40070
Conversation
Moving the maintenance mode logic to its own class. This enables users to extend/overwrite the class with their own logic. Overwriting the logic may be needed if for example the application is running on multiple servers. Fixes #36474
Adding the isUp method to check if the application is up or is down for maintenance. Relates to #36474
Extend the documentation about what the code does. Also use FQN. Relates to #36474
Use the make method instead of the app function to retrieve an instance of the MaintenanceMode container. Relates to #36474
Update the docblock with the newly added method param. Relates to #36474
Update the docblock with the newly added method param. Relates to #36474
Updating documentation style to comply to the style used within the Laravel codebase.
Updating documentation style to comply to the style used within the Laravel codebase.
Move the class from its own namespace to the Foundation namespace. Relates to #36474
Updating documentation style to comply to the style used within the Laravel codebase.
Updating documentation style to comply to the style used within the Laravel codebase.
src/Illuminate/Foundation/Http/Middleware/PreventRequestsDuringMaintenance.php
Outdated
Show resolved
Hide resolved
src/Illuminate/Foundation/Http/Middleware/PreventRequestsDuringMaintenance.php
Outdated
Show resolved
Hide resolved
driesvints
changed the title
…gMaintenance.php Co-authored-by: Dries Vints <dries@vints.io>
…gMaintenance.php Co-authored-by: Dries Vints <dries@vints.io>
Create an application methode to retrieve the MaintenanceMode instance. This method reduces the need to fetch the MaintenanceMode with the usage of an additional argument in the middleware. Relates to #36474
|
Because of the changes made the PR no longer introduces new method arguments to existing code. Should this PR shill be pointed at master (9.x) or instead to the 8.x branch? |
Co-authored-by: Dries Vints <dries@vints.io>
One thing that can very much help this PR get merged is explain the use case a bit more thoroughly in your PR description. So state clearly why this is needed (multiple servers but also different stores). Thanks! |
Co-authored-by: Dries Vints <dries@vints.io>
|
This PR no longer introduces new method arguments to existing code but it adds a new method into |
Remove the comparison with the false value. The method always returns a bool which only allows a true or false return value. This make the comparison redundant. Relates to #36474
There was a problem hiding this comment.
I like the PR. I'd maybe even make some proxy methods on Application for isUp, isDown, up and down to make the code even cleaner but let's see what @taylorotwell says.
Remove the unused import. Because the FQN is used in the DocBlock the import has become redundant.
|
This looks like a really useful feature. Would it be worth adding a contract/interface for the |
Add an interface under the contracts to enable developers to easily extend the MaintenanceMode class.
Updating documentation style to comply to the style used within the Laravel codebase.
|
Agreed I think it would benefit from a contract. The contract would make it easier for developers to overwrite the original class with their own logic. |
Bind the default MaintenanceMode class to the contract reference in the service container. This enables users to set-up their own contract binding to overwrite the default Maintenance Mode behavior.
Updating documentation style to comply to the style used within the Laravel codebase.
src/Illuminate/Foundation/Providers/FoundationServiceProvider.php
Outdated
Show resolved
Hide resolved
Co-authored-by: Choraimy Kroonstuiver <3661474+axlon@users.noreply.github.com>
|
Can you allow me to push to this branch so I can update the PR? Allow edits from maintainers. |
|
@taylorotwell I've given you permission to the repository. I couldn't find the setting on this PR page, so I gave you the needed permissions on the forked repo instead. Is that workable? I am missing the option because the fork was created under an organisation. If needed I could move it to my own account and recreate the pull request so I can allow the option. |
taylorotwell
deleted the
#36474-enable_overwriting_maintenance_methods
branch
|
Thanks for contributing to Laravel! ❤️ |
|
Thanks for your PR @wimulkeman 👍 |
Moving the maintenance mode logic to its own class. This enables users to extend/overwrite the class with their own logic.
Overwriting the logic may be needed if for example the application is run on multiple servers. In this case the developer would want to extend the storage logic for the maintenance mode to implement a solution that can change the maintenance state for all the servers at once and reverse the maintenance mode with one command.
By separating the maintenance mode logic the framework opens the door for packages that could handle the maintenance mode through different mechanisms like using the cache (as proposed in a blog https://www.raypold.com/posts/laravel-distributed-maintenance-mode/).
The need for this was shown within the discussions laravel/ideas#1944 and #36474.
This PR is a follow-up to the PR #40059 with suggested changes and pointed to the master instead of the 8.x branch because it contains backward incompatible changes.