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 health endpoint-related code into a separate file #344

Merged
merged 10 commits into from
Apr 16, 2024

Conversation

palant
Copy link
Contributor

@palant palant commented Apr 14, 2024

Description

This creates a new module health responsible for the health endpoint. So far it handles the server initialization in the init() method and request processing in the pre_process() method. Ideally, handling of the health setting in CLI and config file will also be added here in future (not trivial given how parsing is tied to data structures).

Related Issue

This is a tiny first step towards #342.

How Has This Been Tested?

Tests pass, and I’ve added a few new tests as well. I did not perform any functional testing at this point.

This is a first step towards #342. At this point HealthHandler merely encapsulates static methods. The idea is to have init() method also handle moving Settings::General::health to RequestHandlerOpts::health in future. Ideally, handling of the health setting CLI and config file will be added here as well (not trivial given how parsing is tied to data structures).
Copy link

semanticdiff-com bot commented Apr 14, 2024

Review changes with SemanticDiff.

Analyzed 4 of 4 files.

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

Filename Status
✔️ src/handler.rs 15.0% smaller
✔️ src/health.rs Analyzed
✔️ src/lib.rs Analyzed
✔️ src/server.rs 35.49% smaller

@palant palant marked this pull request as draft April 14, 2024 22:49
@palant palant mentioned this pull request Apr 14, 2024
13 tasks
@palant palant marked this pull request as ready for review April 15, 2024 09:22
@joseluisq joseluisq changed the title chore: Move code related to the health endpoint into a separate file Move code related to the health endpoint into a separate file Apr 16, 2024
@joseluisq joseluisq changed the title Move code related to the health endpoint into a separate file Move health endpoint code related into a separate file Apr 16, 2024
@joseluisq joseluisq changed the title Move health endpoint code related into a separate file Move health endpoint-related code into a separate file Apr 16, 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.

Just minor changes but looks fine to me overall.

src/lib.rs Outdated Show resolved Hide resolved
src/handler.rs Show resolved Hide resolved
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 joseluisq assigned joseluisq and unassigned joseluisq Apr 16, 2024
@joseluisq joseluisq added enhancement New feature or request v2 v2 release codebase Related to the project's source code labels Apr 16, 2024
@joseluisq joseluisq merged commit fe6a2a1 into static-web-server:master Apr 16, 2024
27 checks passed
@palant palant deleted the health-module branch April 16, 2024 20:29
joseluisq pushed a commit that referenced this pull request Apr 18, 2024
This is largely analogous to #344, another step towards fixing #342.

Loading logger module in lib.rs was moved up for sake of consistency: modules loaded after it will have macros like server_info imported implicitly, the ones loaded before won't. Further alphabetical rearranging of modules was performed by rustftm.
@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