> On April 29, 2016, 11:18 p.m., Jaimin Jetly wrote:
> > ambari-web/app/mappers/stack_mapper.js, line 140
> > <https://reviews.apache.org/r/46460/diff/2/?file=1367769#file1367769line140>
> >
> >     A minor thing. This line can be changed to
> >     stack.use_redhat_satellite = 
> > !item.operating_systems[0].OperatingSystems.ambari_managed_repositories;
> >     
> >     Logiclly it's the same but this way it's just more readable.
> 
> Zhe (Joe) Wang wrote:
>     The reason I wrote it that is:
>     if item.operating_systems[0].OperatingSystems.ambari_managed_repositories 
> is undefined, we want stack.use_redhat_satellite to be false.
>     So there is a slight difference between these to ways of writing it.
> 
> Jaimin Jetly wrote:
>     I see. I was not aware getting undefined value from API is a possiblity 
> over here. Usually that's not the case anywhere else in ambari APIs. can we 
> do any one of the following so that someone else does not refactor the code 
> incorrectly in the future:
>     1) Leave 1-line comment over there explaining this.
>     2) Add a unit test. so it correctly breaks if someone tries to refactor 
> it in any other way
>     3) Write it in a way it's clear that we are accounting for undefined 
> values: var ambariManagedVersion = 
> item.operating_systems[0].OperatingSystems.ambari_managed_repositories; 
> stack.use_redhat_satellite = !Em.none(ambariManagedVersion) && 
> !ambariManagedVersion

Will do. And actually you are right: based on current code, it's not possible 
to get an undefined there. I was just being safe.


- Zhe (Joe)


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


On April 29, 2016, 11:12 p.m., Zhe (Joe) Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46460/
> -----------------------------------------------------------
> 
> (Updated April 29, 2016, 11:12 p.m.)
> 
> 
> Review request for Ambari, Jaimin Jetly, Nate Cole, Richard Zang, Xi Wang, 
> and Yusaku Sako.
> 
> 
> Bugs: AMBARI-16002
>     https://issues.apache.org/jira/browse/AMBARI-16002
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Provide RedHat satellite option. It can be used wherever we show redhat6 
> urls, and applicable to redhat6/redhat7. For example, registering repos, etc.
> The API call to update links when using VDF can specify the new property. In 
> order to make it more generic (and default true), it is called 
> "ambari_managed_repositories". API looks like:
> PUT /api/v1/stacks/HDP/versions/2.3/repository_versions/2
> {
>   "RepositoryVersions" : {
>     "id" : 2
>   },
>   "operating_systems" : [
>     {
>       "OperatingSystems" : {
>         "ambari_managed_repositories": false,
>         "os_type" : "redhat6",
>         "repository_version_id" : 2,
>         "stack_name" : "HDP",
>         "stack_version" : "2.3"
>       },
>       "repositories" : [
>         {
>           "Repositories" : {
>             "base_url" : "http://repos.ambari.apache.org/hdp/HDP-2.3.4.14-4";,
>             "repo_id" : "HDP-2.3",
>             "repo_name" : "HDP"
>           }
>         },
>         {
>           "Repositories" : {
>             "base_url" : 
> "http://repos.ambari.apache.org/hdp/HDP-UTILS-1.1.0.20";,
>             "repo_id" : "HDP-UTILS-1.1.0.20",
>             "repo_name" : "HDP-UTILS"
>           }
>         }
>       ]
>     }
>   ]
> }
> 
> 
> Diffs
> -----
> 
>   ambari-web/app/controllers/installer.js 39a7812 
>   ambari-web/app/mappers/stack_mapper.js 4f73612 
>   ambari-web/app/models/stack.js 992a79c 
>   ambari-web/app/templates/wizard/step1.hbs 0476580 
>   ambari-web/app/utils/ajax/ajax.js 83914a7 
>   
> ambari-web/app/views/main/admin/stack_upgrade/upgrade_version_column_view.js 
> 168e8db 
> 
> Diff: https://reviews.apache.org/r/46460/diff/
> 
> 
> Testing
> -------
> 
> Modified unit test.
> Local ambari-web test passed.
> 25660 tests complete (24 seconds)
> 154 tests pending
> Manual testing done.
> 
> 
> Thanks,
> 
> Zhe (Joe) Wang
> 
>

Reply via email to