-
Notifications
You must be signed in to change notification settings - Fork 62
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
Revisit the message body of health #125
Comments
Not sure I agree that Also, not sure how Service Mesh plays into this, can you elaborate? |
I think its desirable to users to have a consistent payload across all of their microservices - regardless of the framework (or language) that each is built in. There isn't a significant level of consistency in the way of convention today, but certainly the most common approach is to use |
@cescoffier further to our discussion at EclipseCon, it is a cool thing that we can sync up in different tools and languages. Wait to see your PR and great to see you are back to Health spec again! |
If there's a consensus to rename |
Maybe renaming |
+1 to have I would like to discuss the backward compatibility, as it's a breaking change. Application analysing the content of the response would need to update to the new format. We can mark in the spec the |
Another point, should we add a |
If we add more fields, then it could also be good to optionally allow hints for operations folks, like a short description and a url to more information. |
@pilhuhn for each procedure? or in the global report? |
I would do it for each procedure and the global report. In many cases we will only see the global report, but if there are many procedures then it is expected that those parts can fail independently and thus the hints would be needed for each of the individual parts that can fail. |
Rename check state to status This is related to eclipse#125
I've opened a first PR doing the renaming: #126. |
@pilhuhn the PR mentioned in my previous comment does not include extra-metadata. I would prefer having a second PR extending the model with them. Would you be ok with this? |
On 21 Jun 2018, at 14:55, Clement Escoffier wrote:
extending the model with them. Would you be ok with this?
Yes of course
|
This is related to eclipse#125 Signed-off-by: Clement Escoffier <[email protected]>
Updated PR: #127 |
@cescoffier pleas also update the readme doc accordingly. |
Closed by #127 |
@Emily-Jiang can you close this issue? (it seems I can't) |
@Emily-Jiang I've updated the README, did I miss something? |
Thank you @cescoffier ! Looks good. |
Can we rename this issue to "Rename outcome/state to status/status" or similar, so that it is obvious what this issue deals with? Can we also revert this breaking change and continue this discussion until Health 2.0 is being considered? I think service meshes should be app framework aware and should talk to each app in it's own way, whether Spring, MicroProfile, Dropwizard, Play, or home-brew stackoverflow copy/paste job. Service meshes that aren't ping/health service style aware aren't good reasons to impose changes that break service meshes in Health 1.1! Thanks for reconsidering this change! |
If MP Health 2.0 is being developed in the We're obviously not done revisiting the message body of MP Health. I'd like to get #110 added in for 2.0. I'd also like to recommend working with inadarei/rfc-healthcheck#24. |
In the current health spec, we have
In the message it says 'outcome'. I think it should be changed to 'status', which is better understood and also match with what Spring says. In this way, in service-mesh platform, it is much easier to be communicated. Thoughts?
The text was updated successfully, but these errors were encountered: