Skip to content

Conversation

@allmightyspiff
Copy link
Member

This is just some of the groundwork for a new set of methods to manage and view monitoring information.
Currently "sl monitor status" will just show you the SERVICE_PING results for each server, and the last date it was checked.

screen shot 2014-10-29 at 2 59 35 pm

I wanted to get this pull request in early incase anyone has any architecture concerns.

@allmightyspiff
Copy link
Member Author

merging in master ruined everything... hold off on actually looking at this

@sudorandom
Copy link
Contributor

@allmightyspiff Yeah, I changed... basically everything about how the command line stuff works. Take a look at the current state of the repo to see how it's currently done. It now uses this library for most command-line based stuff: http://click.pocoo.org/3/

@allmightyspiff
Copy link
Member Author

ok, got my monitor stuff in with the click goodness. You might want to double check it though to make sure I did everything inline with what you expect.

The command itself does work like I expect though

Copy link
Contributor

Choose a reason for hiding this comment

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

Use click to handle colors for you. http://click.pocoo.org/3/utils/#ansi-colors

It makes the code a fair bit more readable and understandable.

Copy link
Member Author

Choose a reason for hiding this comment

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

@sudorandom thanks, I'll add that.

@allmightyspiff
Copy link
Member Author

@sudorandom ok, changes you requested have been added.

@sudorandom
Copy link
Contributor

@allmightyspiff I made a pull request to your pull request. allmightyspiff#1 It worked fine, too. Please take a look at the last comment in the PR and incorporate those changes yourself. The interface to fixture things is slightly different now. Sorry for the sudden changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can't there be multiple monitors per guest/hardware?

Copy link
Contributor

Choose a reason for hiding this comment

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

For example, ping the frontend and backend IP addresses.

@allmightyspiff
Copy link
Member Author

@sudorandom let me know what you think about these changes now.

I've changed the monitoring status list to show all results that get pulled in, instead of just the first one. So you should now see everything that is being monitored with a SERVICE_PING or SLOW_PING.

@underscorephil
Copy link
Contributor

ping @sudorandom

Copy link
Contributor

Choose a reason for hiding this comment

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

utils.IdentifierMixin isn't needed. You don't specify ways to look up whatever a "monitor" would be based on something other than IDs.

Copy link
Contributor

Choose a reason for hiding this comment

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

To be more explicit, maybe it's better to do this instead:

if 'lastResult' in monitor:
 ...
else:
...

The way it works now, a key error for another value could happen and subsequently hidden.

@allmightyspiff allmightyspiff deleted the issues57 branch August 31, 2020 22:26
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.

3 participants