Add a status dashboard #31
Conversation
|
|
|
@blag could you put up a screenshot in the PR description? Thanks! |
|
Done! :) Working under the assumption that default templates are more guidelines than complete solutions, I made the default template fairly basic. However, I did add in Bootstrap 3 classes, and the modal windows are implemented with Bootstrap. The page should gracefully degrade if a site doesn't use Bootstrap, and although it won't look pretty, it should still be functional. |
|
First of all, I'd just like to say "Thank you!" for this PR - looks great! I'm wondering if it doesn't make more sense to make the template + styling + behaviour (modals for tracebacks, etc.) more standalone, rather than relying on Bootstrap and falling back to no styling. Seems like it would be pretty straightforward to build out the standalone template, and just load Bootstrap (or something smaller like PureCSS / min) from a CDN? While Bootstrap is pretty common, I wouldn't be surprised if there are a lot of people not using it, or still using Bootstrap 2. This is also a tool meant for internal teams, so a bit of extra time to download another css framework on first page load doesn't seem ridiculous. Thoughts? |
|
Of the Django apps that I've taken a look at, Bootstrap 3 seems to be the most commonly supported framework. That doesn't mean that older versions of those apps aren't being run, just that most apps have updated their templates to assume Bootstrap 3. I would not be surprised to hear about internal tools using no framework, Bootstrap 2, Foundations, or something else, but I think the thing to keep in mind is that this template isn't very complicated and - being a Django1 template - is easily overridable. That basically means that we are really only discussing what to support by default. For this PR, I wanted something that looks decently pretty "out of the box" with a minimum of fuss but still worked without dependencies, and Bootstrap 3 filled that role perfectly. So to be perfectly clear: I don't see it being worth it to switch to something else. 1 Meaning awesome. |
|
I think I'd prefer to make the dashboard functional / pretty out of the box, and I think that's easiest to do my making the dashboard template standalone, rather than extending a base template from the Django project. Slightly selfishly, we don't use Bootstrap 3 at work, and I'd prefer not to have to override the template for every project we use django-watchman on.. 😄 I checked out your branch and poked around at it a bit:
Here's the code: {% load i18n %}
<!DOCTYPE html>
<html lang="en">
<head>
<meta name="viewport" content="width=device-width, initial-scale=1">
<link rel="stylesheet" href="//maxcdn.bootstrapcdn.com/bootstrap/3.3.5/css/bootstrap.min.css">
<script src="//maxcdn.bootstrapcdn.com/bootstrap/3.3.5/js/bootstrap.min.js"></script>
</head>
<body>
{% block content %}
<div class="container">
<h2>{% trans 'System Status' %}</h2>
<h3 class="{% if overall_status == 'OK' %}text-success{% else %}text-danger{% endif %}">
{% if overall_status == 'OK' %}
<span class="glyphicon glyphicon-ok"></span> {% trans 'All systems up and running!' %}
{% else %}
<span class="glyphicon glyphicon-remove"></span> {% trans 'WARNING - Some systems down!' %}
{% endif %}
</h3>
<small></small>
<table class="table table-bordered table-hover">
<thead>
<tr>
<th>{% trans 'Type' %}</th>
<th>{% trans 'Name' %}</th>
<th>{% trans 'Status' %}</th>
</tr>
</thead>
<tbody>
{% for type in checks %}
{% for thing in type.statuses %}
<tr class="{% if type.overall_status == 'OK' %}success{% else %}danger{% endif %}">
<td>{{ type.type|title }}</td>
<td>{{ thing.name|title }}</td>
{% if thing.status == 'OK' %}
<td class="success">{% trans 'OK' %}</td>
{% else %}
<td class="danger">
<button type="button" class="btn btn-danger" data-toggle="modal" data-target="#{{ type.type }}-{{ thing.name }}">{% trans 'ERROR!' %}</button>
<div class="modal fade" id="{{ type.type }}-{{ thing.name }}" tabindex="-1" role="dialog" aria-labelledby="{{ type.type }}-{{ thing.name }}-title">
<div class="modal-dialog" role="document">
<div class="modal-content">
<div class="modal-header">
<button type="button" class="close" data-dismiss="modal" aria-label="Close"><span aria-hidden="true">×</span></button>
<h4 class="modal-title" id="{{ type.type }}-{{ thing.name }}-title">{{ type.type|title }} - {{ thing.name|title }}</h4>
</div><!-- class="modal-header" -->
<div class="modal-body">
<h4><pre>{{ thing.error }}</pre></h4>
<pre>{{ thing.traceback }}</pre>
</div><!-- class="model-body" -->
<div class="modal-footer">
<button type="button" class="btn btn-default" data-dismiss="modal">Close</button>
</div><!-- class="modal-footer" -->
</div>
</div>
</div>
</td>
{% endif %}{# thing.status == 'OK' #}
</tr>
{% endfor %}{# for thing in type.statuses #}
{% empty %}
<tr>
<td colspan="3">{% trans 'No checks indicated.' %}</td>
</tr>
{% endfor %}{# for type in checks #}
</tbody>
</table>
{% if overall_status != 'OK' %}<span class="pull-right"><small>{% trans 'Click an error button to see the full traceback' %}</small></span>{% endif %}{# overall_status != 'OK' #}
</div>
{% endblock %}{# status_content #}
</body>
</html>What do you think about doing something like that? |
|
That looks great! The only change I would make is to use the |
|
|
|
Sure, I can do that! It will very likely be sometime later this weekend though. |
|
No prob. :) |
…ry and Bootstrap 3, and use the 'alert' (not 'remove') glyphicon
| # Example: [{'default': {'ok': True}}] | ||
| statuses = [] | ||
|
|
||
| for a in _check[_type]: |
There was a problem hiding this comment.
Could we use something a little more descriptive than a here? Maybe result?
|
Think it's worth adding a few django TestClient tests for this view? |
|
@blag Sorry for the delay, finished my first full pass through the code. Hopefully my suggestions make sense.. if not, let me know. 😄 |
|
|
|
Yeah, I've also seen tests to verify that the correct template is loaded. Not sure if that really adds much value here though.. maybe more so if the template is dynamic. |
| <meta name="viewport" content="width=device-width, initial-scale=1"> | ||
| <link rel="stylesheet" href="//maxcdn.bootstrapcdn.com/bootstrap/3.3.5/css/bootstrap.min.css"> | ||
| <script src="//code.jquery.com/jquery-2.1.4.min.js"></script> | ||
| <script src="//maxcdn.bootstrapcdn.com/bootstrap/3.3.5/js/bootstrap.min.js"></script> |
There was a problem hiding this comment.
Apparently we should probably just hardcode these to https://: http://www.paulirish.com/2010/the-protocol-relative-url/
Now that SSL is encouraged for everyone and doesn’t have performance concerns, this technique is now an anti-pattern. If the asset you need is available on SSL, then always use the https:// asset.
Allowing the snippet to request over HTTP opens the door for attacks like the recent Github Man-on-the-side attack. It’s always safe to request HTTPS assets even if your site is on HTTP, however the reverse is not true.
|
Ok, a couple of last comments on another pass through, and then I think I'm good to go with this PR! |
…hing' in the template
|
@blag Hoping to get these merged in tomorrow. |
|
Cool! I can be on standby today to fix any last minute issues that may arise. The odd Travis issues I experienced have made me just a tiny bit worried and skeptical that it's going to go perfectly. |
|
That looks like it was a network failure: All of the builds passed except for one while installing pip packages, which also leads me to believe it was a temporary issues: |
|
Now that these are merged in, can you cut a new release on PyPI? I'd like to remove my vendored version of watchman. Thanks for all of your work reviewing/commenting/advising! 😃 |
|
@blag Hopefully soon, doing some testing on master with all of the PRs merged together. |
|
@blag I've pushed up a |
|
@blag Oh, also, would you like to be referred to as |
|
@blag Ok, released 0.6.0 final, with a few extra tweaks (you can pass get params to the dashboard view to customize the checks like you can with the json view, and I added a |
|
Thanks for asking about my name preferences - what you have now is fine. If I really cared I would have made that part of my PR. 😉 I took a look at #34, and it looked good to me, aside from the (truly) tiny defense-in-depth violation I pointed out. Thanks for releasing I will probably work on #5 and #8 at some point "real soon now", because I will need them for a project. |
|
@blag No problem. That'd be awesome to get the ES / celery checks built - let me know if you have any questions and I can try to help you out. |
Visual indications of overall and subsystem "good" status for quick and easy checking:
Visual and textual hints of overall and subsystem "error" status to direct attention to failing subsystems and provide hints to view more failure information:
Full error text and tracebacks rendered in modal windows to avoid cluttering the main table but still give useful failure information (a mocked-up version is shown here):
Likely closes #11.