Skip to content
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

implement new REST API for admin-http and front page as single page application #21

Merged
merged 2 commits into from
Oct 8, 2014
Merged

implement new REST API for admin-http and front page as single page application #21

merged 2 commits into from
Oct 8, 2014

Conversation

pwielgolaski
Copy link
Contributor

As you can see I implemented REST api for routes and rewrite front application.
What do you think, would it be good to merge?

@Dieterbe
Copy link
Contributor

for the record, this is based on earlier convo is #11.

haven't tried it yet, but conceptually it looks good to me.
angularjs scares me a bit, it's notorious in that the code can get obscure and complex, and as i mentioned in #11 i'm personally more interested in more lightweight solutions. but i guess it will be fine.

some questions though:

  • we can't put in the pagetitle where we are in the app? like we did in the original version
  • what does this mean on line 1 in app.js? "ngResource", "ui.bootstrap" ?
  • did you test various cases of edit/save/new with errors, without errors, etc?
  • do you think this will allow us to better modify the "schema" (which fields there are etc) of routes?
  • if I add some performance variables, exposed via json via the golang expvar mechanism, could those easily be shown on the webUI in the main page? (see expose [route] performance metrics #19) that would be awesome! maybe in an "expand for details" menu or something. and if we also put in the expvar's whether the route is currently up or down, than we could color the row in red if the route is down, or something like that.

@pwielgolaski
Copy link
Contributor Author

I'm aware that you are not to keen to go with angular, but if you have idea how to do it with something lighter, we can change it, I'm personally not big fan of backbone.

Some answers:

we can't put in the pagetitle where we are in the app? like we did in the original version

You are right, but now it is single page, so there is no context, but you just execute some action.

what does this mean on line 1 in app.js? "ngResource", "ui.bootstrap" ?

Angular allows to include additional dependencies and in this way you express that your application is using ngResource (https://docs.angularjs.org/api/ngResource/service/$resource) and angular-ui (http://angular-ui.github.io/bootstrap/)

did you test various cases of edit/save/new with errors, without errors, etc?

I tried to cover what come to my mind, so it should be ok.

do you think this will allow us to better modify the "schema" (which fields there are etc) of routes?

I think it is more clean separation and you could image that different application could use REST to administration

if I add some performance variables, exposed via json via the golang expvar mechanism, could those easily be shown on the webUI in the main page? (see #19) that would be awesome! maybe in an "expand for details" menu or something. and if we also put in the expvar's whether the route is currently up or down, than we could color the row in red if the route is down, or something like that.

it sound great, I'm not familiar with expvar, but I don't see why not do it.
and definitely I love idea to have different color if route is down

@Dieterbe
Copy link
Contributor

ok great. i will test this out and probably merge it later.
i would love to work with you on further enhancing this to display performance variables. expvar is basically just a http endpoint like localhost/expvar that returns a json document with performance variables. in there i want to put performance metrics for the different routes, which we could then also display in the ui. are you on irc? might be easier to talk about this in more real-time

@pwielgolaski
Copy link
Contributor Author

I’d be happy to help if I only find some time and sufficient know-how :)
I don’t use irc and recently I don’t have too much free time, but if you share your idea in github, we could work something out.
Assuming that you have any go struct that want to expose it should be rather trivial to expose it and then we have open question how would you like to present it :)

Piotr Wielgolaski
Sent with Sparrow (http://www.sparrowmailapp.com/?sig)

On Tuesday 30 September 2014 at 21:00, Dieter Plaetinck wrote:

ok great. i will test this out and probably merge it later.
i would love to work with you on further enhancing this to display performance variables. expvar is basically just a http endpoint like localhost/expvar that returns a json document with performance variables. in there i want to put performance metrics for the different routes, which we could then also display in the ui. are you on irc? might be easier to talk about this in more real-time


Reply to this email directly or view it on GitHub (#21 (comment)).

if err != nil {
return nil, &handlerError{nil, "Could not find entry " + key, http.StatusNotFound}
}
return make(map[string]string), nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a comment why we need to make a new map here, what is it used for?

@Dieterbe Dieterbe merged commit 07de49b into grafana:master Oct 8, 2014
@Dieterbe
Copy link
Contributor

Dieterbe commented Oct 8, 2014

ok looks good, i merged it. however i have a few followup comments, for which i opened #25

@pwielgolaski pwielgolaski deleted the admin_rest branch December 8, 2014 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants