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

No data available status #2194

Merged
merged 1 commit into from
Oct 24, 2016

Conversation

nicolasfagotti
Copy link
Contributor

We added a new status for the components, "No Data Available". This doesn't seem meaningful for manual modifications, but when components are regularly updated through Cachet API, it's pretty useful to have it.

If it's not desirable, my apologies and just cancel the pull request :)

@jbrooksuk jbrooksuk added this to the V2.4.0 milestone Oct 18, 2016
@jbrooksuk
Copy link
Member

Hmm, I like the idea of having an almost indeterminate status, but I don't know if I like the "No data available" status.

Actually, under what circumstances would you have no data?

@nixmomo
Copy link

nixmomo commented Oct 18, 2016

Some monitorings use the "unkown" status if they can't get informations about the service health from the monitoring client. Nagios as example use it :-)

If you use cachet as a internal status Page it can be helpful to have it to see that something is going wrong. if you match it hard to "down" and the service isn't really down so you have a status between down and up.

@nicolasfagotti
Copy link
Contributor Author

nicolasfagotti commented Oct 18, 2016

Well, for example, we query our components statuses not by hitting them directly but querying a central log application.

Cachet -> log app <- components

So, in our case, having no information in the logs doesn't mean anything about a component status. What we are looking for is a way to say that is not possible to define the status of the component.

@jbrooksuk
Copy link
Member

Okay, thanks for providing a better explanation.

I think I prefer "Unknown". Thoughts? Ping @CachetHQ/core.

@jbrooksuk jbrooksuk self-assigned this Oct 19, 2016
@nicolasfagotti
Copy link
Contributor Author

Yes, it sounds better "Unknown".

@jbrooksuk
Copy link
Member

Okay and two more points here:

  1. Can you only update the English translation, please? The reason is that we sync the English files up to CrowdIn, then they generate the files when needed. I'd have to go through every language and re-sync them. This makes it much easier for me to deal with 👍
  2. You'll need to update the ./app/Bus/Commands/Component command files to validate a status of 0 too, as it's currently only allowing 1-4.

@nicolasfagotti
Copy link
Contributor Author

If I'm not mistaken, the test that failed is not related to the changes on this branch.

@jbrooksuk
Copy link
Member

Can you squash into one commit, please? We have a commit adding languages, then removing them. I'll check that this does everything we expect on Sunday :)

@nicolasfagotti
Copy link
Contributor Author

Done.

Sorry about that. It's my first time with Cachet, Laravel, Github as issue tracker, etc. I hope my next pull request be cleaner :)

@jbrooksuk
Copy link
Member

Can you rebase this please? We changed some stuff in the Laravel upgrade. Once debased we will merge this immediately.

@jbrooksuk jbrooksuk mentioned this pull request Oct 24, 2016
@nicolasfagotti
Copy link
Contributor Author

Done

@jbrooksuk jbrooksuk merged commit a8307fa into cachethq:2.4 Oct 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants