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

Move most of maintenance mode logic into maintenance_mode module #350

Merged
merged 3 commits into from
Apr 20, 2024
Merged

Move most of maintenance mode logic into maintenance_mode module #350

merged 3 commits into from
Apr 20, 2024

Conversation

palant
Copy link
Contributor

@palant palant commented Apr 19, 2024

Description

This establishes the same internal API for maintenance mode as for health and metrics. While at it, I adjusted get_response() to make producing body_content more straightforward – no long a mutable variable that might or might not be overwritten but two straightforward branches. Side-effect: empty maintenance mode file will no longer result in default message. IMHO this makes sense because a) someone might actually show an empty message and b) the message about falling back to default message is only logged when the maintenance mode file doesn’t exist, not when it is empty.

I also changed DEFAULT_BODY_CONTENT because the old message is IMHO grammatically incorrect. It should be either “server is under maintenance” or “server is in maintenance mode” (I went with the latter).

Related Issue

This is another step towards fixing #342.

How Has This Been Tested?

I added some rudimentary unit tests. Functional testing on Linux was also successful.

Copy link

semanticdiff-com bot commented Apr 19, 2024

Review changes with SemanticDiff.

Analyzed 3 of 3 files.

Overall, the semantic diff is 9% smaller than the GitHub diff.

Filename Status
✔️ src/handler.rs 13.84% smaller
✔️ src/maintenance_mode.rs 5.75% smaller
✔️ src/server.rs 20.13% smaller

@palant palant mentioned this pull request Apr 19, 2024
13 tasks
@joseluisq joseluisq added enhancement New feature or request v2 v2 release codebase Related to the project's source code labels Apr 20, 2024
Copy link
Collaborator

@joseluisq joseluisq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@joseluisq
Copy link
Collaborator

Side-effect: empty maintenance mode file will no longer result in default message. IMHO this makes sense because a) someone might actually show an empty message and b) the message about falling back to default message is only logged when the maintenance mode file doesn’t exist, not when it is empty.

Fair point.

@joseluisq joseluisq merged commit 1246e37 into static-web-server:master Apr 20, 2024
27 checks passed
@palant palant deleted the separate-maintenance-mode branch April 20, 2024 13:59
@joseluisq joseluisq added this to the v2.30.0 milestone Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
codebase Related to the project's source code enhancement New feature or request v2 v2 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants