> On Jan. 17, 2014, 6:24 p.m., Thomas Rampelberg wrote: > > src/webui/master/static/js/services.js, line 110 > > <https://reviews.apache.org/r/13606/diff/2/?file=423536#file423536line110> > > > > This could be something like: > > > > _.each([ 'cpus_user_time_secs' ], function(n) { this[n] = 0.0 }); > > > > Ben Mahler wrote: > Are you suggesting we keep a list of these metric names somewhere and > operate on them? I'm inclined to directly set them here as it's a little > easier for others to read.
This is totally a style comment. The way they are right now is totally fine, I just personally prefer to abstract the assignment out a little bit. > On Jan. 17, 2014, 6:24 p.m., Thomas Rampelberg wrote: > > src/webui/master/static/js/services.js, line 121 > > <https://reviews.apache.org/r/13606/diff/2/?file=423536#file423536line121> > > > > Because the parameters are the same here, why not just do this: > > > > _.each(this.metrics, function(v, name) { > > this.metrics[name] += stats[name]; > > } > > Ben Mahler wrote: > Does this work if new metrics are added from the backend and we didn't > capture them above in our initialization? It will have undefined behavior. By using the same list of variables that the initializer is using, however, you'll get guaranteed behavior (both in initialization and assignment). > On Jan. 17, 2014, 6:24 p.m., Thomas Rampelberg wrote: > > src/webui/master/static/slave.html, line 141 > > <https://reviews.apache.org/r/13606/diff/2/?file=423537#file423537line141> > > > > Having to look up something from a root object seems like the wrong way > > to do things. Maybe change the ng-repeat to fetch from both monitor and > > frameworks (doing an _.extend on the data) at the same time? > > Ross Allen wrote: > Maybe the controller should set a scope `statistics` variable that is > `monitor.frameworks[framework.id].statistics` so the view doesn't need to > repeat that lookup. The sidebar might also be appropriate as a directive. Good call, that would allow inheritance and reduce the code that's showing up in views. - Thomas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13606/#review32159 ----------------------------------------------------------- On Jan. 15, 2014, 2:30 a.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13606/ > ----------------------------------------------------------- > > (Updated Jan. 15, 2014, 2:30 a.m.) > > > Review request for mesos, Benjamin Hindman and Vinod Kone. > > > Repository: mesos-git > > > Description > ------- > > This adds a Top abstraction to the webui for periodically polling the > monitoring endpoint. > > Instead of having each controller have to deal with monitoring information > during the update cycle, this allows each controller to setup asynchronous > monitoring updates, stopping it when the controller becomes inactive. > > > Diffs > ----- > > src/webui/master/static/js/controllers.js > 01fe64ca6b784b210a5687f4e1cda60cead8671d > src/webui/master/static/js/services.js > 122519a5a4de93edf1fd7a5256e565cc78c59670 > src/webui/master/static/slave.html 134aa0b10a232a37654a4ef9ac4bb149dbbebdea > src/webui/master/static/slave_executor.html > 81c10cbf4dd77f65dd8c3081281740c2be1e5b56 > src/webui/master/static/slave_framework.html > 04a041e9a4e8e1364617d09412f0a81a160ee48a > > Diff: https://reviews.apache.org/r/13606/diff/ > > > Testing > ------- > > Ran the long-lived-framework. > > > Thanks, > > Ben Mahler > >
