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

[serve] Add healthz endpoint for HttpProxy. #26347

Merged

Conversation

fishbone
Copy link
Contributor

@fishbone fishbone commented Jul 7, 2022

Why are these changes needed?

This PR add an endpoint for http proxy for health check. This feature is critical for serve operator because serve service needs to add the node to the service endpoint in a dynamic way and it'll use this endpoint for health check and add the health one.

Related issue number

Checks

  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@edoakes
Copy link
Collaborator

edoakes commented Jul 7, 2022

@iycheng I think by standard convention this should just be /healthz?

@fishbone
Copy link
Contributor Author

fishbone commented Jul 7, 2022

@iycheng I think by standard convention this should just be /healthz?

@edoakes I put it in -/ because it's kind of in a different namespace. For example, user can still add /healthz for their usecase.

But it's your call. I'm ok to move it to /healthz.

@simon-mo simon-mo merged commit f2f1086 into ray-project:master Jul 7, 2022
@simon-mo
Copy link
Contributor

simon-mo commented Jul 7, 2022

/-/healthz is perfect for now. Thanks @iycheng

Stefan-1313 pushed a commit to Stefan-1313/ray_mod that referenced this pull request Aug 18, 2022
Signed-off-by: Stefan van der Kleij <s.vanderkleij@viroteq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants