> On April 9, 2014, 1:04 a.m., Yusaku Sako wrote:
> > We seem to be using StackServiceComponent, ServiceComponent, and 
> > StackComponent interchangeably.
> > Can we stick to one (I would vote for ServiceComponent).

StackServiceComponent is the Class. Instances of this class are the components 
supported by the chosen stack. Renaming this to ServiceComponent might be 
misleading. A new developer might perceive it to be the Class for all installed 
service components. 
ServiceComponent is the function variables used in certain functions in the 
code. Mainly it refers to filtering installed service components in a function.
StackComponent is only used once and it's also a function variable for 
handleStackDependedComponents function. I will rename it to 
StackServiceComponent which is more meaningful


> On April 9, 2014, 1:04 a.m., Yusaku Sako wrote:
> > ambari-web/app/controllers/wizard/step5_controller.js, line 230
> > <https://reviews.apache.org/r/20116/diff/1/?file=552266#file552266line230>
> >
> >     Should component_name be componentName?

No. masterComponents object variable is the argument received by this function 
from another function in this controller. This object saves component's name as 
component_name.


> On April 9, 2014, 1:04 a.m., Yusaku Sako wrote:
> > ambari-web/app/controllers/wizard/step5_controller.js, line 231
> > <https://reviews.apache.org/r/20116/diff/1/?file=552266#file552266line231>
> >
> >     Should component_name be componentName?

No. masterComponents object variable is the argument received by this function 
from another function in this controller. This object saves component's name as 
component_name.


> On April 9, 2014, 1:04 a.m., Yusaku Sako wrote:
> > ambari-web/app/controllers/wizard/step6_controller.js, line 255
> > <https://reviews.apache.org/r/20116/diff/1/?file=552267#file552267line255>
> >
> >     Why did we make this change?

Step-6 is Assign Slaves and Clients page. We made changes to 
getComponentDisplayName function to get the component's display name from the 
App.StackServiceComponent displayName attribute. This works for all the slave 
components as they are truly a service component and exist as an instance of 
App.StackServiceComponent model. But CLIENT is not any service component (It's 
UI specific terminology which represents all clients like HCAT ClIENT, HDFS 
CLIENT etc). So We needed to hard-code it over here.


- Jaimin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20116/#review39849
-----------------------------------------------------------


On April 8, 2014, 2:19 a.m., Jaimin Jetly wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20116/
> -----------------------------------------------------------
> 
> (Updated April 8, 2014, 2:19 a.m.)
> 
> 
> Review request for Ambari and Yusaku Sako.
> 
> 
> Bugs: AMBARI-5389
>     https://issues.apache.org/jira/browse/AMBARI-5389
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Web ui maintains a copy of service components supported by stack at 
> ambari-web/app/data/service_components.js. Whenever a new service component 
> is added in a stack, this copy needs to be updated. 
> As a part of resolution ambari-web should not rely on this file but get the 
> service component data from API.
> 
> 
> Diffs
> -----
> 
>   ambari-web/app/app.js e83319e 
>   ambari-web/app/assets/data/stacks/HDP-2.1/service_components.json 
> PRE-CREATION 
>   ambari-web/app/controllers/global/cluster_controller.js 7a1304f 
>   ambari-web/app/controllers/installer.js 4d8e21e 
>   ambari-web/app/controllers/main/host/add_controller.js 179c92a 
>   ambari-web/app/controllers/main/service/add_controller.js 8b1ce05 
>   ambari-web/app/controllers/main/service/item.js f78a57b 
>   ambari-web/app/controllers/main/service/manage_config_groups_controller.js 
> bcd7e15 
>   ambari-web/app/controllers/wizard.js 4affb3b 
>   ambari-web/app/controllers/wizard/step5_controller.js 40c1f99 
>   ambari-web/app/controllers/wizard/step6_controller.js b7f4639 
>   ambari-web/app/controllers/wizard/step7_controller.js b7a9cfd 
>   ambari-web/app/controllers/wizard/step8_controller.js 9958dda 
>   ambari-web/app/controllers/wizard/step9_controller.js 5a133ed 
>   ambari-web/app/data/HDP2/global_properties.js a3eedaa 
>   ambari-web/app/data/service_components.js f066c46 
>   ambari-web/app/initialize.js 777779a 
>   ambari-web/app/mappers.js PRE-CREATION 
>   ambari-web/app/mappers/stack_service_component_mapper.js PRE-CREATION 
>   ambari-web/app/models.js ce53c73 
>   ambari-web/app/models/stack_service_component.js PRE-CREATION 
>   ambari-web/app/utils/ajax.js 4f25d93 
>   ambari-web/app/utils/component.js dec45a7 
>   ambari-web/app/views/main/host/summary.js d367e89 
>   ambari-web/test/app_test.js 4830b58 
>   ambari-web/test/installer/step5_test.js 57cef17 
>   ambari-web/test/installer/step6_test.js e323f2d 
>   ambari-web/test/installer/step9_test.js 4ecd864 
> 
> Diff: https://reviews.apache.org/r/20116/diff/
> 
> 
> Testing
> -------
> 
> tested e2e and fixed broken unit tests impacted by the patch.
> 
> 
> Thanks,
> 
> Jaimin Jetly
> 
>

Reply via email to