grafana / carbon-relay-ng

Fast carbon relay+aggregator with admin interfaces for making changes online - production ready

Other

467 stars 151 forks source link

post REST & SPA comments #25

Open Dieterbe opened 10 years ago

Dieterbe commented 10 years ago

21 brought a revamped admin UI as a single page app, based on a rest api exposed by the go program. neat stuff, and it got merged.

however there's a few small remarks I'ld like to addressed:

when no fields are filled in, and you click "add route", it doesn't show error messages like the previous codebase did.
when a route comes back up, it stays red, until you reload the page. might be nice to automatically update the online state. (if it's not too much of a hassle)
removeRoute needs a comment explaining why we create a new map[string]string

cc @pwielgolaski

pwielgolaski commented 10 years ago

when no fields are filled in, and you click "add route", it doesn't show error messages like the previous codebase did.

I did it in a way that you can not press Add if fields are invalid, do you think it would be better to click it and get some error message?

when a route comes back up, it stays red, until you reload the page. might be nice to automatically update the online state. (if it's not too much of a hassle)

I think that I can do autorefresh every minutes if you think it would be what you want

removeRoute needs a comment explaining why we create a new map[string]string

I can add comment, the reason is to fulfil contract with ServeHTTP method

Dieterbe commented 10 years ago

I did it in a way that you can not press Add if fields are invalid, do you think it would be better to click it and get some error message?

yeah, I think it's better to be more clear that there's an error in the input

I think that I can do autorefresh every minutes if you think it would be what you want

let's not do that. this could become very annoying if it refreshes right when you're in the middle of something. i was hoping maybe there was an elegant, simple way to update on the fly (by polling for updates in the background every second or so, or maybe even the server pushing updates when the route state changes) further down the road, i really want the ui to display performance metrics in real-time and have them auto-update (without refreshing the page)

pwielgolaski commented 10 years ago

I think that I wasn't clear enough, when I said refresh it is not refresh of whole page, but only list of routes.

You can grab gist version of app.js and give a try: https://gist.github.com/pwielgolaski/520b133e72e98dc512fa

Dieterbe commented 10 years ago

aha yes that updates the routes status nicely. is there any possibly race conditions when you're trying to edit a route and save it but at the bad time it updates with fresh values from the refresh, or something? otherwise, let's merge this? :+1:

pwielgolaski commented 10 years ago

I dont see any issue with race condition, but I'd like to wait for https://github.com/graphite-ng/carbon-relay-ng/issues/22 to be fixed, as it is annoying that you can get routes in different order and it will automatically refresh list changing order of routes.

Dieterbe commented 9 years ago

hey @pwielgolaski any interest in taking back up the web interface? you did nice work last year but i had to rearchitect some core code and concepts. now that stuff has matured a bit more and it's mostly the web and telnet interfaces that need some love :)