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
Issues57 - groundwork for monitoring #429
Conversation
merging in master ruined everything... hold off on actually looking at this |
@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/ |
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 |
server = utils.NestedDict(server) | ||
res = server['networkMonitors'][0]['lastResult']['responseStatus'] | ||
date = server['networkMonitors'][0]['lastResult']['finishTime'] | ||
status = '\033[35mUNKNOWN\033[0m' |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
@sudorandom ok, changes you requested have been added. |
@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. |
results = hardware + guest | ||
for server in results: | ||
server = utils.NestedDict(server) | ||
res = server['networkMonitors'][0]['lastResult']['responseStatus'] |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
@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. |
ping @sudorandom |
from SoftLayer import utils | ||
|
||
|
||
class MonitoringManager(utils.IdentifierMixin, object): |
There was a problem hiding this comment.
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.
date = monitor['lastResult']['finishTime'] | ||
ip_address = monitor['ipAddress'] | ||
monitor_type = monitor['queryType']['name'] | ||
except KeyError: |
There was a problem hiding this comment.
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.
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.
I wanted to get this pull request in early incase anyone has any architecture concerns.