-
Notifications
You must be signed in to change notification settings - Fork 55
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
docs: add more api specs #543
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Approving so that you can merge later. I've added specific questions/suggestions (see the comments) and I have a couple more thoughts:
-
For the two unsupported endpoints, I think it's best to exclude them from the doc if possible. To make it easier to navigate around the other endpoints
-
I'm a bit confused about
GET /v1/health
- Does this return a 200 response forhealthy: true
andhealthy: false
? From reading the code, it seems like the response forhealthy: false
ishttp.StatusBadGateway
(code 502). Maybe I misunderstood, but don't we need two different responses for this endpoint?
Removed.
Yes, it's 200 when healthy is true and 502 when false. However, in a discussion with Ben earlier, we agreed to only show responses for 200 and not 4xx/5xx responses, which are shown at the beginning of the doc, so I have only one response/example here. |
Adding more endpoints to the OpenAPI spec.
Refactor
pebble help
output.Style updates
Newly added specs
/v1/system-info
. The code is here, and the possible boot-id value is here./v1/health
. The code is here./v1/changes
, code is here./v1/changes/{id}
, both GET and Post. Code: get, post./v1/services/{name}
, not implemented in the code/v1/checks
, code.Sorry for this big PR, but it's only logical::
/v1/system-info
is more of a miscellaneous endpoint that doesn't belong to any group/tag./v1/health
is another public API besides/v1/system-info
, I included it as well, only to find out later that/v1/changes
belongs to the same group so I included it in the same PR.