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

Minor feedback #22

Closed
taliaga opened this issue Oct 12, 2018 · 3 comments
Closed

Minor feedback #22

taliaga opened this issue Oct 12, 2018 · 3 comments

Comments

@taliaga
Copy link
Contributor

taliaga commented Oct 12, 2018

Hi, nice initiative!

Fwiw, here some minor observations from my experience trying your proposal:

  • There's a typo in section 4 (value eside).
  • When talking about status:
    • 'warn' is mentioned with a MUST and later with a SHOULD.
    • It may be time-saving for the reader to give some specific examples of which HTTP codes could be used in the returns. It could say 207 for 'warn' and 424 for 'fail' maybe? It's an example of course, but saves time having to go an read all http codes.
  • In details, take "cassandra:connections" for example. It may be an overkill to use arrays. I believe if this proposal promotes "hierarchies" of health reports, then "cassandra:connections" should be responsible for aggregating the health of its upstream dependencies. Otherwise it may bring duplication and/or risk inconsistencies, wouldn't it?

Cheers!

@dret
Copy link
Contributor

dret commented Oct 13, 2018 via email

@inadarei
Copy link
Owner

Thank you Tomas.

  • re "'warn' is mentioned with a MUST and later with a SHOULD." can you please give more info where this is happening or submit PR that fixes it? I am failing to find what you are referring to.

The reason details use arrays is to support multi-node downstream dependencies. And granted not all dependencies are multi-node, but if you allow arrays a value being sometimes an array and sometimes not would put undue hurdle on parsers (clients). Better to use single-value arrays.

@taliaga
Copy link
Contributor Author

taliaga commented Oct 19, 2018

Added #25 regarding the MUST - SHOULD for warning status.

I totally agree that it;s better to play safe towards the single-element-array rather than falling short on accommodating use-cases. But just to understand, by "multi-node downstream" you mean like for example if service S depends on a 3 replica-set of mongoDB instances the array would be used to return the status of the three replicas?.

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

No branches or pull requests

3 participants