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

Improve docs for AdminServlet et al #1905

Merged
merged 7 commits into from
May 4, 2021

Conversation

mschechter-bellese
Copy link
Contributor

Added documentation for the AdminServlet and the other metrics-provided servlets, including the new configuration added as part of #1890.

Resolves #1896

@mschechter-bellese mschechter-bellese requested review from a team as code owners May 3, 2021 13:44
Copy link
Member

@joschi joschi left a comment

Choose a reason for hiding this comment

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

@mschechter-bellese Thanks for the awesome contribution! ❤️

@@ -14,12 +14,52 @@ HealthCheckServlet
``HealthCheckServlet`` responds to ``GET`` requests by running all the [health checks](#health-checks)
and returning ``501 Not Implemented`` if no health checks are registered, ``200 OK`` if all pass, or
``500 Internal Service Error`` if one or more fail. The results are returned as a human-readable
``text/plain`` entity.
``application/json`` entity.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we could just write "as a human-readable JSON entity"?

Suggested change
``application/json`` entity.
JSON entity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated - thanks!

Comment on lines 58 to 60
* ``httpStatusIndicator`` (`Boolean``): Determines whether the HTTP status code is used to
determine whether the application is healthy; if not provided, it defaults to the value from the
initialization parameter
Copy link
Member

Choose a reason for hiding this comment

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

Could we describe the possible HTTP response codes (200, 500, 501)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They're in the lead paragraph; I split them out for clarity

@joschi joschi added this to the 4.1.22 milestone May 3, 2021
Copy link
Member

@joschi joschi left a comment

Choose a reason for hiding this comment

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

@mschechter-bellese Thanks a lot! ❤️

@joschi joschi changed the title Added missing documentation Improve docs for AdminServlet et al May 4, 2021
@joschi joschi enabled auto-merge (squash) May 4, 2021 06:24
@joschi joschi merged commit 4516bae into dropwizard:release/4.1.x May 4, 2021
@mschechter-bellese mschechter-bellese deleted the servlet-docs branch May 4, 2021 12:13
@mschechter-bellese
Copy link
Contributor Author

@mschechter-bellese Thanks a lot! ❤️

You're welcome - glad to help out! I probably should have done this on my previous issue :)

Next up is getting these changes integrated into Dropwizard via AbstractServerFactory...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create documentation for #1890
2 participants