----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42912/#review119629 -----------------------------------------------------------
Fix it, then Ship it! I've added a few minor issues that need attention, but otherwise the patch looks fine to me. I've reviewed this patch from the perspective of the server-side Ambari changes, the .js code changes should be reviewed by someone with UI expertise. One question though: Why can't this fix be tested end-to-end? Isn't this just a matter of verifying that the UI uses the proper names consistently for the client components listed here? Thanks for providing this patch! ambari-server/src/main/java/org/apache/ambari/server/controller/ServiceComponentHostResponse.java (line 103) <https://reviews.apache.org/r/42912/#comment180981> Why does this property need a setter? If the display name always comes from the metadata, can't this property be immutable? ambari-server/src/main/java/org/apache/ambari/server/controller/ServiceComponentResponse.java (line 105) <https://reviews.apache.org/r/42912/#comment180982> Same as my comment above: Can't this property be immutable, since it is always defined by the metainfo.xml metadata? - Robert Nettleton On Feb. 18, 2016, 9:44 a.m., Daniel Gergely wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42912/ > ----------------------------------------------------------- > > (Updated Feb. 18, 2016, 9:44 a.m.) > > > Review request for Ambari, Jaimin Jetly, Oliver Szabo, Oleg Nechiporenko, > Robert Nettleton, Sumit Mohanty, Sebastian Toader, and Yusaku Sako. > > > Bugs: AMBARI-14830 > https://issues.apache.org/jira/browse/AMBARI-14830 > > > Repository: ambari > > > Description > ------- > > Clients names were different on host details page and filter for components > Pig, Sqoop, Slider, Mahout. > Displayed names now come from metainfo.xml > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/controller/ServiceComponentHostResponse.java > c25c970 > > ambari-server/src/main/java/org/apache/ambari/server/controller/ServiceComponentResponse.java > f7dd301 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ComponentResourceProvider.java > 3ad6e64 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostComponentResourceProvider.java > a2a58e8 > > ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentImpl.java > 7e1dd1d > > ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostImpl.java > bfb6214 > > ambari-server/src/main/resources/common-services/MAHOUT/1.0.0.2.3/metainfo.xml > bd8ef70 > > ambari-server/src/main/resources/common-services/PIG/0.12.0.2.0/metainfo.xml > 27a9c35 > > ambari-server/src/main/resources/common-services/SLIDER/0.60.0.2.2/metainfo.xml > a2002f1 > > ambari-server/src/main/resources/common-services/SQOOP/1.4.4.2.0/metainfo.xml > b5db91b > ambari-server/src/main/resources/properties.json 4052ad2 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProviderTest.java > 7643abb > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ComponentResourceProviderTest.java > f38fab1 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostComponentResourceProviderTest.java > f9c1fe4 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/HostResourceProviderTest.java > c480156 > ambari-web/app/controllers/global/update_controller.js 0a5d913 > ambari-web/app/mappers/components_state_mapper.js 0f2b627 > ambari-web/app/mappers/hosts_mapper.js 7691f2b > ambari-web/app/models/client_component.js 816950c > ambari-web/app/models/host_component.js 77395f2 > > Diff: https://reviews.apache.org/r/42912/diff/ > > > Testing > ------- > > The existing unit tests are aligned to the modifications, but I did not find > a way to e2e test the fix. > > > Thanks, > > Daniel Gergely > >
