> 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 > >
