nsqadmin: display errors #421
Conversation
mreiferson
force-pushed
the
nsqadmin_errors_421
branch
from
2802cd6 to
a7b6c0e
Compare
|
work in progress:
|
mreiferson
force-pushed
the
nsqadmin_errors_421
branch
from
a7b6c0e to
a0c34e9
Compare
|
@mreiferson this approach looks good |
mreiferson
force-pushed
the
nsqadmin_errors_421
branch
4 times, most recently
from
70d956b to
55a1597
Compare
mreiferson
force-pushed
the
nsqadmin_errors_421
branch
from
b9abf49 to
33e09ea
Compare
|
@rolyatmax would you mind taking a look at the JS error handling approaches here? ❤️ |
|
@jehiah want to give this a spin? |
| @@ -2,7 +2,7 @@ var Backbone = require('backbone'); | |||
|
|
|||
| var AppState = require('../app_state'); | |||
|
|
|||
| var Node = require('../models/node'); | |||
| var Node = require('../models/node'); //eslint-disable-line no-undef | |||
There was a problem hiding this comment.
You should add:
"globals": {
"module": true,
"require": true
}
to your .eslintrc.
There was a problem hiding this comment.
is it because Node is a restricted keyword?
There was a problem hiding this comment.
Not a restricted keyword.
Nodejs is a holy word - maybe that's what you're thinking of?
|
updated @rolyatmax |
|
Updates look good to me. Have you tried producing all these error states? 😬 |
|
most of them 👊 |
|
Sweeeeet - I'll leave the Go review for a GoPro. |
|
@jehiah tested this a bunch against a real cluster - used https://github.com/apenwarr/sshuttle/ which is pretty amazing |
mreiferson
force-pushed
the
nsqadmin_errors_421
branch
from
5a40f5a to
198f7ba
Compare
mreiferson
force-pushed
the
nsqadmin_errors_421
branch
3 times, most recently
from
f89e3c5 to
8b05ee1
Compare
|
OK, I think this is RFR @jehiah |
| @@ -13,7 +14,7 @@ | |||
| <div id="container"></div> | |||
|
|
|||
| <script type="text/javascript"> | |||
| var USER_AGENT = 'nsqadmin'; | |||
| var USER_AGENT = 'nsqadmin v{{.Version}}'; | |||
There was a problem hiding this comment.
nsqadmin/{{.Version}} to follow rfc7231 section 5.5.3
|
LGTM aside from my nitpick. (sorry for the delay in review; i was offline for a few days) |
mreiferson
force-pushed
the
nsqadmin_errors_421
branch
from
8b05ee1 to
b6e463a
Compare
|
no worries, fixed and squashed in (remembered to update bin dater too this time) RFM |
nsqadmin doesn't display when it's unable to query all the appropriate hosts to return stats. It should list them as errors at the top of the page.
(it can be missleading when the query to one host timesout but you don't realize it)