nsqadmin: JS error with no clients #642
Conversation
|
Here's what the hostname stuff (#341) looks like:
|
| BroadcastAddress string `json:"broadcast_address"` | ||
| Hostname string `json:"hostname"` | ||
| HTTPPort int `json:"http_port"` | ||
| TCPPort int `json:"tcp_port"` |
There was a problem hiding this comment.
Looking at what else is included in /stats should start_time or health be included here?
There was a problem hiding this comment.
Hmmm, one way of thinking about the difference between the endpoints is that /info returns "static" while /stats returns dynamic metadata.
If that's true, then I don't think we would add health but perhaps start_time though?
There was a problem hiding this comment.
That sounds reasonable. Since realizing that /stats can be very heavy weight i keep thinking of lighter ways to get some of that data. We do have topic & channel filtering there so that certainly helps.
|
updated @jehiah |
| parse: function(response) { | ||
| response['nodes'] = _.map(response['nodes'] || [], function(node) { | ||
| var nodeParts = node['node'].split(':'); | ||
| node['show_broadcast_address'] = node['hostname'] !== nodeParts[0]; |
There was a problem hiding this comment.
Noticing from your screenshot, can you make the comparison of hostname case insensitive here and the other function?
|
👍 🔨 |
mreiferson
force-pushed
the
nsqadmin_js_error_642
branch
from
ee17ac8 to
5bd6fa4
Compare
|
squashed |
nsqadmin: JS error with no clients
| Version: infoResp.Version, | ||
| VersionObj: version, | ||
| BroadcastAddress: infoResp.BroadcastAddress, | ||
| Hostname: infoResp.Hostname, |
There was a problem hiding this comment.
It didn't occur to me at the time, but there is a compatibility issue with expecting this information back from the new /info endpoint. Namely w/o these fields nsqadmin is unable to display the topic or channel page for older hosts because it will attempt to query /info and use this empty field to query /stats.
[nsqadmin] 2015/09/21 13:37:11.922482 CI: querying nsqd http://127.0.0.1:4151/info
[nsqadmin] 2015/09/21 13:37:11.923098 CI: querying nsqd http://:0/stats?format=json
[nsqadmin] 2015/09/21 13:37:11.923515 ERROR: failed to get channel metadata - failed to query any nsqd
There was a problem hiding this comment.
I was aware of this, however the /nodes page was already completely broken when configuring nsqd directly, so I didn't think it made any sense to try to be backwards compatible.
I suppose if you went directly to the page for a specific node you'd still encounter this issue?
There was a problem hiding this comment.
For context, this change unexpectedly broke /topics/... and /topics/.../channel which previously worked.
This started off as a fix to a bug we introduced in our recent
nsqadminrefactoring (#323)... and then I got carried away and fixed #627 and #341,
which required propagating certain metadata differently in the
internal/clusterinfopackage as well as adding more basic metadata to
nsqd's/infoendpoint.RFR @jehiah