> On Янв. 19, 2015, 4:19 п.п., Yurii Shylov wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/PrereqCheckRequest.java,
> >  line 27
> > <https://reviews.apache.org/r/30038/diff/1/?file=825132#file825132line27>
> >
> >     Looks like PrereqCheckRequest is going to become a common class for any 
> > checks and thus repositoryVersion is no longer a necessary requirement. We 
> > can remove TODO as not valid anymore.
> 
> Robert Levas wrote:
>     I think this should be a separate issue since it seems to be more than 
> making `repositoryVersion` final.  I am not sure how to title the JIRA for 
> this, any suggestions?

I think that current behavior is correct. Constructor should take only 
necessary fields, others should be added by setters. Repositoryversion is not 
needed for kerberezation, so it is right that it's not in the constructor and 
is not final. I'd suggest to remove that line with TODO and that's all.
But if you feel that it's needed to make different request classes for 
different set of checks - then feel free to create a jira for that.


- Yurii


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


On Янв. 19, 2015, 4:08 п.п., Robert Levas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30038/
> -----------------------------------------------------------
> 
> (Updated Янв. 19, 2015, 4:08 п.п.)
> 
> 
> Review request for Ambari, Emil Anca, John Speidel, Nate Cole, Tom Beerbower, 
> and Yurii Shylov.
> 
> 
> Bugs: AMBARI-9102
>     https://issues.apache.org/jira/browse/AMBARI-9102
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The logic in the org.apache.ambari.server.state.UpgradeCheckHelper should be 
> exposed so that resources may use it.  
> 
> The following steps will be taken:
> * Move the static classes out of 
> `org.apache.ambari.server.state.UpgradeCheckHelper` and into a package named 
> `org.apache.ambari.server.checks`
> * Rename `UpgradeCheckDescriptor` to `AbstractCheckDescriptor`}
> * Create `org.apache.ambari.server.checks.CheckHelper` and move 
> `org.apache.ambari.server.state.UpgradeCheckHelper#performPreUpgradeChecks` 
> into it
> * Adjust 
> `org.apache.ambari.server.controller.internal.PreUpgradeCheckResourceProvider`
>  accordingly
> * `UpgradeCheckRequest`, `UpgradeCheck` and other similar resources which 
> encapsulate check data will be generalized to `PrereqCheckRequest`, 
> `PrerequisiteCheck` and so on
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/AbstractCheckDescriptor.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/HostsHeartbeatCheck.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/HostsMasterMaintenanceCheck.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/HostsRepositoryVersionCheck.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/ServicesDecommissionCheck.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/ServicesJobsDistributedCacheCheck.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/ServicesMaintenanceModeCheck.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/ServicesNamenodeHighAvailabilityCheck.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/ServicesUpCheck.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/checks/ServicesYarnWorkPreservingCheck.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/PreUpgradeCheckRequest.java
>  bc7e7f3 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/PrereqCheckRequest.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PreUpgradeCheckResourceProvider.java
>  d4ec2de 
>   ambari-server/src/main/java/org/apache/ambari/server/state/CheckHelper.java 
> PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeCheckHelper.java
>  d3b2df2 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/PrereqCheckStatus.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/PrereqCheckType.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/PrerequisiteCheck.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/UpgradeCheck.java
>  80c4bbf 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/UpgradeCheckStatus.java
>  ee70525 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/UpgradeCheckType.java
>  dfef47e 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/CheckHelperTest.java
>  PRE-CREATION 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeCheckHelperTest.java
>  c5f27c7 
> 
> Diff: https://reviews.apache.org/r/30038/diff/
> 
> 
> Testing
> -------
> 
> Updated unit tests
> 
> #Jenkins Test Results
> 
> Running org.apache.ambari.server.state.CheckHelperTest
> Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.817 sec
> 
> Full ambari-server test suite
> Tests run: 2546, Failures: 0, Errors: 0, Skipped: 15
> 
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Total time: 01:03 h
> [INFO] Finished at: 2015-01-19T14:49:21+00:00
> [INFO] Final Memory: 43M/495M
> [INFO] 
> ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Robert Levas
> 
>

Reply via email to