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




ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatHandler.java
 (lines 791 - 794)
<https://reviews.apache.org/r/43425/#comment180184>

    With only 2 lines, can we get rid of this method and just invoke the event 
publisher directly?



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java
 (line 1171)
<https://reviews.apache.org/r/43425/#comment180186>

    I dislike how we have 2 enums to actually track the same thing:
    
    UpgradeState
    RepositoryVersionState
    
    Having the RepositoryVersionState as "UPGRADING" is kind of weird since 
you're not upgrading the repo; you're upgrading to the repo. 
    
    But even worse is that we have this other enum, UpgradeState, with values 
like IN_PROGRESS.
    
    My vote is to actually change our code to eliminate 1 enum entirely. 
    
    It seems like the values from RepositoryVersionState are the ones we want 
(UPGRADING, UPGRADED) but we should call the enum UpgradeState to make it clear.
    
    Also, can we get rid of UpgradeState.PENDING since it makes zero sense.



ambari-server/src/main/java/org/apache/ambari/server/events/listeners/upgrade/StackVersionListener.java
 (line 104)
<https://reviews.apache.org/r/43425/#comment180187>

    If you're upgrading, shouldn't this be something like UPGRADED (see my 
earlier comment about too many enums and what we can do to make it clearer). 
NONE, to me, means nothing is happening, which is not the case.



ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostImpl.java
 (lines 1790 - 1794)
<https://reviews.apache.org/r/43425/#comment180188>

    Let's also update the documentation to reflect that this only happens 
during an upgrade.


- Jonathan Hurley


On Feb. 10, 2016, 1:55 p.m., Dmitro Lisnichenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43425/
> -----------------------------------------------------------
> 
> (Updated Feb. 10, 2016, 1:55 p.m.)
> 
> 
> Review request for Ambari, Jonathan Hurley and Nate Cole.
> 
> 
> Bugs: AMBARI-14996
>     https://issues.apache.org/jira/browse/AMBARI-14996
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> When performing an upgrade, we need to specify that a component's desired 
> version is getting changed.
> 
> Going to open additional jiras for TODOs in code
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatHandler.java
>  210fe17 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java
>  db8c079 
>   
> ambari-server/src/main/java/org/apache/ambari/server/events/HostComponentVersionEvent.java
>  ee65d3d 
>   
> ambari-server/src/main/java/org/apache/ambari/server/events/listeners/upgrade/StackVersionListener.java
>  74d4f4b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/events/publishers/VersionEventPublisher.java
>  3a11f38 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostComponentStateEntity.java
>  f92f645 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ServiceComponentDesiredStateEntity.java
>  d2d1b42 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/MasterHostResolver.java
>  561350b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponent.java
>  7803045 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentHost.java
>  f1e8d62 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentImpl.java
>  4afc857 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeState.java 
> ced1dd3 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostImpl.java
>  92828af 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostSummary.java
>  1c36143 
>   
> ambari-server/src/test/java/org/apache/ambari/server/events/listeners/upgrade/StackVersionListenerTest.java
>  ae05a6b 
>   
> ambari-server/src/test/java/org/apache/ambari/server/events/publishers/VersionEventPublisherTest.java
>  071c6f0 
> 
> Diff: https://reviews.apache.org/r/43425/diff/
> 
> 
> Testing
> -------
> 
> Pending E2E RU/EU upgrades/downgrades on live cluster
> 
> Did not work on unit tests yet
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>

Reply via email to