ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Richard Zang" <rz...@hortonworks.com>
Subject Re: Review Request 28736: Ambari View > Versions > Versions table + Registration E2E integration
Date Sat, 06 Dec 2014 00:20:24 GMT


> On Dec. 5, 2014, 12:53 p.m., Andrii Tkach wrote:
> > ambari-admin/src/main/resources/ui/admin-web/app/scripts/controllers/stackVersions/StackVersionsListCtrl.js,
line 97
> > <https://reviews.apache.org/r/28736/diff/1/?file=783448#file783448line97>
> >
> >     Better to use local variable and then set final array to $scope.stacks, because
changes to $scope.stacks affect DOM.

I think this usage is OK under the context it's in. This callback function does not trigger
any network based asynchronization which means angular $digest loop won't kick in during the
execution. I double checked this in angular doc at https://docs.angularjs.org/guide/scope
end of the page. 
It's a good suggestion to modify variable under $scope in local copy. For a reference type
variable, unless we make a deep clone of it, holding it with another reference won't separate
the actual object from been $watched.


> On Dec. 5, 2014, 12:53 p.m., Andrii Tkach wrote:
> > ambari-admin/src/main/resources/ui/admin-web/app/scripts/services/StackVersions.js,
line 83
> > <https://reviews.apache.org/r/28736/diff/1/?file=783450#file783450line83>
> >
> >     Repository version should take value of Version Name input, not a combination
of stack and version.

I haven't finished the auto population of stack version prefix in repo version input. I'm
planning to make repo version input hold only subversion of the repo which means it only hold
0.1-882 when the whole repo version is 2.2.0.1-882, that's why I concatenate stack and version.
After I finish everything, repo version input will be disabled first and then prepopulated
with stack version after user select the stack. User will not be able to alter stack version
prefix, they can only add sub version to it like shown below:

Stack          2.2
Version Name:  2.2[<user-input>]


> On Dec. 5, 2014, 12:53 p.m., Andrii Tkach wrote:
> > ambari-admin/src/main/resources/ui/admin-web/app/scripts/services/StackVersions.js,
line 84
> > <https://reviews.apache.org/r/28736/diff/1/?file=783450#file783450line84>
> >
> >     I guess display name should be arbitrary label entered by user

This is according to design PPT.


> On Dec. 5, 2014, 12:53 p.m., Andrii Tkach wrote:
> > ambari-admin/src/main/resources/ui/admin-web/app/scripts/services/StackVersions.js,
line 97
> > <https://reviews.apache.org/r/28736/diff/1/?file=783450#file783450line97>
> >
> >     The /api/v1/stacks/HDP/versions/2.2/operating_systems?fields=repositories/*
already contains properties of repositories such as "repo_id" and "repo_name", only "base_url"
could be modified

This is what we do currentlhy for repo_id as we agreed in the meeting.


- Richard


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


On Dec. 6, 2014, 12:19 a.m., Richard Zang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28736/
> -----------------------------------------------------------
> 
> (Updated Dec. 6, 2014, 12:19 a.m.)
> 
> 
> Review request for Ambari, Andrii Tkach and Yusaku Sako.
> 
> 
> Bugs: AMBARI-8554
>     https://issues.apache.org/jira/browse/AMBARI-8554
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Ambari View > Versions > Versions table + Registration E2E integration
> 
> 
> Diffs
> -----
> 
>   ambari-admin/src/main/resources/ui/admin-web/app/assets/data/cluster/clusters.json
PRE-CREATION 
>   ambari-admin/src/main/resources/ui/admin-web/app/scripts/controllers/stackVersions/StackVersionsCreateCtrl.js
ae984b8 
>   ambari-admin/src/main/resources/ui/admin-web/app/scripts/controllers/stackVersions/StackVersionsListCtrl.js
698bc6c 
>   ambari-admin/src/main/resources/ui/admin-web/app/scripts/services/Cluster.js c474918

>   ambari-admin/src/main/resources/ui/admin-web/app/scripts/services/StackVersions.js
ae499cc 
>   ambari-admin/src/main/resources/ui/admin-web/app/views/stackVersions/create.html 11e954f

>   ambari-admin/src/main/resources/ui/admin-web/app/views/stackVersions/list.html 97e67c9

> 
> Diff: https://reviews.apache.org/r/28736/diff/
> 
> 
> Testing
> -------
> 
> Manually tested on live cluster. No unit test applicable.
> 
> 
> Thanks,
> 
> Richard Zang
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message