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

Add endpoints for health checks #622

Merged
merged 11 commits into from
Apr 11, 2023
Merged

Conversation

janjagusch
Copy link
Collaborator

Closes #620.

quetz/main.py Outdated Show resolved Hide resolved
@janjagusch janjagusch added the enhancement New feature or request label Apr 10, 2023
Copy link

@borchero borchero left a comment

Choose a reason for hiding this comment

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

Thanks for implementing @janjagusch! I'd rename the endpoints to be consistent with the Kubernetes API health endpoints, i.e.

  • /health/ready -> /readyz
  • /health/live -> /livez

quetz/tests/test_health.py Show resolved Hide resolved
@janjagusch janjagusch marked this pull request as ready for review April 10, 2023 10:35
@janjagusch
Copy link
Collaborator Author

janjagusch commented Apr 11, 2023

Thanks for implementing @janjagusch! I'd rename the endpoints to be consistent with the Kubernetes API health endpoints, i.e.

  • /health/ready -> /readyz
  • /health/live -> /livez

@borchero, have you ever seen this convention anywhere outside the k8s API? Most articles about health check endpoints suggest a naming scheme similar to the one I implemented here (e.g. here). I must say I prefer the current naming scheme. :)

@codecov-commenter
Copy link

codecov-commenter commented Apr 11, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.86 ⚠️

Comparison is base (87ec595) 82.22% compared to head (11423dd) 81.36%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #622      +/-   ##
==========================================
- Coverage   82.22%   81.36%   -0.86%     
==========================================
  Files          78       78              
  Lines        6120     6124       +4     
==========================================
- Hits         5032     4983      -49     
- Misses       1088     1141      +53     
Impacted Files Coverage Δ
quetz/main.py 86.25% <100.00%> (+0.07%) ⬆️

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@borchero
Copy link

@janjagusch I've seen it a bunch of times (see https://stackoverflow.com/questions/43380939/where-does-the-convention-of-using-healthz-for-application-health-checks-come-f which would also imply it's used in quite a few places) but I don't have a strong opinion. I've also seen the /health/xxx naming 😄

@ivergara
Copy link
Collaborator

Both can be created and the discussion is over!

@janjagusch janjagusch merged commit bc0ac65 into mamba-org:main Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add health endpoint
4 participants