----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56426/#review164745 -----------------------------------------------------------
ambari-web/app/controllers/main/service/manage_config_groups_controller.js (line 569) <https://reviews.apache.org/r/56426/#comment236500> parentconfiggroup should already be sorted to beging with as we are sorting all config groups in config group mapper. rmoving a host should keep the list sorted is again sorting required over here ? ambari-web/app/controllers/main/service/manage_config_groups_controller.js (line 590) <https://reviews.apache.org/r/56426/#comment236502> same as above. if a host is being emoved from a sorted list, again sorting the list may not be required. ambari-web/app/mappers/configs/config_groups_mapper.js (line 71) <https://reviews.apache.org/r/56426/#comment236503> sorting a list might not be very expensive on client side. But if there are multiple config groups with considerable hosts in each then it can have some performace impact. I don't see this to be significant. so I am ok with commiting this patch. But good to track this as a future requirement for ambari-server to expose an API that gives sorted hostnames for a configgroup by default or via sortBy predicate. - Jaimin Jetly On Feb. 8, 2017, 3:34 a.m., Richard Zang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56426/ > ----------------------------------------------------------- > > (Updated Feb. 8, 2017, 3:34 a.m.) > > > Review request for Ambari and Xi Wang. > > > Bugs: AMBARI-19912 > https://issues.apache.org/jira/browse/AMBARI-19912 > > > Repository: ambari > > > Description > ------- > > Keep host list sorted on initial loading as well as after any operation. > > > Diffs > ----- > > ambari-web/app/controllers/main/service/manage_config_groups_controller.js > 07756f0 > ambari-web/app/mappers/configs/config_groups_mapper.js 8700135 > > Diff: https://reviews.apache.org/r/56426/diff/ > > > Testing > ------- > > Manually tested on live cluster. > All unit tests passed. > 20327 passing (21s) > 153 pending > > > Thanks, > > Richard Zang > >
